From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 325113A267; Tue, 9 Jan 2024 16:41:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E9ECAC433C7; Tue, 9 Jan 2024 16:41:02 +0000 (UTC) Date: Tue, 9 Jan 2024 11:42:00 -0500 From: Steven Rostedt To: Vincent Donnefort Cc: Masami Hiramatsu , linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com, kernel-team@android.com Subject: Re: [PATCH v10 1/2] ring-buffer: Introducing ring-buffer mapping functions Message-ID: <20240109114200.01367342@gandalf.local.home> In-Reply-To: References: <20240105094729.2363579-1-vdonnefort@google.com> <20240105094729.2363579-2-vdonnefort@google.com> <20240109234230.e99da87104d58fee59ad75c6@kernel.org> X-Mailer: Claws Mail 3.19.1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 9 Jan 2024 15:13:51 +0000 Vincent Donnefort 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