Re: [PATCH] trace: convert init_trace_bufs() to constructor

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 13/03/2025 1:38 pm, Jan Beulich wrote:
> There's no need for each arch to invoke it directly, and there's no need
> for having a stub either. With the present placement of the calls to
> init_constructors() it can easily be a constructor itself.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

This has a side effect of wiring it up on RISC-V and PPC, as they
process constructors.  It looks safe enough, but have you double checked?


However, the position and logic during init is nonsense, I think.

It registers a cpu notifier which only does spin_lock_init() on a
per-cpu variable, which I think only works today because 0 is the init
value.

alloc_trace_bufs() on the other hand has a for_each_online_cpu() loop
because it's too late and ought to be a presmp_initcall().

Also the allocations could be NUMA-local for all but the biggest of
servers (given the 16T upper limit because there are raw uint32_t's
involved in the protocol).

I'm tempted to ack this on the basis that it is an improvement, but a /*
TODO this is all mad, please fix */ wouldn't go amiss either.

~Andrew