Re: [RFC PATCH 02/11] ring-buffer: Introducing ring-buffer writer

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 13 Aug 2024 15:21:54 +0100
Vincent Donnefort <vdonnefort@google.com> 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