Re: [PATCH RFC 0/4] fs: port files to rcuref_long_t

[Christian Brauner <brauner@kernel.org>]

On Sun, Oct 06, 2024 at 11:09:30AM GMT, Linus Torvalds wrote:
> On Sun, 6 Oct 2024 at 03:21, Christian Brauner <brauner@kernel.org> wrote:
> >
> > Iiuc, then we should retain the deadzone handling but should replace
> > atomic_long_add_negative() with atomic_long_add_negative_relaxed().
> 
> I assume you meant the other way around.
> 
> However, then it's not the same as the regular rcuref any more. It
> looks similar, it sounds similar, but it's something completely
> different.
> 
> I definitely do *not* want to have "rcuref_long_get()" fundamentally
> different from just plain "rcuref_get()" .

Right, that's why I added a separate helper. IOW, I didn't change the
behavior of the helper.

> 
> Now, maybe we should just make the plain version also do a full memory
> barrier. Honestly, we have exactly *one* user of rcyref_get(): the
> networking code dst cache. Using the relaxed op clearly makes no
> difference at all on x86, and it _probably_ makes little to no
> difference on other relevant architectures either.
> 
> But if the networking people want their relaxed version, I really
> really don't want rcuref_long_get() using non-relaxed one. And with
> just one single user of the existing rcuref code, and now another
> single user of the "long" variant, I really don't think it makes much
> sense as a "library".
> 
> IOW, my gut feeling is that you'd actually be better off just taking
> the rcuref code, changing it to using atomic_long_t and the
> non-relaxed version, and renaming it to "file_ref", and keep it all
> purely in fs/file.c (actually right now it's oddly split between
> fs/file.c and fs/file_table.c, but whatever - you get the idea).
> 
> Trying to make it a library when it has one user and that one user
> wants a very very different model than the other user that looked
> similar smells like a BAD idea to me.

Ok, sounds good.