* [PATCH] printk: ringbuffer: fix error in comment @ 2026-03-28 2:18 Loïc Grégoire 2026-03-30 8:06 ` John Ogness 0 siblings, 1 reply; 5+ messages in thread From: Loïc Grégoire @ 2026-03-28 2:18 UTC (permalink / raw) To: Petr Mladek, Steven Rostedt, John Ogness, Sergey Senozhatsky, open list Cc: Loïc Grégoire 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> --- kernel/printk/printk_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c index 56c8e3d031f4..11dae8229103 100644 --- a/kernel/printk/printk_ringbuffer.c +++ b/kernel/printk/printk_ringbuffer.c @@ -14,7 +14,7 @@ * * Data Structure * -------------- - * The printk_ringbuffer is made up of 3 internal ringbuffers: + * The printk_ringbuffer is made up of 2 internal ringbuffers: * * desc_ring * A ring of descriptors and their meta data (such as sequence number, -- 2.53.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] printk: ringbuffer: fix error in comment 2026-03-28 2:18 [PATCH] printk: ringbuffer: fix error in comment Loïc Grégoire @ 2026-03-30 8:06 ` John Ogness 2026-03-30 13:12 ` Petr Mladek 0 siblings, 1 reply; 5+ messages in thread From: John Ogness @ 2026-03-30 8:06 UTC (permalink / raw) To: Loïc Grégoire, Petr Mladek, Steven Rostedt, Sergey Senozhatsky, open list Cc: Loïc Grégoire 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> 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? John 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. */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] printk: ringbuffer: fix error in comment 2026-03-30 8:06 ` John Ogness @ 2026-03-30 13:12 ` Petr Mladek 2026-03-30 14:14 ` John Ogness 0 siblings, 1 reply; 5+ messages in thread From: Petr Mladek @ 2026-03-30 13:12 UTC (permalink / raw) To: John Ogness Cc: Loïc Grégoire, Steven Rostedt, Sergey Senozhatsky, open list 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? Best Regards, Petr ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] printk: ringbuffer: fix error in comment 2026-03-30 13:12 ` Petr Mladek @ 2026-03-30 14:14 ` John Ogness 2026-03-31 15:56 ` Petr Mladek 0 siblings, 1 reply; 5+ messages in thread From: John Ogness @ 2026-03-30 14:14 UTC (permalink / raw) To: Petr Mladek Cc: Loïc Grégoire, Steven Rostedt, Sergey Senozhatsky, open list 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? Sure. Signed-off-by: John Ogness <john.ogness@linutronix.de> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] printk: ringbuffer: fix error in comment 2026-03-30 14:14 ` John Ogness @ 2026-03-31 15:56 ` Petr Mladek 0 siblings, 0 replies; 5+ messages in thread From: Petr Mladek @ 2026-03-31 15:56 UTC (permalink / raw) To: John Ogness Cc: Loïc Grégoire, Steven Rostedt, Sergey Senozhatsky, open list 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-31 15:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-03-28 2:18 [PATCH] printk: ringbuffer: fix error in comment 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 is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox