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?
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
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...
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
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.