Re: [PATCH] printk: ringbuffer: fix error in comment

[John Ogness]

On 2026-03-30, Petr Mladek <pmladek@xxxxxxxx> wrote:
> On Mon 2026-03-30 10:12:08, John Ogness wrote:
>> On 2026-03-27, Loïc Grégoire <loicgre@xxxxxxxxx> 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@xxxxxxxxx>
>> 
>> Reviewed-by: John Ogness <john.ogness@xxxxxxxxxxxxx>
>
> Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
>
>> 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@xxxxxxxx>
>
> 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@xxxxxxxxxxxxx>