linux-kernel - RE: [Linuxarm] Re: [PATCH for next v1 1/2] gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a61ef337fd1c4538a47fe855920f95d3@hisilicon.com>
Date:   Fri, 12 Feb 2021 05:05:51 +0000
From:   "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>
To:     Grygorii Strashko <grygorii.strashko@...com>,
        Arnd Bergmann <arnd@...nel.org>
CC:     luojiaxing <luojiaxing@...wei.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        "open list:GPIO SUBSYSTEM <linux-gpio@...r.kernel.org>,
        linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxarm@...neuler.org" <linuxarm@...neuler.org>
Subject: RE: [Linuxarm]  Re: [PATCH for next v1 1/2] gpio: omap: Replace
 raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler()



> -----Original Message-----
> From: Grygorii Strashko [mailto:grygorii.strashko@...com]
> Sent: Friday, February 12, 2021 9:17 AM
> To: Arnd Bergmann <arnd@...nel.org>
> Cc: luojiaxing <luojiaxing@...wei.com>; Linus Walleij
> <linus.walleij@...aro.org>; Andy Shevchenko <andy.shevchenko@...il.com>; Andy
> Shevchenko <andriy.shevchenko@...ux.intel.com>; Santosh Shilimkar
> <ssantosh@...nel.org>; Kevin Hilman <khilman@...nel.org>; open list:GPIO
> SUBSYSTEM <linux-gpio@...r.kernel.org>, linux-kernel@...r.kernel.org
> <linux-kernel@...r.kernel.org>; linuxarm@...neuler.org
> Subject: [Linuxarm] Re: [PATCH for next v1 1/2] gpio: omap: Replace
> raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler()
> 
> 
> 
> On 11/02/2021 21:39, Arnd Bergmann wrote:
> > On Thu, Feb 11, 2021 at 7:25 PM Grygorii Strashko
> > <grygorii.strashko@...com> wrote:
> >> On 08/02/2021 10:56, Luo Jiaxing wrote:
> >>> There is no need to use API with _irqsave in omap_gpio_irq_handler(),
> >>> because it already be in a irq-disabled context.
> >>
> >> NACK.
> >> Who said that this is always hard IRQ handler?
> >> What about RT-kernel or boot with "threadirqs"?
> >
> > In those cases, the interrupt handler is just a normal thread, so the
> > preempt_disable() that is implied by raw_spin_lock() is sufficient.
> >
> > Disabling interrupts inside of an interrupt handler is always incorrect,
> > the patch looks like a useful cleanup to me, if only for readability.
> 
> Note. there is also generic_handle_irq() call inside.

So generic_handle_irq() is not safe to run in thread thus requires
an interrupt-disabled environment to run? If so, I'd rather this
irqsave moved into generic_handle_irq() rather than asking everyone
calling it to do irqsave.

On the other hand, the author changed a couple of spin_lock_irqsave
to spin_lock, if only this one is incorrect, it seems it is worth a
new version to fix this.

> 
> --
> Best regards,
> grygorii

Thanks
Barry