Re: [RFC PATCH 00/11] Add a percpu subsection for hot data

[Brian Gerst]

On Sun, Feb 23, 2025 at 5:30 AM Uros Bizjak <ubizjak@xxxxxxxxx> wrote:
>
> On Sun, Feb 23, 2025 at 11:20 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >
> > On Sun, 23 Feb 2025 at 10:37, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> > >
> > >
> > > * Brian Gerst <brgerst@xxxxxxxxx> wrote:
> > >
> > > > Add a new percpu subsection for data that is frequently accessed and
> > > > exclusive to each processor.  This is intended to replace the pcpu_hot
> > > > struct on X86, and is available to all architectures.
> > > >
> > > > The one caveat with this approach is that it depends on the linker to
> > > > effeciently pack data that is smaller than machine word size.  The
> > > > binutils linker does this properly:
> > > >
> > > > ffffffff842f6000 D __per_cpu_hot_start
> > > > ffffffff842f6000 D softirq_pending
> > > > ffffffff842f6002 D hardirq_stack_inuse
> > > > ffffffff842f6008 D hardirq_stack_ptr
> > > > ffffffff842f6010 D __ref_stack_chk_guard
> > > > ffffffff842f6010 D __stack_chk_guard
> > > > ffffffff842f6018 D const_cpu_current_top_of_stack
> > > > ffffffff842f6018 D cpu_current_top_of_stack
> > > > ffffffff842f6020 D const_current_task
> > > > ffffffff842f6020 D current_task
> > > > ffffffff842f6028 D __preempt_count
> > > > ffffffff842f602c D cpu_number
> > > > ffffffff842f6030 D this_cpu_off
> > > > ffffffff842f6038 D __x86_call_depth
> > > > ffffffff842f6040 D __per_cpu_hot_end
> > > >
> > > > The LLVM linker doesn't do as well with packing smaller data objects,
> > > > causing it to spill over into a second cacheline.
> > >
> > > ... now it's linker-ordered. But on the other hand that can be an
> > > advantage too: the linker will try to (or at least has a chance to)
> > > order the fields optimally for cache density, while the hand-packing
> > > always has the potential to bitrot without much of an outside,
> > > actionable indicator for the bitrot.
> > >
> >
> > The linker will need some help here - by default, it just emits these
> > variables in the order they appear in the input.
> >
> > If we emit each such variable 'foo' into .data..hot.foo, and define
> > the contents of the section as
> >
> > *(SORT_BY_ALIGNMENT(.data..hot.*))
> >
> > we should get optimal packing as long as the alignment of these
> > variables does not exceed their size.
>
> Is it possible to warn/error when data is spilled over the cache line?
> Previously, there was:
>
> -static_assert(sizeof(struct pcpu_hot) == 64);
>
> that failed the build in this case.

I think it should be a warning and not an error.  If it does spill
into a second cacheline the kernel will still boot and function
properly so it's not a fatal error, it just could hit performance a
bit.  By decentralizing this it does make it harder to account for
size, especially with conditional builds.  Unfortunately, the linker
script language does not have a WARNING() counterpart to ASSERT().


Brian Gerst