[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <db43de06-3183-7401-30f2-0e9302cc48e0@gmx.de>
Date: Sun, 21 Feb 2021 22:39:42 +0100
From: Helge Deller <deller@....de>
To: John Ogness <john.ogness@...utronix.de>,
Petr Mladek <pmladek@...e.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Steven Rostedt <rostedt@...dmis.org>,
Thomas Gleixner <tglx@...utronix.de>,
linux-kernel@...r.kernel.org, linux-parisc@...r.kernel.org
Subject: Re: [PATCH printk-rework 08/14] printk: add syslog_lock
On 2/19/21 5:33 PM, John Ogness wrote:
> Added CC: linux-parisc@...r.kernel.org
>
> On 2021-02-19, John Ogness <john.ogness@...utronix.de> wrote:
>>>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>>>> index 20c21a25143d..401df370832b 100644
>>>> --- a/kernel/printk/printk.c
>>>> +++ b/kernel/printk/printk.c
>>>> +/* Return a consistent copy of @syslog_seq. */
>>>> +static u64 read_syslog_seq_irq(void)
>>>> +{
>>>> + u64 seq;
>>>> +
>>>> + raw_spin_lock_irq(&syslog_lock);
>>>> + seq = syslog_seq;
>>>> + raw_spin_unlock_irq(&syslog_lock);
>>>
>>> Is there any particular reason to disable interrupts here?
>>>
>>> It would make sense only when the lock could be taken in IRQ
>>> context. Then we would need to always disable interrupts when
>>> the lock is taken. And if it is taken in IRQ context, we would
>>> need to safe flags.
>>
>> All other instances of locking @syslog_lock are done with interrupts
>> disabled. And we have:
>>
>> register_console()
>> logbuf_lock_irqsave()
>> raw_spin_lock(&syslog_lock)
>>
>> I suppose I need to go through all the console drivers to see if any
>> register in interrupt context. If not, that logbuf_lock_irqsave()
>> should be replaced with logbuf_lock_irq(). And then locking
>> @syslog_lock will not need to disable interrupts.
>
> I found a possible call chain in interrupt context. From arch/parisc
> there is the interrupt handler:
>
> handle_interruption(code=1) /* High-priority machine check (HPMC) */
> pdc_console_restart()
> pdc_console_init_force()
> register_console()
>
> All other register_console() calls in the kernel are either during init
> (within __init sections and probe functions) or are clearly not in
> interrupt context (using mutex, kzalloc, spin_lock_irq, etc).
>
> I am not familiar with parisc, but I am assuming handle_interruption()
> is always called with interrupts disabled (unless the HPMC interrupt is
> somehow an exception).
Yes, handle_interruption() is the irq handler, running with irqs off.
HPMC is the crash handler - it's called when the kernel will stop
anyway. pdc_console is a very basic firmware console which prints
the last messages before the machine halts on fatal errors.
So, this code it's not the typical use case....
Helge
Powered by blists - more mailing lists