On Sun, Oct 06, 2024 at 11:09:30AM GMT, Linus Torvalds wrote: > On Sun, 6 Oct 2024 at 03:21, Christian Brauner <brauner@xxxxxxxxxx> 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.
