Re: hostile takeover: Re: [PATCH printk v2 3/8] printk: nbcon: Add acquire/release logic

[John Ogness]

On 2023-09-07, Petr Mladek <pmladek@xxxxxxxx> wrote:
> Hmm, I am actually not sure if nbcon_emit_next_record()
> calls printk_get_next_message() and con->write_atomic(con, wctxt)
> in "safe" or "unsafe" context.

It does not! In the early versions of this code it was not a problem
because we had a per-priority buffer for each console. But now we only
have one buffer that is shared by the NORMAL and EMERGENCY priorities
per console. For v4 I am creating a safe region around
printk_get_next_message() and console_prepend_dropped(). That will fix
the issue. Nice catch!

For the callbacks write_atomic() (and later, write_thread()) it should
not be called in unsafe. It is up to the drivers to decide what is safe
and unsafe.

>> 1. try direct
>> 2. try safe takeover
>> 3. try handover
>> 4. try hostile takeover
>
> I rather meant:
>
>   1. try direct
>   2. try handover +
>      try safe takeover in every waiting cycle
>   3. try hostile takeover
>
> But then it won't be a handover anymore.
>
> Anyway, I would keep it as is for now. As mentioned above,
> the current handover is more conservative approach because
> the lock is passed on well defined locations.

For v3 I made the change as I suggested above. So it will perform a
direct takeover if the priority is higher and it is safe. Yes, waiting
first might seem more polite and conservative. But if the driver is in a
safe state, I see no reason to make a higher priority context wait. If
it would be a problem for the first context to suddenly lose ownership,
then it should be marking it an unsafe region.

For fun I implemented it such that it only directly takes over an owner
after having waited. But it adds quite a bit of ugliness to the routine
and I don't think it is worth it.

For v4 I will keep it the same as v3: a direct takeover of an existing
owner when safe before trying the handover.

John Ogness