Messages in this thread | | | Subject | Re: [PATCH printk-rework 08/14] printk: add syslog_lock | From | Helge Deller <> | Date | Sun, 21 Feb 2021 22:39:42 +0100 |
| |
On 2/19/21 5:33 PM, John Ogness wrote: > Added CC: linux-parisc@vger.kernel.org > > On 2021-02-19, John Ogness <john.ogness@linutronix.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
| |