Messages in this thread | | | Date | Tue, 27 Jun 2023 09:43:28 +0800 | From | Kent Gibson <> | Subject | Re: [PATCH] gpio: gpiolib-cdev: Fix potential &lr->wait.lock deadlock issue |
| |
On Mon, Jun 26, 2023 at 05:50:47PM +0200, Bartosz Golaszewski wrote: > On Mon, Jun 26, 2023 at 9:23 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > spin_lock_bh() should be sufficient, given that edge_irq_thread() is run > > in a softirq? That is faster and would allow the hard irq handlers to > > still run, and timestamp the event, but inhibit the edge_irq_thread() > > from being called on that CPU until the lock is released. > > (hmmm, gpio_desc_to_lineinfo() also uses spin_lock_irqsave() but it is > > never called from hard irq context, so there is a good chance I'm missing > > something here??) > > More on spin_lock choice below. > > Again: this is incorrect - edge_irq_thread() doesn't execute in > softirq context which can be verified by calling in_softirq() from it. >
Ok, that matches what I had initially thought. Wading through the kernel doc got me thinking the secondary handler was run as a softirq. But it is a threaded irq used here, so the thread handler runs in a kernel thread, as does the debounce_work_func() and hte thread handler process_hw_ts_thread(). That's a relief.
While we are on the subject of spin_locks, why does gpio_desc_to_lineinfo() use spin_lock_irqsave()? I assume the _irq is necessary as the desc could be updated at interrupt level, but AFAICT gpio_desc_to_lineinfo() is only ever called from process context, so why not just spin_lock_irq()?
Cheers, Kent.
| |