On Tue, 9 Jan 2024 15:13:51 +0000
Vincent Donnefort <vdonnef...@google.com> wrote:
> > > @@ -388,6 +389,7 @@ struct rb_irq_work {
> > > bool waiters_pending;
> > > bool full_waiters_pending;
> > > bool wakeup_full;
> > > + bool is_cpu_buffer;
> >
> > I think 'is_cpu_buffer' is a bit unclear (or generic),
> > what about 'meta_page_update'?
>
> Hum not sure about that change. This was really to identify if parent of
> rb_irq_work is a cpu_buffer or a trace_buffer. It can be a cpu_buffer
> regardless
> of the need to update the meta-page.
Yeah, this was added because the irq_work is called with the rb_work for
both the cpu_buffer and the global struct tracing_buffer object. The
meta_page is only available to the cpu_buffer and does not exist on the
struct trace_buffer, so this is checked before doing a "container_of()" on
the wrong structure. Both the cpu_buffer and the global buffer call the
same function with the rb_work structure.
So "is_cpu_buffer" is the right terminology as it's unrelated to the meta page:
struct trace_buffer {
[..]
struct rb_irq_work irq_work;
[..]
};
struct trace_buffer *buffer;
buffer->irq_work.is_cpu_buffer = false;
struct ring_buffer_per_cpu {
[..]
struct rb_irq_work irq_work;
[..]
}
struct ring_buffer_per_cpu *cpu_buffer;
cpu_buffer->irq_work.is_cpu_buffer = true;
[..]
init_irq_work(&buffer->irq_work.work, rb_wake_up_waiters);
[..]
init_irq_work(&cpu_buffer->irq_work.work, rb_wake_up_waiters);
// both the buffer and cpu_buffer call rb_wake_up_waiters()
static void rb_wake_up_waiters(struct irq_work *work)
{
struct rb_irq_work *rbwork = container_of(work, struct rb_irq_work,
work);
// This container_of() gets rbwork which is the rb_irq_work structure that
// both buffer and cpu_buffer have.
if (rbwork->is_cpu_buffer) {
struct ring_buffer_per_cpu *cpu_buffer;
cpu_buffer = container_of(rbwork, struct ring_buffer_per_cpu,
irq_work);
// The above crashes if done by the buffer and not the cpu_buffer.
// The "is_cpu_buffer" is there to differentiate the two rb_work entities.
// It is a way to say this is safe to do the above "container_of()".
/*
* If the waiter is a cpu_buffer, this might be due to a
* userspace mapping. Let's update the meta-page.
*/
rb_update_meta_page(cpu_buffer);
}
-- Steve
