[RESEND PATCH] cred: separate the refcount from frequently read fields

Mateusz Guzik mjguzik at gmail.com
Thu Aug 22 21:31:39 UTC 2024


On Thu, Aug 22, 2024 at 10:58 PM Paul Moore <paul at paul-moore.com> wrote:
>
> On Thu, Aug 22, 2024 at 9:15 AM Mateusz Guzik <mjguzik at gmail.com> wrote:
> >
> > The refcount shares the cacheline with uid, gid and other frequently
> > read fields.
> >
> > Said counter gest modified a lot (for example every time a file is
> > closed or opened) in turn causing avoidable bounces when another thread
> > tries to do permission checks. Said bouncing can be avoided without
> > growing the struct by reordering the fields -- keyring (enabled by
> > default) is comparatively rarely used and can suffer bouncing instead.
> >
> > An additional store is performed to clear the non_rcu flag. Since the
> > flag is rarely set (a special case in the access(2) system call) and
> > transitions at most once, it can get placed in a read-mostly area and is
> > only conditionally written to.
> >
> > With this in place regular permission checks no longer bounce cachelines
> > in face of refcount changes.
> >
> > Validated with a simple test where one thread opens and closes a file
> > (dirtying creds twice), while another keeps re-reading *another* file in
> > a loop (ops/s):
> > before: 4353763
> > after:  4742792 (+9%)
> >
> > Signed-off-by: Mateusz Guzik <mjguzik at gmail.com>
> > ---
> >
> > This is a resend with a reworded commit message and added Linus since he
> > wrote the non_rcu thing.
> >
> > Note each process has its on creds, so this is not causing bounces
> > globally.
> >
> > Even so, there is stuff I plan to do in the area and this patch can be
> > considered prep (only one store to non_rcu).
> >
> > I'll also note I don't see a way to *whack* non_rcu either. :)
> >
> > 0 rush
> >
> >  fs/open.c            |  2 +-
> >  include/linux/cred.h | 31 +++++++++++++++----------------
> >  kernel/cred.c        |  6 +++---
> >  3 files changed, 19 insertions(+), 20 deletions(-)
>
> [NOTE: no comment on the patch context yet, just process comments below]
>
> FWIW, I really haven't commented on these last couple of cred patches
> mostly because I've been assuming someone else would emerge from the
> shadows as the cred maintainer, or at least someone who felt strongly
> enough about these changes would merge them.  Unfortunately, I worry
> that isn't really going to happen and I'd hate for some of the cred
> improvements to die on the lists.
>
> If no one starts grabbing your cred patches I can start taking cred
> patches as part of the LSM tree, I've done it a couple of times in the
> past and Linus didn't seem to mind.
>

ye I remember trouble getting any response in the past.

fwiw I expect to write one more patch (maybe split into two) to
distribute the refcount into task_struct, then modulo a bugfix should
someone find a problem hopefully I wont be submitting any more cred
stuff :)

-- 
Mateusz Guzik <mjguzik gmail.com>



More information about the Linux-security-module-archive mailing list