On Tue, 13 Aug 2024 15:21:54 +0100
Vincent Donnefort <[email protected]> wrote:
> > > +
> > > +#define for_each_rb_page_desc(__pdesc, __cpu, __trace_pdesc)
> > > \
> > > + for (__pdesc = (struct rb_page_desc *)&((__trace_pdesc)->__data[0]),
> > > __cpu = 0; \
> > > + __cpu < (__trace_pdesc)->nr_cpus;
> > > \
> > > + __cpu++, __pdesc = __next_rb_page_desc(__pdesc))
> > > +
> > > +static inline
> > > +struct rb_page_desc *rb_page_desc(struct trace_page_desc *trace_pdesc,
> > > int cpu)
> > > +{
> > > + struct rb_page_desc *pdesc;
> > > + int i;
> > > +
> > > + if (!trace_pdesc)
> > > + return NULL;
> > > +
> > > + for_each_rb_page_desc(pdesc, i, trace_pdesc) {
> > > + if (pdesc->cpu == cpu)
> >
> > Is there a reason for the linear search?
> >
> > Why not just:
> >
> > if (cpu >= trace_pdesc->nr_cpus)
> > return NULL;
> >
> > len = struct_size(pdesc, page_va, pdesc->nr_page_va);
> > pdesc = (void *)&(trace_pdesc->__data[0]);
> >
> > return pdesc + len * cpu;
> >
> > (note I don't think you need to typecast the void pointer).
>
> I supposed we can't assume buffers will be allocated for each CPU, hence the
> need to look at each buffer.
OK, but by default that should be the fast path. We could add the above
and then do:
pdesc += len * cpu;
if (pdesc->cpu == cpu)
return pdesc;
/* Missing CPUs, need to do a linear search */
for_each_rb_page_desc(pdesc, i, trace_pdesc) {
if (pdesc->cpu == cpu)
[...]
-- Steve
