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

Linus Torvalds torvalds at linux-foundation.org
Fri Aug 23 15:25:54 UTC 2024


On Fri, 23 Aug 2024 at 20:33, Mateusz Guzik <mjguzik at gmail.com> wrote:
>
> On Fri, Aug 23, 2024 at 2:06 AM Linus Torvalds
> <torvalds at linux-foundation.org> wrote:
> >
> >
> > Yes, it is rarely actually written to and as such can be "mostly
> > read-only", but since it is both read and written next to refcounts,
> > why do that?
> >
> > Did I miss some common use?
> >
>
> It gets looked at every time you grab a ref.

Mateusz - read my email. That's what I daid.

But the *ref* is already in cacheline 0. With your change it looked like this:

   struct cred {
        atomic_long_t   usage;
        struct rcu_head rcu;            /* RCU deletion hook */

and if you had kept the union with that 'struct rcu_head', then
'non_rcu' would be RIGHT THERE.

> Thus consumers which grab the ref and then look at the most commonly
> used fields get the non_rcu + rest combo "for free".

They'd get it for free JUST BECAUSE IT'S NEXT TO THE REF. In cacheline
0 - that is already dirtied by the reference count. Which makes a
*store* cheaper.

And you also wouldn't waste separate space for it.

> consumers which already had a ref don't suffer any extra misses

Consumers that already had a ref don't touch 'non_rcu' at all as far
as I can see, so they have no reason to have it next to those "most
commonly used fields".

See my argument? You seem to have pointlessly separated out the
'non_rcu' from being together with the rcu_head, and thus wasted
potentially useful space in the structure.

Your own email even had that:

>        bool                       non_rcu;              /*   100     1 */
>
>        /* XXX 3 bytes hole, try to pack */

which would have been a /* 4 byte hole, try to pack */

A 4-byte hole is more useful than a 3-byte one. A 3-byte one is much
harder to use.

In fact, even without the union, I think your current cacheline 0 ends
up having a 3-byte hole due to that

        unsigned char   jit_keyring;    /* default keyring to attach requested

with CONFIG_KEYS.

Without CONFIG_KEYS, you have something like a 40-byte hole there due to the

    kuid_t uid ____cacheline_aligned_in_smp;

which seems very wasteful, but I guess CONFIG_KEYS is the common case.

So I repeat: what did I miss?

       Linus



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