From:   Thomas Gleixner <tglx@linutronix.de>
To:     Eric Dumazet <edumazet@google.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linus Torvalds <torvalds@linuxfoundation.org>, x86@kernel.org,
        Wangyang Guo <wangyang.guo@intel.com>,
        Arjan van De Ven <arjan@linux.intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Marc Zyngier <maz@kernel.org>
Subject: Re: [patch 1/3] net: dst: Prevent false sharing vs.
 dst_entry::__refcnt
In-Reply-To: <CANn89iKM6yMP2Doy0MuCrfX1LASPFt_OnpPY-aNg+hu=F3W7AA@mail.gmail.com>
References: <20230228132118.978145284@linutronix.de>
 <20230228132910.934296889@linutronix.de>
 <CANn89iKM6yMP2Doy0MuCrfX1LASPFt_OnpPY-aNg+hu=F3W7AA@mail.gmail.com>
Date:   Tue, 28 Feb 2023 22:31:40 +0100
Message-ID: <875yblmq83.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Eric!

On Tue, Feb 28 2023 at 16:17, Eric Dumazet wrote:
> On Tue, Feb 28, 2023 at 3:33=E2=80=AFPM Thomas Gleixner <tglx@linutronix.=
de> wrote:
>> perf top identified two affected read accesses. dst_entry::lwtstate and
>> rtable::rt_genid.
>>
>> dst_entry:__refcnt is located at offset 64 of dst_entry, which puts it i=
nto
>> a seperate cacheline vs. the read mostly members located at the beginning
>> of the struct.
>
> This will probably increase struct rt6_info past the 4 cache line
> size, right ?

pahole says: /* size: 256, cachelines: 4, members: 11 */

> It would be nice to allow sharing the 'hot' cache line with seldom
> used fields.

Sure.

> Instead of mere pads, add some unions, and let rt6i_uncached/rt6i_uncache=
d_list
> use them.

If I understand correctly, you suggest to move

   rt6_info::rt6i_uncached[_list], rtable::rt_uncached[_list]
=20=20=20
into struct dst_entry and fixup the usage sites, right?

I don't see why that would need a union. dst_entry::rt_uncached[_list]
would work for both, no?

Thanks,

        tglx