Re: [PATCH printk v3 5/7] printk: nbcon: Add sequence handling

[Petr Mladek]

On Sun 2023-09-03 17:11:37, John Ogness wrote:
> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> 
> Add an atomic_long_t field @nbcon_seq to the console struct to
> store the sequence number for nbcon consoles. For nbcon consoles
> this will be used instead of the non-atomic @seq field. The new
> field allows for safe atomic sequence number updates without
> requiring any locking.
> 
> On 64bit systems the new field stores the full sequence number.
> On 32bit systems the new field stores the lower 32 bits of the
> sequence number, which are expanded to 64bit as needed by
> folding the values based on the sequence numbers available in
> the ringbuffer.
> 
> For 32bit systems, having a 32bit representation in the console
> is sufficient. If a console ever gets more than 2^31 records
> behind the ringbuffer then this is the least of the problems.
> 
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> +/**
> + * nbcon_seq_try_update - Try to update the console sequence number
> + * @ctxt:	Pointer to an acquire context that contains
> + *		all information about the acquire mode
> + * @new_seq:	The new sequence number to set
> + *
> + * @ctxt->seq is updated to the new value of @con::nbcon_seq. This could
> + * be a different value than @new_seq if nbcon_seq_force() was used or the
> + * current context no longer owns the console. In the later case, it will
> + * stop printing anyway.
> + */
> +__maybe_unused
> +static void nbcon_seq_try_update(struct nbcon_context *ctxt, u64 new_seq)
> +{
> +	unsigned long nbcon_seq = __seq_to_nbcon_seq(ctxt->seq);
> +	struct console *con = ctxt->console;
> +
> +	if (atomic_long_try_cmpxchg(&ACCESS_PRIVATE(con, nbcon_seq), &nbcon_seq,
> +				    __seq_to_nbcon_seq(new_seq))) {
> +		ctxt->seq = new_seq;
> +	} else {
> +		ctxt->seq = atomic_long_read(&ACCESS_PRIVATE(con, nbcon_seq));

This should be:

		ctxt->seq = nbcon_seq_read(con);

so that it extends the 32-bit nbcon_seq to the full 64-bit seq on
32-bit systems.

> +	}
> +}
> +
>  /**
>   * nbcon_context_try_acquire_direct - Try to acquire directly
>   * @ctxt:	The context of the caller

Otherwise, it looks good. With the above fix:

Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>

Best Regards,
Petr