From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: "Loïc Grégoire" <loicgre@gmail.com>,
"Steven Rostedt" <rostedt@goodmis.org>,
"Sergey Senozhatsky" <senozhatsky@chromium.org>,
"open list" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] printk: ringbuffer: fix error in comment
Date: Tue, 31 Mar 2026 17:56:57 +0200 [thread overview]
Message-ID: <acvuyU-QKqofwjYt@pathway.suse.cz> (raw)
In-Reply-To: <87mrzpmmk0.fsf@jogness.linutronix.de>
On Mon 2026-03-30 16:20:39, John Ogness wrote:
> On 2026-03-30, Petr Mladek <pmladek@suse.com> wrote:
> > On Mon 2026-03-30 10:12:08, John Ogness wrote:
> >> On 2026-03-27, Loïc Grégoire <loicgre@gmail.com> wrote:
> >> > The printk ringbuffer implementation is described in the comment as
> >> > using three ringbuffers, but the current implementation uses two (desc
> >> > and data). Update the comment so it matches the code.
> >> >
> >> > Signed-off-by: Loïc Grégoire <loicgre@gmail.com>
> >>
> >> Reviewed-by: John Ogness <john.ogness@linutronix.de>
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> >
> >> However, this is only a single fixup when there are other documentation
> >> errors that need fixing as well. Does it make sense to commit such tiny
> >> fixups separately? For example, below are needed fixes that I am aware
> >> of.
> >>
> >> Perhaps we should fold these into a single doc-fixup patch?
> >>
> >> diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
> >> index 56c8e3d031f49..3f019207d2688 100644
> >> --- a/kernel/printk/printk_ringbuffer.c
> >> +++ b/kernel/printk/printk_ringbuffer.c
> >> @@ -224,7 +224,7 @@
> >> *
> >> * prb_rec_init_rd(&r, &info, &text_buf[0], sizeof(text_buf));
> >> *
> >> - * prb_for_each_record(0, &test_rb, &seq, &r) {
> >> + * prb_for_each_record(0, &test_rb, seq, &r) {
> >> * if (info.seq != seq)
> >> * pr_warn("lost %llu records\n", info.seq - seq);
> >> *
> >> @@ -1365,7 +1365,7 @@ static struct prb_desc *desc_reopen_last(struct prb_desc_ring *desc_ring,
> >> *
> >> * WMB from _prb_commit:A to _prb_commit:B
> >> * matching
> >> - * MB If desc_reopen_last:A to prb_reserve_in_last:A
> >> + * MB from desc_reopen_last:A to prb_reserve_in_last:A
> >> */
> >> if (!atomic_long_try_cmpxchg(&d->state_var, &prev_state_val,
> >> DESC_SV(id, desc_reserved))) { /* LMM(desc_reopen_last:A) */
> >> @@ -1770,9 +1770,9 @@ static void _prb_commit(struct prb_reserved_entry *e, unsigned long state_val)
> >> *
> >> * Relies on:
> >> *
> >> - * MB _prb_commit:B to prb_commit:A
> >> + * MB from _prb_commit:B to prb_commit:A
> >> * matching
> >> - * MB desc_reserve:D to desc_make_final:A
> >> + * MB from desc_reserve:D to desc_make_final:A
> >> */
> >> if (!atomic_long_try_cmpxchg(&d->state_var, &prev_state_val,
> >> DESC_SV(e->id, state_val))) { /* LMM(_prb_commit:B) */
> >> @@ -2035,7 +2035,7 @@ u64 prb_first_seq(struct printk_ringbuffer *rb)
> >> *
> >> * MB from desc_push_tail:B to desc_reserve:F
> >> * matching
> >> - * RMB prb_first_seq:B to prb_first_seq:A
> >> + * RMB from prb_first_seq:B to prb_first_seq:A
> >> */
> >> smp_rmb(); /* LMM(prb_first_seq:C) */
> >> }
> >> diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
> >> index 1651b53ece34f..fb7d2ee29af23 100644
> >> --- a/kernel/printk/printk_ringbuffer.h
> >> +++ b/kernel/printk/printk_ringbuffer.h
> >> @@ -388,7 +388,7 @@ for ((s) = from; prb_read_valid(rb, s, r); (s) = (r)->info->seq + 1)
> >> *
> >> * This is a macro for conveniently iterating over a ringbuffer.
> >> * Note that @s may not be the sequence number of the record on each
> >> - * iteration. For the sequence number, @r->info->seq should be checked.
> >> + * iteration. For the sequence number, @i->seq should be checked.
> >> *
> >> * Context: Any context.
> >> */
> >
> > All these changes look correct as well:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> >
> > I could squash these changes into the original patch when comitting
> > into printk/linux.git.
> >
> > John, could I use your Signed-off-by to mark you as a co-author
> > and keep Loïc as the author of the patch?
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
JFYI, the patch has been comitted into printk/linux.git,
branch rework/prb-fixes.
I have merged the extra fixes proposed by John, see
https://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/commit/?h=rework/prb-fixes&id=bf56987c111372a54ae877934a42f7fb0953a6ca
Best Regards,
Petr
prev parent reply other threads:[~2026-03-31 15:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-28 2:18 Loïc Grégoire
2026-03-30 8:06 ` John Ogness
2026-03-30 13:12 ` Petr Mladek
2026-03-30 14:14 ` John Ogness
2026-03-31 15:56 ` Petr Mladek [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=acvuyU-QKqofwjYt@pathway.suse.cz \
--to=pmladek@suse.com \
--cc=john.ogness@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=loicgre@gmail.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.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
Powered by JetHome