Re: [PATCH] printk/console: Check consistent sequence number when handling race in console_unlock()

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (21/06/30 09:58), Petr Mladek wrote:
[..]
> > `retry` can be falsely set, console_trylock() does not spin on owner,
> > so the context that just released the lock can grab it again only if
> > it's unlocked. For the context that just has released the console_sem
> > and then acquired it again, because of the race, - console_seq will be
> > valid after it acquires the lock, then it'll jump to `retry` and
> > re-validated the console_seq - prb_read_valid(). If it's valid, it'll
> > print the message; and should another CPU printk that CPU will spin on
> > owner and then the current console_sem owner will yield to it via
> > console_lock_spinning branch.
> 
> I am not sure that I follow it correctly. IMHO, there are two possible
> races. I believe that you are talking about the 2nd scenario:

I guess I was thinking about two scenarios simultaneously, but you
certainly did a much better job describing them.

Thanks a lot for spending time on this!

> 1st scenario: console_unlock() retries but the message has been proceed
>    in the meantime:
[..]
> Result: CPU0 retired just to realize that the message
> 	has already been procceed.

Ack.

> 2nd scenario: printk() caller spins when other process is already
> 	processing it's message
[..]
> Result: CPU1 was spinning just to realize that the message has already
> 	been proceed.

Ack.

> It is not ideal. But the result is always correct.
>
> The races have been there already before. Only the race window in 1st
> scenario was a bit smaller.

Yeah, this was my assertion as well, but I wanted to double check.