On Tue 11-03-25 19:18:04, Mateusz Guzik wrote: > While the structure is refcounted, the only consumer incrementing it is > audit and even then the atomic operation is only needed when it > interacts with io_uring. > > If putname spots a count of 1, there is no legitimate way for anyone to > bump it. > > If audit is disabled, the count is guaranteed to be 1, which > consistently elides the atomic for all path lookups. If audit is > enabled, it still manages to elide the last decrement. > > Note the patch does not do anything to prevent audit from suffering > atomics. See [1] and [2] for a different approach. > > Benchmarked on Sapphire Rapids issuing access() (ops/s): > before: 5106246 > after: 5269678 (+3%) > > Link 1: > https://lore.kernel.org/linux-fsdevel/[email protected]/ > Link 2: https://lore.kernel.org/linux-fsdevel/20250307164216.GI2023217@ZenIV/ > Signed-off-by: Mateusz Guzik <[email protected]>
Yeah, I like this much more than the original. Feel free to add: Reviewed-by: Jan Kara <[email protected]> Honza > This is an alternative to the patch I linked above. > > I think the improved commit message should also cover the feedback > Christian previously shared concerning it. > > This is a trivial win until the atomic issue gets resolved, I don't see > any reason to NOT include it. At the same time I don't have that much > interest arguing about it either. > > That is to say, here is a different take, if you don't like it, I'm > dropping the subject. > > cheers > > fs/namei.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/namei.c b/fs/namei.c > index 06765d320e7e..add90981cfcd 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -275,14 +275,19 @@ EXPORT_SYMBOL(getname_kernel); > > void putname(struct filename *name) > { > + int refcnt; > + > if (IS_ERR_OR_NULL(name)) > return; > > - if (WARN_ON_ONCE(!atomic_read(&name->refcnt))) > - return; > + refcnt = atomic_read(&name->refcnt); > + if (refcnt != 1) { > + if (WARN_ON_ONCE(!refcnt)) > + return; > > - if (!atomic_dec_and_test(&name->refcnt)) > - return; > + if (!atomic_dec_and_test(&name->refcnt)) > + return; > + } > > if (name->name != name->iname) { > __putname(name->name); > -- > 2.43.0 > -- Jan Kara <[email protected]> SUSE Labs, CR
