From: Jacob Keller <[email protected]> Date: Tue, 28 May 2024 17:43:34 -0700
> > > On 5/28/2024 6:48 AM, Alexander Lobakin wrote: >> Now that the queue and queue vector structures are separated and laid >> out optimally, group the fields as read-mostly, read-write, and cold >> cachelines and add size assertions to make sure new features won't push >> something out of its place and provoke perf regression. > > > >> Despite looking innocent, this gives up to 2% of perf bump on Rx. >> > > Could you explain this a bit more for my education? This patch does > clearly change the layout from what it was before this patch, but the > commit message here claims it was already laid out optimally? I guess > that wasn't 100% true? Or do these group field macros also provide > further hints to the compiler about read_mostly or cold, etc? Queue structure split placed fields grouped more optimally, but didn't place ro/rw/cold into separate cachelines. This commit performs the separation via libeth_cacheline_group(). Doing that in one commit didn't look atomically, especially given that the queue split is already big enough. > >> Reviewed-by: Przemek Kitszel <[email protected]> >> Signed-off-by: Alexander Lobakin <[email protected]> >> --- > > Having the compiler assert some of this so that we can more easily spot > regressions in the layout is a big benefit. [...] >> @@ -504,59 +505,70 @@ struct idpf_intr_reg { >> >> /** >> * struct idpf_q_vector >> + * @read_mostly: CL group with rarely written hot fields > > I wonder if there is a good way to format the doc here since we almost > want read_mostly to be some sort of header making it clear which fields > belong to it? I don't know how we'd achieve that with current kdoc though. Since commit [0], we need to explicitly describe struct groups in kdocs. @read_mostly and friends are struct groups themselves and in the first patch, where I add these macros, I also add them to the kdoc script, so that it treats them as struct groups, thus they also need to be described. Given that one may use libeth_cacheline_group() to declare some custom groups, like libeth_cacheline_group(my_cl, fields ); it makes sense as I'd like to know what this @my_cl is about. Here I use "default" CL names, so this kdocs looks like Ctrl-{C,V} explaining obvious things :D [0] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=5f8e4007c10d Thanks, Olek
