Re: [PATCH 2/4] serial: core: fix broken sysrq port unlock

From: Dmitry Safonov
Date: Tue Jun 02 2020 - 11:34:25 EST


On 6/2/20 3:48 PM, Andy Shevchenko wrote:
> On Tue, Jun 2, 2020 at 5:03 PM Johan Hovold <johan@xxxxxxxxxx> wrote:
>>
>> Commit d6e1935819db ("serial: core: Allow processing sysrq at port
>> unlock time") worked around a circular locking dependency by adding
>> helpers used to defer sysrq processing to when the port lock was
>> released.
>>
>> A later commit unfortunately converted these inline helpers to exported
>> functions despite the fact that the unlock helper was restoring irq
>> flags, something which needs to be done in the same function that saved
>> them (e.g. on SPARC).
>
> I'm not familiar with sparc, can you elaborate a bit what is ABI /
> architecture lock implementation background?

I remember that was a limitation a while ago to save/restore flags from
the same function. Though, I vaguely remember the reason.
I don't see this limitation in Documentation/*

Google suggests that it's related to storage location:
https://stackoverflow.com/a/34279032

Which is definitely non-issue with tty drivers: they call
spin_lock_irqsave() with local flags and pass them to
uart_unlock_and_check_sysrq().

Looking into arch/sparc I also can't catch if it's still a limitation.

Also, looking around, xa_unlock_irqrestore() is called not from the same
function. Maybe this issue is in history?

Johan, is it a theoretical problem or something you observe?
Also, some comments would be nice near functions in the header.

Thanks,
Dmitry