MFD: mutex from ISR in ezx-pcap?

MFD: mutex from ISR in ezx-pcap?

[Jiri Slaby]

Hi,

pcap_adc_irq tries to perform non-atomic operations in ISR which is not
allowed. Above that, it may deadlock.

Since all the operations there (read, write, mutex locking) may sleep,
any objections to convert the ISR contents to a workqueue?

Re: MFD: mutex from ISR in ezx-pcap?

[Daniel Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

Hi Jiri,

Em Seg, 2009-06-29 às 21:34 +0200, Jiri Slaby escreveu:
> Hi,
> 
> pcap_adc_irq tries to perform non-atomic operations in ISR which is not
> allowed. Above that, it may deadlock.
> 
> Since all the operations there (read, write, mutex locking) may sleep,
> any objections to convert the ISR contents to a workqueue?

pcap_adc_irq actually already runs from a workqueue (pcap_isr_work).

-- 
Daniel Ribeiro

[-- Attachment #2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: MFD: mutex from ISR in ezx-pcap?

[Jiri Slaby]

On 06/29/2009 10:20 PM, Daniel Ribeiro wrote:
> Em Seg, 2009-06-29 às 21:34 +0200, Jiri Slaby escreveu:
>> pcap_adc_irq tries to perform non-atomic operations in ISR which is not
>> allowed. Above that, it may deadlock.
>>
>> Since all the operations there (read, write, mutex locking) may sleep,
>> any objections to convert the ISR contents to a workqueue?
> 
> pcap_adc_irq actually already runs from a workqueue (pcap_isr_work).

Aha, thanks, there is a specific irq handler. Ok, sounds fair.

But interrupts are disabled in pcap_isr_work anyway, so that
pcap_adc_irq shouldn't sleep, right? Or maybe I missed something again...

Re: MFD: mutex from ISR in ezx-pcap?

[Daniel Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

Em Ter, 2009-06-30 às 00:11 +0200, Jiri Slaby escreveu: 
> > pcap_adc_irq actually already runs from a workqueue (pcap_isr_work).
> 
> Aha, thanks, there is a specific irq handler. Ok, sounds fair.
> 
> But interrupts are disabled in pcap_isr_work anyway,

Right.

> so that
> pcap_adc_irq shouldn't sleep, right? Or maybe I missed something again...

No. Interrupts are enabled again before pcap_adc_irq() runs.

pcap_isr_work() doesn't call pcap_adc_irq directly, it calls the
irq_flow_handler, handle_simple_irq(), which needs to run with
interrupts disabled. Interrupts are enabled again on handle_IRQ_event(),
as pcap_adc_irq is requested _without_ IRQF_DISABLED.

-- 
Daniel Ribeiro

[-- Attachment #2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: MFD: mutex from ISR in ezx-pcap?

[Jiri Slaby]

On 06/30/2009 10:57 AM, Daniel Ribeiro wrote:
> Em Ter, 2009-06-30 às 00:11 +0200, Jiri Slaby escreveu: 
>> so that
>> pcap_adc_irq shouldn't sleep, right? Or maybe I missed something again...
> 
> No. Interrupts are enabled again before pcap_adc_irq() runs.
> 
> pcap_isr_work() doesn't call pcap_adc_irq directly, it calls the
> irq_flow_handler, handle_simple_irq(), which needs to run with
> interrupts disabled. Interrupts are enabled again on handle_IRQ_event(),
> as pcap_adc_irq is requested _without_ IRQF_DISABLED.

Correct. Thanks for clarification. I posted at least a lock fix which
made me poke in this all.