From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux trace kernel <linux-trace-kernel@vger.kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Dan Carpenter <dan.carpenter@linaro.org>,
outreachy@lists.linux.dev, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] tracing: Update snapshot buffer on resize if it is allocated
Date: Tue, 12 Dec 2023 08:14:13 +0900 [thread overview]
Message-ID: <20231212081413.c4710c1b643ac4e0b975a122@kernel.org> (raw)
In-Reply-To: <20231211125152.045f8b8a@gandalf.local.home>
On Mon, 11 Dec 2023 12:51:52 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 11 Dec 2023 21:31:34 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > On Sun, 10 Dec 2023 22:54:47 -0500
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > >
> > > The snapshot buffer is to mimic the main buffer so that when a snapshot is
> > > needed, the snapshot and main buffer are swapped. When the snapshot buffer
> > > is allocated, it is set to the minimal size that the ring buffer may be at
> > > and still functional. When it is allocated it becomes the same size as the
> > > main ring buffer, and when the main ring buffer changes in size, it should
> > > do.
> >
> > nit: There seems two "when the snapshot buffer is allocated" case, maybe latter
> > "it" means main buffer?
>
> I changed the paragraph to be:
>
> The snapshot buffer is to mimic the main buffer so that when a snapshot is
> needed, the snapshot and main buffer are swapped. When the snapshot buffer
> is allocated, it is set to the minimal size that the ring buffer may be at
> and still functional. When it is allocated it becomes the same size as the
> main ring buffer, and when the main ring buffer changes in size, the
> snapshot should also change in size if it is allocated.
Yeah, this makes clearer.
>
> >
> > >
> > > Currently, the resize only updates the snapshot buffer if it's used by the
> > > current tracer (ie. the preemptirqsoff tracer). But it needs to be updated
> > > anytime it is allocated.
> > >
> > > When changing the size of the main buffer, instead of looking to see if
> > > the current tracer is utilizing the snapshot buffer, just check if it is
> > > allocated to know if it should be updated or not.
> > >
> > > Also fix typo in comment just above the code change.
> > >
> >
> > Looks good to me.
> >
> > Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Thanks!
>
> >
> > BTW, the historical naming leads this kind of issues.
> > Maybe we'd better to rename 'max_buffer' to 'snapshot_buffer'?
>
> Agreed. But that's a cleanup for another day. Hmm, maybe that too should be
> marked as "KTODO"?
Yes!
>
> https://lore.kernel.org/all/369bc919-1a1d-4f37-9cc9-742a86a41282@kadam.mountain/
>
Ah, this is a good idea.
>
> There's a lot of things that we have been discussing on these ring-buffer
> patches that could be KTODO items.
Thanks,
>
> -- Steve
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
prev parent reply other threads:[~2023-12-11 23:14 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20231210225447.48476a6a@rorschach.local.home>
[not found] ` <20231211213134.bd21cf745b8c5a0892891946@kernel.org>
2023-12-11 17:51 ` [PATCH] tracing: Update snapshot buffer on resize if it is allocated Steven Rostedt
2023-12-11 23:14 ` Masami Hiramatsu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231212081413.c4710c1b643ac4e0b975a122@kernel.org \
--to=mhiramat@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=outreachy@lists.linux.dev \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).