On Fri, Nov 22, 2024 at 09:21:58PM -0800, Linus Torvalds wrote: > On Fri, 22 Nov 2024 at 01:57, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > - Introduction and use of revert/override_creds_light() helpers, that were > > suggested by Christian as a mitigation to cache line bouncing and false > > sharing of fields in overlayfs creator_cred long lived struct cred copy. > > So I don't actively hate this, but I do wonder if this shouldn't have > been done differently. > > In particular, I suspect *most* users of override_creds() actually > wants this "light" version, because they all already hold a ref to the > cred that they want to use as the override. > > We did it that safe way with the extra refcount not because most > people would need it, but it was expected to not be a big deal. > > Now you found that it *is* a big deal, and instead of just fixing the > old interface, you create a whole new interface and the mental burden > of having to know the difference between the two. > So may I ask that you look at perhaps just converting the (not very > many) users of the non-light cred override to the "light" version? I think that could be a good idea in general. But I have to say I'm feeling a bit defensive after having read your message even though I usually try not to. :) So just to clarify when that issue was brought up I realized that the cred bump was a big deal for overlayfs but from a quick grep I didn't think for any of the other cases it really mattered that much. Realistically, overlayfs is the prime example where that override cred matters big time because it's called everywhere and in all core operations one can think of. But so far I at least haven't heard complaints outside of that and so the immediate focus was to bring about a solution for overlayfs. The reason the revert_creds_light() variant doesn't return the old creds is so that callers don't put_cred() them blindly. Because for overlayfs (and from a quick glance io_uring and nfs) the refcount for the temporary creds is kept completely independent of the callsites. The lifetime is bound to the superblock and so the final put on the temporary creds has nothing to do with the callers at all.
