RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Sat, 2012-10-13 at 14:21 +0900, anish kumar wrote: > On Fri, 2012-10-12 at 22:52 +0200, Thomas Gleixner wrote: > > On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > > -Original Message- > > > > From: anish kumar [mailto:anish198519851...@gmail.com] > > > > Sent: Friday, October 12, 2012 11:25 PM > > > > To: Liu, Chuansheng > > > > Cc: Thomas Gleixner; linux-kernel@vger.kernel.org > > > > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support > > > > with irq > > > > thread > > > > > > > > On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: > > > > > > On SMP an interrupt which is raised after the ack() again before the > > > > > > handler finishes, can invoke another delivery on a different CPU, > > > > > > which then sees the IRQ_INPROGESS flag, masks it and flags it > > > > > > PENDING. When the primary handler on the first CPU returns, it sees > > > > > > the PENDING flag, unmasks and invokes the handler another time. > > > > > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will > > > > > mask and ack it, if before the primary handler on the first CPU > > > > > returns, > > > > > the edge interrupt is raised again, it will be lost, right? > > > > Why will the interrupt be raised again?Is not it masked?I read tglx > > > I means because it is masked, if at this time device issues edge irq, > > > It will not be delivered and lost. > > > > No, it is NOT lost. The irq is marked PENDING already, so we invoke > It is fairly easy for an edge triggered interrupt to be missed - for > example if interrupts have to be masked for a period - and unless there > is some type of hardware latch that records the event it is impossible > to recover. > tglx, explanation will only work if we have a hardware latch which when > unmasked sends all those edge interrupts again (which had come when it > was masked while the CPU was handling the same interrupts). > > PS:http://kernel.org/doc/htmldocs/genericirq.html Hello tglx, Does this explanation makes sense? > > the handler again and handle it. And before we invoke the handler > > another time we unmask it. > > > > It does not matter at all whether the interrupt has been sent five > > times while it was masked. What matters is that we recorded the first > > one and set the PENDING flag. That way we invoke the interrupt handler > > again and keep stuff rolling. > > > > Thanks, > > > > tglx > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Sat, 2012-10-13 at 14:21 +0900, anish kumar wrote: On Fri, 2012-10-12 at 22:52 +0200, Thomas Gleixner wrote: On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: anish kumar [mailto:anish198519851...@gmail.com] Sent: Friday, October 12, 2012 11:25 PM To: Liu, Chuansheng Cc: Thomas Gleixner; linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? Why will the interrupt be raised again?Is not it masked?I read tglx I means because it is masked, if at this time device issues edge irq, It will not be delivered and lost. No, it is NOT lost. The irq is marked PENDING already, so we invoke It is fairly easy for an edge triggered interrupt to be missed - for example if interrupts have to be masked for a period - and unless there is some type of hardware latch that records the event it is impossible to recover. tglx, explanation will only work if we have a hardware latch which when unmasked sends all those edge interrupts again (which had come when it was masked while the CPU was handling the same interrupts). PS:http://kernel.org/doc/htmldocs/genericirq.html Hello tglx, Does this explanation makes sense? the handler again and handle it. And before we invoke the handler another time we unmask it. It does not matter at all whether the interrupt has been sent five times while it was masked. What matters is that we recorded the first one and set the PENDING flag. That way we invoke the interrupt handler again and keep stuff rolling. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 2012-10-12 at 22:52 +0200, Thomas Gleixner wrote: > On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > -Original Message- > > > From: anish kumar [mailto:anish198519851...@gmail.com] > > > Sent: Friday, October 12, 2012 11:25 PM > > > To: Liu, Chuansheng > > > Cc: Thomas Gleixner; linux-kernel@vger.kernel.org > > > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with > > > irq > > > thread > > > > > > On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: > > > > > On SMP an interrupt which is raised after the ack() again before the > > > > > handler finishes, can invoke another delivery on a different CPU, > > > > > which then sees the IRQ_INPROGESS flag, masks it and flags it > > > > > PENDING. When the primary handler on the first CPU returns, it sees > > > > > the PENDING flag, unmasks and invokes the handler another time. > > > > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will > > > > mask and ack it, if before the primary handler on the first CPU returns, > > > > the edge interrupt is raised again, it will be lost, right? > > > Why will the interrupt be raised again?Is not it masked?I read tglx > > I means because it is masked, if at this time device issues edge irq, > > It will not be delivered and lost. > > No, it is NOT lost. The irq is marked PENDING already, so we invoke It is fairly easy for an edge triggered interrupt to be missed - for example if interrupts have to be masked for a period - and unless there is some type of hardware latch that records the event it is impossible to recover. tglx, explanation will only work if we have a hardware latch which when unmasked sends all those edge interrupts again (which had come when it was masked while the CPU was handling the same interrupts). PS:http://kernel.org/doc/htmldocs/genericirq.html > the handler again and handle it. And before we invoke the handler > another time we unmask it. > > It does not matter at all whether the interrupt has been sent five > times while it was masked. What matters is that we recorded the first > one and set the PENDING flag. That way we invoke the interrupt handler > again and keep stuff rolling. > > Thanks, > > tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > But IRQS_ONESHOT does not work well for edge interrupt. > > > And pasting the IRQS_ONESHOT description: > > > * IRQS_ONESHOT- irq is not unmasked in primary handler > > > > Right, and edge type interrupts doe not support it. > > Can we do something? Thanks your sharing. No, we cannot do anything. The edge handler is not going to change. End of story. > In request_thread_irq() case with FLAG IRQS_ONESHOT, for edge interrupt, > in function irq_finalize_oneshot(): > if (!desc->threads_oneshot && !irqd_irq_disabled(>irq_data) && > irqd_irq_masked(>irq_data)) > unmask_irq(desc); > > It is possible unmask_irq() is called, but the below code is just > aiming for masking action in irq handler, so I guess if I called the > mask_irq() in non-core code, when irq_finalize_oneshot is called, > the unmask_irq is called, and it is not we wanted, right? Do not > test this case:) You're guessing wrong again. Non core code CANNOT call mask_irq() except via irq_disable(). No have a look at irq_disable() and then read the above condition again. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > On SMP an interrupt which is raised after the ack() again before the > > handler finishes, can invoke another delivery on a different CPU, > > which then sees the IRQ_INPROGESS flag, masks it and flags it > > PENDING. When the primary handler on the first CPU returns, it sees > > the PENDING flag, unmasks and invokes the handler another time. > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will > mask and ack it, if before the primary handler on the first CPU returns, > the edge interrupt is raised again, it will be lost, right? > So I think set PENDING just confirm one time, it just depends on primary > handler > execution time and irq frequency. It does NOT matter. We marked it PENDING. And that guarantees that we invoke the handler again after unmasking. And that keeps stuff rolling. We would lose when we masked and did not notice that there was one, but we notice and store that information with the PENDING flag. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > -Original Message- > > From: anish kumar [mailto:anish198519851...@gmail.com] > > Sent: Friday, October 12, 2012 11:25 PM > > To: Liu, Chuansheng > > Cc: Thomas Gleixner; linux-kernel@vger.kernel.org > > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with > > irq > > thread > > > > On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: > > > > On SMP an interrupt which is raised after the ack() again before the > > > > handler finishes, can invoke another delivery on a different CPU, > > > > which then sees the IRQ_INPROGESS flag, masks it and flags it > > > > PENDING. When the primary handler on the first CPU returns, it sees > > > > the PENDING flag, unmasks and invokes the handler another time. > > > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will > > > mask and ack it, if before the primary handler on the first CPU returns, > > > the edge interrupt is raised again, it will be lost, right? > > Why will the interrupt be raised again?Is not it masked?I read tglx > I means because it is masked, if at this time device issues edge irq, > It will not be delivered and lost. No, it is NOT lost. The irq is marked PENDING already, so we invoke the handler again and handle it. And before we invoke the handler another time we unmask it. It does not matter at all whether the interrupt has been sent five times while it was masked. What matters is that we recorded the first one and set the PENDING flag. That way we invoke the interrupt handler again and keep stuff rolling. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 2012-10-12 at 15:29 +, Liu, Chuansheng wrote: > > > -Original Message- > > From: anish kumar [mailto:anish198519851...@gmail.com] > > Sent: Friday, October 12, 2012 11:25 PM > > To: Liu, Chuansheng > > Cc: Thomas Gleixner; linux-kernel@vger.kernel.org > > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with > > irq > > thread > > > > On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: > > > > On SMP an interrupt which is raised after the ack() again before the > > > > handler finishes, can invoke another delivery on a different CPU, > > > > which then sees the IRQ_INPROGESS flag, masks it and flags it > > > > PENDING. When the primary handler on the first CPU returns, it sees > > > > the PENDING flag, unmasks and invokes the handler another time. > > > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will > > > mask and ack it, if before the primary handler on the first CPU returns, > > > the edge interrupt is raised again, it will be lost, right? > > Why will the interrupt be raised again?Is not it masked?I read tglx > I means because it is masked, if at this time device issues edge irq, > It will not be delivered and lost. That depends on the hardware, doesn't it?If if the frequency of interrupt is too much that device is not able to cope up with that then don't you think that the interrupt controller is faulty? Well I am not a expert on the hardware but it looks to me that the hardware should be at a fault here. > > > statement as this:if the interrupt is being handled on one core, then > > the delivery of new interrupt can be on the second core and in that case > > it will see IRQ_INPROGRESS flag and it will *mask* it and set the flag > > as pending.So there is no chance of any new interrupt. > > > So I think set PENDING just confirm one time, it just depends on primary > > handler > > > execution time and irq frequency. > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > > the body of a message to majord...@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > Please read the FAQ at http://www.tux.org/lkml/ > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
> -Original Message-
> From: anish kumar [mailto:anish198519851...@gmail.com]
> Sent: Friday, October 12, 2012 11:25 PM
> To: Liu, Chuansheng
> Cc: Thomas Gleixner; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq
> thread
>
> On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote:
> > > On SMP an interrupt which is raised after the ack() again before the
> > > handler finishes, can invoke another delivery on a different CPU,
> > > which then sees the IRQ_INPROGESS flag, masks it and flags it
> > > PENDING. When the primary handler on the first CPU returns, it sees
> > > the PENDING flag, unmasks and invokes the handler another time.
> > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will
> > mask and ack it, if before the primary handler on the first CPU returns,
> > the edge interrupt is raised again, it will be lost, right?
> Why will the interrupt be raised again?Is not it masked?I read tglx
I means because it is masked, if at this time device issues edge irq,
It will not be delivered and lost.
> statement as this:if the interrupt is being handled on one core, then
> the delivery of new interrupt can be on the second core and in that case
> it will see IRQ_INPROGRESS flag and it will *mask* it and set the flag
> as pending.So there is no chance of any new interrupt.
> > So I think set PENDING just confirm one time, it just depends on primary
> handler
> > execution time and irq frequency.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: > > On SMP an interrupt which is raised after the ack() again before the > > handler finishes, can invoke another delivery on a different CPU, > > which then sees the IRQ_INPROGESS flag, masks it and flags it > > PENDING. When the primary handler on the first CPU returns, it sees > > the PENDING flag, unmasks and invokes the handler another time. > In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will > mask and ack it, if before the primary handler on the first CPU returns, > the edge interrupt is raised again, it will be lost, right? Why will the interrupt be raised again?Is not it masked?I read tglx statement as this:if the interrupt is being handled on one core, then the delivery of new interrupt can be on the second core and in that case it will see IRQ_INPROGRESS flag and it will *mask* it and set the flag as pending.So there is no chance of any new interrupt. > So I think set PENDING just confirm one time, it just depends on primary > handler > execution time and irq frequency. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
> On SMP an interrupt which is raised after the ack() again before the > handler finishes, can invoke another delivery on a different CPU, > which then sees the IRQ_INPROGESS flag, masks it and flags it > PENDING. When the primary handler on the first CPU returns, it sees > the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? So I think set PENDING just confirm one time, it just depends on primary handler execution time and irq frequency. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
Thanks your beautiful explain for edge interrupt handler, still has one confusing for unmask_irq in irq_finalize_oneshot(). Could you see below comments? Thanks. > -Original Message- > From: Thomas Gleixner [mailto:t...@linutronix.de] > Sent: Friday, October 12, 2012 9:48 PM > To: Liu, Chuansheng > Cc: linux-kernel@vger.kernel.org > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq > thread > > On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > -Original Message- > > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > > Sent: Friday, October 12, 2012 8:46 PM > > > To: Liu, Chuansheng > > > Cc: linux-kernel@vger.kernel.org > > > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support > with irq > > > thread > > > > > > On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > > > -Original Message- > > > > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > > > > Sent: Friday, October 12, 2012 8:32 PM > > > > > To: Liu, Chuansheng > > > > > Cc: linux-kernel@vger.kernel.org > > > > > Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support > > > with irq > > > > > thread > > > > > > > > > > On Fri, 12 Oct 2012, Chuansheng Liu wrote: > > > > > > > > > > > > In our system, there is one edge interrupt, and we want it to be > > > > > > irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), > > > > > > even with IRQS_ONESHOT, the irq is still unmasked without care of > > > > > > flag IRQS_ONESHOT. > > > > > > > > > > > > It causes IRQS_ONESHOT can not work well for edge interrupt, but > also > > > > > > after the irq thread finished with flag IRQS_ONESHOT, the irq will > > > > > > be > > > > > > possible to be unmasked again, it should be messing mask/unmask > logic. > > > > > > > > > > This is just wrong. By masking edge interrupts you will run into > > > > > situations where you will lose interrupts. > > > > > > > > > > Can you please explain, why you want to mask your edge interrupt? > > > > > > > When I request_irq with irq thread handler and flag IRQS_ONESHOT, if > > > > do not mask the edge interrupt, the primary handler and irq thread > > > > maybe run at the same time, and in my real case it causes spin > > > > deadlock. > > > > > > Then your code is simply wrong and you need to fix it instead of hacking a > > > workaround into the core code. Locking is not that hard. > > > > I got means, so I want to use flag IRQS_ONESHOT to avoid the case that two > > handlers running at the same time. Is it right direction? > > Again, this does not work for edge type interrupts. Period. And we are > not adding something which is known to be broken. > > Also there is no problem when a hard interrupt comes in while the > thread handler is running. It's just a matter of proper code and > proper locking. > > > But IRQS_ONESHOT does not work well for edge interrupt. > > And pasting the IRQS_ONESHOT description: > > * IRQS_ONESHOT - irq is not unmasked in primary handler > > Right, and edge type interrupts doe not support it. Can we do something? Thanks your sharing. In request_thread_irq() case with FLAG IRQS_ONESHOT, for edge interrupt, in function irq_finalize_oneshot(): if (!desc->threads_oneshot && !irqd_irq_disabled(>irq_data) && irqd_irq_masked(>irq_data)) unmask_irq(desc); It is possible unmask_irq() is called, but the below code is just aiming for masking action in irq handler, so I guess if I called the mask_irq() in non-core code, when irq_finalize_oneshot is called, the unmask_irq is called, and it is not we wanted, right? Do not test this case:) > > > And I need irq handler is because some heavy work is needed, it can avoid > local > > irq disabling time thru irq handler. > > Disable the interrupt at the device level in your primary handler, but > do not try to impose something to the core code which is fundamentaly > wrong. > > > > > > > > You means it is not right with IRQS_ONESHOT for edge interrupt? > > > > > > It's wrong. Simnply because you can lose interrupts. > > > > > > interrupt raised > > >handle_edge_irq() > > > mask_ack_irq() > >
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > -Original Message- > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > Sent: Friday, October 12, 2012 8:46 PM > > To: Liu, Chuansheng > > Cc: linux-kernel@vger.kernel.org > > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with > > irq > > thread > > > > On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > > -Original Message- > > > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > > > Sent: Friday, October 12, 2012 8:32 PM > > > > To: Liu, Chuansheng > > > > Cc: linux-kernel@vger.kernel.org > > > > Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support > > with irq > > > > thread > > > > > > > > On Fri, 12 Oct 2012, Chuansheng Liu wrote: > > > > > > > > > > In our system, there is one edge interrupt, and we want it to be > > > > > irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), > > > > > even with IRQS_ONESHOT, the irq is still unmasked without care of > > > > > flag IRQS_ONESHOT. > > > > > > > > > > It causes IRQS_ONESHOT can not work well for edge interrupt, but also > > > > > after the irq thread finished with flag IRQS_ONESHOT, the irq will be > > > > > possible to be unmasked again, it should be messing mask/unmask logic. > > > > > > > > This is just wrong. By masking edge interrupts you will run into > > > > situations where you will lose interrupts. > > > > > > > > Can you please explain, why you want to mask your edge interrupt? > > > > > When I request_irq with irq thread handler and flag IRQS_ONESHOT, if > > > do not mask the edge interrupt, the primary handler and irq thread > > > maybe run at the same time, and in my real case it causes spin > > > deadlock. > > > > Then your code is simply wrong and you need to fix it instead of hacking a > > workaround into the core code. Locking is not that hard. > > I got means, so I want to use flag IRQS_ONESHOT to avoid the case that two > handlers running at the same time. Is it right direction? Again, this does not work for edge type interrupts. Period. And we are not adding something which is known to be broken. Also there is no problem when a hard interrupt comes in while the thread handler is running. It's just a matter of proper code and proper locking. > But IRQS_ONESHOT does not work well for edge interrupt. > And pasting the IRQS_ONESHOT description: > * IRQS_ONESHOT- irq is not unmasked in primary handler Right, and edge type interrupts doe not support it. > And I need irq handler is because some heavy work is needed, it can avoid > local > irq disabling time thru irq handler. Disable the interrupt at the device level in your primary handler, but do not try to impose something to the core code which is fundamentaly wrong. > > > > > You means it is not right with IRQS_ONESHOT for edge interrupt? > > > > It's wrong. Simnply because you can lose interrupts. > > > > interrupt raised > > handle_edge_irq() > > mask_ack_irq() > > handle_event() > > wake irq thread > > reti > > > > irq thread runs > > handle device interrupt() > ><--- device issues edge irq > > unmask_irq() > > > > This interrupt is not delivered. So your device stops working. Not > > what you want, right? > Device should not stop:) And even in current handle_edge_irq(), it > is possible that losting Interrupt if primary handler need some time > and the irq is quick enough. I says the below code, it just avoid The code flow is: Interrupt ack() handler() RETI After the ack another interrupt can come in. It's raised in the CPU, but it cannot be delivered because the CPU is running that very interrupt at this point with interupts disabled. After RETI this interrupt is delivered and runs the edge handler again. That's on UP. On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. So nothing gets lost. Now you mask it and if you look at the flow I showed in my last mail, then your device will be stuck. Simply because the interrupt was delivered while the line was masked at the irq chip level which causes a drop. That's a property of edge type interrupts and we have proper code to deal with it. No way to change that just that you can avoid to fix your broken driver design. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
FLAG IRQS_ONESHOT does not work well for edge interrupt. * IRQS_ONESHOT - irq is not unmasked in primary handler Is it possible give some notification or remind? Thanks. I took some time to know it:) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
> -Original Message- > From: Thomas Gleixner [mailto:t...@linutronix.de] > Sent: Friday, October 12, 2012 8:46 PM > To: Liu, Chuansheng > Cc: linux-kernel@vger.kernel.org > Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq > thread > > On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > > -Original Message- > > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > > Sent: Friday, October 12, 2012 8:32 PM > > > To: Liu, Chuansheng > > > Cc: linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support > with irq > > > thread > > > > > > On Fri, 12 Oct 2012, Chuansheng Liu wrote: > > > > > > > > In our system, there is one edge interrupt, and we want it to be > > > > irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), > > > > even with IRQS_ONESHOT, the irq is still unmasked without care of > > > > flag IRQS_ONESHOT. > > > > > > > > It causes IRQS_ONESHOT can not work well for edge interrupt, but also > > > > after the irq thread finished with flag IRQS_ONESHOT, the irq will be > > > > possible to be unmasked again, it should be messing mask/unmask logic. > > > > > > This is just wrong. By masking edge interrupts you will run into > > > situations where you will lose interrupts. > > > > > > Can you please explain, why you want to mask your edge interrupt? > > > When I request_irq with irq thread handler and flag IRQS_ONESHOT, if > > do not mask the edge interrupt, the primary handler and irq thread > > maybe run at the same time, and in my real case it causes spin > > deadlock. > > Then your code is simply wrong and you need to fix it instead of hacking a > workaround into the core code. Locking is not that hard. I got means, so I want to use flag IRQS_ONESHOT to avoid the case that two handlers running at the same time. Is it right direction? But IRQS_ONESHOT does not work well for edge interrupt. And pasting the IRQS_ONESHOT description: * IRQS_ONESHOT - irq is not unmasked in primary handler And I need irq handler is because some heavy work is needed, it can avoid local irq disabling time thru irq handler. > > > You means it is not right with IRQS_ONESHOT for edge interrupt? > > It's wrong. Simnply because you can lose interrupts. > > interrupt raised >handle_edge_irq() > mask_ack_irq() > handle_event() > wake irq thread > reti > > irq thread runs >handle device interrupt() > <--- device issues edge irq >unmask_irq() > > This interrupt is not delivered. So your device stops working. Not > what you want, right? Device should not stop:) And even in current handle_edge_irq(), it is possible that losting Interrupt if primary handler need some time and the irq is quick enough. I says the below code, it just avoid one time lost. desc->istate |= IRQS_PENDING; mask_ack_irq(desc); > > Thanks, > > tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: > > -Original Message- > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > Sent: Friday, October 12, 2012 8:32 PM > > To: Liu, Chuansheng > > Cc: linux-kernel@vger.kernel.org > > Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with > > irq > > thread > > > > On Fri, 12 Oct 2012, Chuansheng Liu wrote: > > > > > > In our system, there is one edge interrupt, and we want it to be > > > irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), > > > even with IRQS_ONESHOT, the irq is still unmasked without care of > > > flag IRQS_ONESHOT. > > > > > > It causes IRQS_ONESHOT can not work well for edge interrupt, but also > > > after the irq thread finished with flag IRQS_ONESHOT, the irq will be > > > possible to be unmasked again, it should be messing mask/unmask logic. > > > > This is just wrong. By masking edge interrupts you will run into > > situations where you will lose interrupts. > > > > Can you please explain, why you want to mask your edge interrupt? > When I request_irq with irq thread handler and flag IRQS_ONESHOT, if > do not mask the edge interrupt, the primary handler and irq thread > maybe run at the same time, and in my real case it causes spin > deadlock. Then your code is simply wrong and you need to fix it instead of hacking a workaround into the core code. Locking is not that hard. > You means it is not right with IRQS_ONESHOT for edge interrupt? It's wrong. Simnply because you can lose interrupts. interrupt raised handle_edge_irq() mask_ack_irq() handle_event() wake irq thread reti irq thread runs handle device interrupt() <--- device issues edge irq unmask_irq() This interrupt is not delivered. So your device stops working. Not what you want, right? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
> -Original Message-
> From: Thomas Gleixner [mailto:t...@linutronix.de]
> Sent: Friday, October 12, 2012 8:32 PM
> To: Liu, Chuansheng
> Cc: linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq
> thread
>
> On Fri, 12 Oct 2012, Chuansheng Liu wrote:
> >
> > In our system, there is one edge interrupt, and we want it to be
> > irq thread with IRQS_ONESHOT, and found in handle_edge_irq(),
> > even with IRQS_ONESHOT, the irq is still unmasked without care of
> > flag IRQS_ONESHOT.
> >
> > It causes IRQS_ONESHOT can not work well for edge interrupt, but also
> > after the irq thread finished with flag IRQS_ONESHOT, the irq will be
> > possible to be unmasked again, it should be messing mask/unmask logic.
>
> This is just wrong. By masking edge interrupts you will run into
> situations where you will lose interrupts.
>
> Can you please explain, why you want to mask your edge interrupt?
When I request_irq with irq thread handler and flag IRQS_ONESHOT, if do not
mask the edge interrupt,
the primary handler and irq thread maybe run at the same time, and in my real
case it causes spin deadlock.
You means it is not right with IRQS_ONESHOT for edge interrupt?
>
> Thanks,
>
> tglx
>
>
> > Signed-off-by: liu chuansheng
> > ---
> > kernel/irq/chip.c |8 +++-
> > 1 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index 57d86d0..f23f524 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -497,7 +497,13 @@ handle_edge_irq(unsigned int irq, struct irq_desc
> *desc)
> > kstat_incr_irqs_this_cpu(irq, desc);
> >
> > /* Start handling the irq */
> > - desc->irq_data.chip->irq_ack(>irq_data);
> > + if (desc->istate & IRQS_ONESHOT) {
> > + mask_ack_irq(desc);
> > + handle_irq_event(desc);
> > + cond_unmask_irq(desc);
> > + goto out_unlock;
> > + } else
> > + desc->irq_data.chip->irq_ack(>irq_data);
> >
> > do {
> > if (unlikely(!desc->action)) {
> > --
> > 1.7.0.4
> >
> >
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Chuansheng Liu wrote:
>
> In our system, there is one edge interrupt, and we want it to be
> irq thread with IRQS_ONESHOT, and found in handle_edge_irq(),
> even with IRQS_ONESHOT, the irq is still unmasked without care of
> flag IRQS_ONESHOT.
>
> It causes IRQS_ONESHOT can not work well for edge interrupt, but also
> after the irq thread finished with flag IRQS_ONESHOT, the irq will be
> possible to be unmasked again, it should be messing mask/unmask logic.
This is just wrong. By masking edge interrupts you will run into
situations where you will lose interrupts.
Can you please explain, why you want to mask your edge interrupt?
Thanks,
tglx
> Signed-off-by: liu chuansheng
> ---
> kernel/irq/chip.c |8 +++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 57d86d0..f23f524 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -497,7 +497,13 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)
> kstat_incr_irqs_this_cpu(irq, desc);
>
> /* Start handling the irq */
> - desc->irq_data.chip->irq_ack(>irq_data);
> + if (desc->istate & IRQS_ONESHOT) {
> + mask_ack_irq(desc);
> + handle_irq_event(desc);
> + cond_unmask_irq(desc);
> + goto out_unlock;
> + } else
> + desc->irq_data.chip->irq_ack(>irq_data);
>
> do {
> if (unlikely(!desc->action)) {
> --
> 1.7.0.4
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Chuansheng Liu wrote:
In our system, there is one edge interrupt, and we want it to be
irq thread with IRQS_ONESHOT, and found in handle_edge_irq(),
even with IRQS_ONESHOT, the irq is still unmasked without care of
flag IRQS_ONESHOT.
It causes IRQS_ONESHOT can not work well for edge interrupt, but also
after the irq thread finished with flag IRQS_ONESHOT, the irq will be
possible to be unmasked again, it should be messing mask/unmask logic.
This is just wrong. By masking edge interrupts you will run into
situations where you will lose interrupts.
Can you please explain, why you want to mask your edge interrupt?
Thanks,
tglx
Signed-off-by: liu chuansheng chuansheng@intel.com
---
kernel/irq/chip.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 57d86d0..f23f524 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -497,7 +497,13 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)
kstat_incr_irqs_this_cpu(irq, desc);
/* Start handling the irq */
- desc-irq_data.chip-irq_ack(desc-irq_data);
+ if (desc-istate IRQS_ONESHOT) {
+ mask_ack_irq(desc);
+ handle_irq_event(desc);
+ cond_unmask_irq(desc);
+ goto out_unlock;
+ } else
+ desc-irq_data.chip-irq_ack(desc-irq_data);
do {
if (unlikely(!desc-action)) {
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
-Original Message-
From: Thomas Gleixner [mailto:t...@linutronix.de]
Sent: Friday, October 12, 2012 8:32 PM
To: Liu, Chuansheng
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq
thread
On Fri, 12 Oct 2012, Chuansheng Liu wrote:
In our system, there is one edge interrupt, and we want it to be
irq thread with IRQS_ONESHOT, and found in handle_edge_irq(),
even with IRQS_ONESHOT, the irq is still unmasked without care of
flag IRQS_ONESHOT.
It causes IRQS_ONESHOT can not work well for edge interrupt, but also
after the irq thread finished with flag IRQS_ONESHOT, the irq will be
possible to be unmasked again, it should be messing mask/unmask logic.
This is just wrong. By masking edge interrupts you will run into
situations where you will lose interrupts.
Can you please explain, why you want to mask your edge interrupt?
When I request_irq with irq thread handler and flag IRQS_ONESHOT, if do not
mask the edge interrupt,
the primary handler and irq thread maybe run at the same time, and in my real
case it causes spin deadlock.
You means it is not right with IRQS_ONESHOT for edge interrupt?
Thanks,
tglx
Signed-off-by: liu chuansheng chuansheng@intel.com
---
kernel/irq/chip.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 57d86d0..f23f524 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -497,7 +497,13 @@ handle_edge_irq(unsigned int irq, struct irq_desc
*desc)
kstat_incr_irqs_this_cpu(irq, desc);
/* Start handling the irq */
- desc-irq_data.chip-irq_ack(desc-irq_data);
+ if (desc-istate IRQS_ONESHOT) {
+ mask_ack_irq(desc);
+ handle_irq_event(desc);
+ cond_unmask_irq(desc);
+ goto out_unlock;
+ } else
+ desc-irq_data.chip-irq_ack(desc-irq_data);
do {
if (unlikely(!desc-action)) {
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:32 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Chuansheng Liu wrote: In our system, there is one edge interrupt, and we want it to be irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), even with IRQS_ONESHOT, the irq is still unmasked without care of flag IRQS_ONESHOT. It causes IRQS_ONESHOT can not work well for edge interrupt, but also after the irq thread finished with flag IRQS_ONESHOT, the irq will be possible to be unmasked again, it should be messing mask/unmask logic. This is just wrong. By masking edge interrupts you will run into situations where you will lose interrupts. Can you please explain, why you want to mask your edge interrupt? When I request_irq with irq thread handler and flag IRQS_ONESHOT, if do not mask the edge interrupt, the primary handler and irq thread maybe run at the same time, and in my real case it causes spin deadlock. Then your code is simply wrong and you need to fix it instead of hacking a workaround into the core code. Locking is not that hard. You means it is not right with IRQS_ONESHOT for edge interrupt? It's wrong. Simnply because you can lose interrupts. interrupt raised handle_edge_irq() mask_ack_irq() handle_event() wake irq thread reti irq thread runs handle device interrupt() --- device issues edge irq unmask_irq() This interrupt is not delivered. So your device stops working. Not what you want, right? Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
-Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:46 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:32 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Chuansheng Liu wrote: In our system, there is one edge interrupt, and we want it to be irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), even with IRQS_ONESHOT, the irq is still unmasked without care of flag IRQS_ONESHOT. It causes IRQS_ONESHOT can not work well for edge interrupt, but also after the irq thread finished with flag IRQS_ONESHOT, the irq will be possible to be unmasked again, it should be messing mask/unmask logic. This is just wrong. By masking edge interrupts you will run into situations where you will lose interrupts. Can you please explain, why you want to mask your edge interrupt? When I request_irq with irq thread handler and flag IRQS_ONESHOT, if do not mask the edge interrupt, the primary handler and irq thread maybe run at the same time, and in my real case it causes spin deadlock. Then your code is simply wrong and you need to fix it instead of hacking a workaround into the core code. Locking is not that hard. I got means, so I want to use flag IRQS_ONESHOT to avoid the case that two handlers running at the same time. Is it right direction? But IRQS_ONESHOT does not work well for edge interrupt. And pasting the IRQS_ONESHOT description: * IRQS_ONESHOT - irq is not unmasked in primary handler And I need irq handler is because some heavy work is needed, it can avoid local irq disabling time thru irq handler. You means it is not right with IRQS_ONESHOT for edge interrupt? It's wrong. Simnply because you can lose interrupts. interrupt raised handle_edge_irq() mask_ack_irq() handle_event() wake irq thread reti irq thread runs handle device interrupt() --- device issues edge irq unmask_irq() This interrupt is not delivered. So your device stops working. Not what you want, right? Device should not stop:) And even in current handle_edge_irq(), it is possible that losting Interrupt if primary handler need some time and the irq is quick enough. I says the below code, it just avoid one time lost. desc-istate |= IRQS_PENDING; mask_ack_irq(desc); Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
FLAG IRQS_ONESHOT does not work well for edge interrupt. * IRQS_ONESHOT - irq is not unmasked in primary handler Is it possible give some notification or remind? Thanks. I took some time to know it:) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:46 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:32 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Chuansheng Liu wrote: In our system, there is one edge interrupt, and we want it to be irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), even with IRQS_ONESHOT, the irq is still unmasked without care of flag IRQS_ONESHOT. It causes IRQS_ONESHOT can not work well for edge interrupt, but also after the irq thread finished with flag IRQS_ONESHOT, the irq will be possible to be unmasked again, it should be messing mask/unmask logic. This is just wrong. By masking edge interrupts you will run into situations where you will lose interrupts. Can you please explain, why you want to mask your edge interrupt? When I request_irq with irq thread handler and flag IRQS_ONESHOT, if do not mask the edge interrupt, the primary handler and irq thread maybe run at the same time, and in my real case it causes spin deadlock. Then your code is simply wrong and you need to fix it instead of hacking a workaround into the core code. Locking is not that hard. I got means, so I want to use flag IRQS_ONESHOT to avoid the case that two handlers running at the same time. Is it right direction? Again, this does not work for edge type interrupts. Period. And we are not adding something which is known to be broken. Also there is no problem when a hard interrupt comes in while the thread handler is running. It's just a matter of proper code and proper locking. But IRQS_ONESHOT does not work well for edge interrupt. And pasting the IRQS_ONESHOT description: * IRQS_ONESHOT- irq is not unmasked in primary handler Right, and edge type interrupts doe not support it. And I need irq handler is because some heavy work is needed, it can avoid local irq disabling time thru irq handler. Disable the interrupt at the device level in your primary handler, but do not try to impose something to the core code which is fundamentaly wrong. You means it is not right with IRQS_ONESHOT for edge interrupt? It's wrong. Simnply because you can lose interrupts. interrupt raised handle_edge_irq() mask_ack_irq() handle_event() wake irq thread reti irq thread runs handle device interrupt() --- device issues edge irq unmask_irq() This interrupt is not delivered. So your device stops working. Not what you want, right? Device should not stop:) And even in current handle_edge_irq(), it is possible that losting Interrupt if primary handler need some time and the irq is quick enough. I says the below code, it just avoid The code flow is: Interrupt ack() handler() RETI After the ack another interrupt can come in. It's raised in the CPU, but it cannot be delivered because the CPU is running that very interrupt at this point with interupts disabled. After RETI this interrupt is delivered and runs the edge handler again. That's on UP. On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. So nothing gets lost. Now you mask it and if you look at the flow I showed in my last mail, then your device will be stuck. Simply because the interrupt was delivered while the line was masked at the irq chip level which causes a drop. That's a property of edge type interrupts and we have proper code to deal with it. No way to change that just that you can avoid to fix your broken driver design. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
Thanks your beautiful explain for edge interrupt handler, still has one confusing for unmask_irq in irq_finalize_oneshot(). Could you see below comments? Thanks. -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 9:48 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:46 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Friday, October 12, 2012 8:32 PM To: Liu, Chuansheng Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 12 Oct 2012, Chuansheng Liu wrote: In our system, there is one edge interrupt, and we want it to be irq thread with IRQS_ONESHOT, and found in handle_edge_irq(), even with IRQS_ONESHOT, the irq is still unmasked without care of flag IRQS_ONESHOT. It causes IRQS_ONESHOT can not work well for edge interrupt, but also after the irq thread finished with flag IRQS_ONESHOT, the irq will be possible to be unmasked again, it should be messing mask/unmask logic. This is just wrong. By masking edge interrupts you will run into situations where you will lose interrupts. Can you please explain, why you want to mask your edge interrupt? When I request_irq with irq thread handler and flag IRQS_ONESHOT, if do not mask the edge interrupt, the primary handler and irq thread maybe run at the same time, and in my real case it causes spin deadlock. Then your code is simply wrong and you need to fix it instead of hacking a workaround into the core code. Locking is not that hard. I got means, so I want to use flag IRQS_ONESHOT to avoid the case that two handlers running at the same time. Is it right direction? Again, this does not work for edge type interrupts. Period. And we are not adding something which is known to be broken. Also there is no problem when a hard interrupt comes in while the thread handler is running. It's just a matter of proper code and proper locking. But IRQS_ONESHOT does not work well for edge interrupt. And pasting the IRQS_ONESHOT description: * IRQS_ONESHOT - irq is not unmasked in primary handler Right, and edge type interrupts doe not support it. Can we do something? Thanks your sharing. In request_thread_irq() case with FLAG IRQS_ONESHOT, for edge interrupt, in function irq_finalize_oneshot(): if (!desc-threads_oneshot !irqd_irq_disabled(desc-irq_data) irqd_irq_masked(desc-irq_data)) unmask_irq(desc); It is possible unmask_irq() is called, but the below code is just aiming for masking action in irq handler, so I guess if I called the mask_irq() in non-core code, when irq_finalize_oneshot is called, the unmask_irq is called, and it is not we wanted, right? Do not test this case:) And I need irq handler is because some heavy work is needed, it can avoid local irq disabling time thru irq handler. Disable the interrupt at the device level in your primary handler, but do not try to impose something to the core code which is fundamentaly wrong. You means it is not right with IRQS_ONESHOT for edge interrupt? It's wrong. Simnply because you can lose interrupts. interrupt raised handle_edge_irq() mask_ack_irq() handle_event() wake irq thread reti irq thread runs handle device interrupt() --- device issues edge irq unmask_irq() This interrupt is not delivered. So your device stops working. Not what you want, right? Device should not stop:) And even in current handle_edge_irq(), it is possible that losting Interrupt if primary handler need some time and the irq is quick enough. I says the below code, it just avoid The code flow is: Interrupt ack() handler() RETI After the ack another interrupt can come in. It's raised in the CPU, but it cannot be delivered because the CPU is running that very interrupt at this point with interupts disabled. After RETI this interrupt is delivered and runs the edge handler again. That's on UP. On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? So I think set PENDING just confirm one time, it just depends on primary handler execution time and irq frequency. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? Why will the interrupt be raised again?Is not it masked?I read tglx statement as this:if the interrupt is being handled on one core, then the delivery of new interrupt can be on the second core and in that case it will see IRQ_INPROGRESS flag and it will *mask* it and set the flag as pending.So there is no chance of any new interrupt. So I think set PENDING just confirm one time, it just depends on primary handler execution time and irq frequency. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
-Original Message-
From: anish kumar [mailto:anish198519851...@gmail.com]
Sent: Friday, October 12, 2012 11:25 PM
To: Liu, Chuansheng
Cc: Thomas Gleixner; linux-kernel@vger.kernel.org
Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq
thread
On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote:
On SMP an interrupt which is raised after the ack() again before the
handler finishes, can invoke another delivery on a different CPU,
which then sees the IRQ_INPROGESS flag, masks it and flags it
PENDING. When the primary handler on the first CPU returns, it sees
the PENDING flag, unmasks and invokes the handler another time.
In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will
mask and ack it, if before the primary handler on the first CPU returns,
the edge interrupt is raised again, it will be lost, right?
Why will the interrupt be raised again?Is not it masked?I read tglx
I means because it is masked, if at this time device issues edge irq,
It will not be delivered and lost.
statement as this:if the interrupt is being handled on one core, then
the delivery of new interrupt can be on the second core and in that case
it will see IRQ_INPROGRESS flag and it will *mask* it and set the flag
as pending.So there is no chance of any new interrupt.
So I think set PENDING just confirm one time, it just depends on primary
handler
execution time and irq frequency.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 2012-10-12 at 15:29 +, Liu, Chuansheng wrote: -Original Message- From: anish kumar [mailto:anish198519851...@gmail.com] Sent: Friday, October 12, 2012 11:25 PM To: Liu, Chuansheng Cc: Thomas Gleixner; linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? Why will the interrupt be raised again?Is not it masked?I read tglx I means because it is masked, if at this time device issues edge irq, It will not be delivered and lost. That depends on the hardware, doesn't it?If if the frequency of interrupt is too much that device is not able to cope up with that then don't you think that the interrupt controller is faulty? Well I am not a expert on the hardware but it looks to me that the hardware should be at a fault here. statement as this:if the interrupt is being handled on one core, then the delivery of new interrupt can be on the second core and in that case it will see IRQ_INPROGRESS flag and it will *mask* it and set the flag as pending.So there is no chance of any new interrupt. So I think set PENDING just confirm one time, it just depends on primary handler execution time and irq frequency. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: anish kumar [mailto:anish198519851...@gmail.com] Sent: Friday, October 12, 2012 11:25 PM To: Liu, Chuansheng Cc: Thomas Gleixner; linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? Why will the interrupt be raised again?Is not it masked?I read tglx I means because it is masked, if at this time device issues edge irq, It will not be delivered and lost. No, it is NOT lost. The irq is marked PENDING already, so we invoke the handler again and handle it. And before we invoke the handler another time we unmask it. It does not matter at all whether the interrupt has been sent five times while it was masked. What matters is that we recorded the first one and set the PENDING flag. That way we invoke the interrupt handler again and keep stuff rolling. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? So I think set PENDING just confirm one time, it just depends on primary handler execution time and irq frequency. It does NOT matter. We marked it PENDING. And that guarantees that we invoke the handler again after unmasking. And that keeps stuff rolling. We would lose when we masked and did not notice that there was one, but we notice and store that information with the PENDING flag. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 12 Oct 2012, Liu, Chuansheng wrote: But IRQS_ONESHOT does not work well for edge interrupt. And pasting the IRQS_ONESHOT description: * IRQS_ONESHOT- irq is not unmasked in primary handler Right, and edge type interrupts doe not support it. Can we do something? Thanks your sharing. No, we cannot do anything. The edge handler is not going to change. End of story. In request_thread_irq() case with FLAG IRQS_ONESHOT, for edge interrupt, in function irq_finalize_oneshot(): if (!desc-threads_oneshot !irqd_irq_disabled(desc-irq_data) irqd_irq_masked(desc-irq_data)) unmask_irq(desc); It is possible unmask_irq() is called, but the below code is just aiming for masking action in irq handler, so I guess if I called the mask_irq() in non-core code, when irq_finalize_oneshot is called, the unmask_irq is called, and it is not we wanted, right? Do not test this case:) You're guessing wrong again. Non core code CANNOT call mask_irq() except via irq_disable(). No have a look at irq_disable() and then read the above condition again. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
On Fri, 2012-10-12 at 22:52 +0200, Thomas Gleixner wrote: On Fri, 12 Oct 2012, Liu, Chuansheng wrote: -Original Message- From: anish kumar [mailto:anish198519851...@gmail.com] Sent: Friday, October 12, 2012 11:25 PM To: Liu, Chuansheng Cc: Thomas Gleixner; linux-kernel@vger.kernel.org Subject: RE: [PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread On Fri, 2012-10-12 at 14:57 +, Liu, Chuansheng wrote: On SMP an interrupt which is raised after the ack() again before the handler finishes, can invoke another delivery on a different CPU, which then sees the IRQ_INPROGESS flag, masks it and flags it PENDING. When the primary handler on the first CPU returns, it sees the PENDING flag, unmasks and invokes the handler another time. In this case, when IRQ_INPROGRESS flag is set, on another CPU, it will mask and ack it, if before the primary handler on the first CPU returns, the edge interrupt is raised again, it will be lost, right? Why will the interrupt be raised again?Is not it masked?I read tglx I means because it is masked, if at this time device issues edge irq, It will not be delivered and lost. No, it is NOT lost. The irq is marked PENDING already, so we invoke It is fairly easy for an edge triggered interrupt to be missed - for example if interrupts have to be masked for a period - and unless there is some type of hardware latch that records the event it is impossible to recover. tglx, explanation will only work if we have a hardware latch which when unmasked sends all those edge interrupts again (which had come when it was masked while the CPU was handling the same interrupts). PS:http://kernel.org/doc/htmldocs/genericirq.html the handler again and handle it. And before we invoke the handler another time we unmask it. It does not matter at all whether the interrupt has been sent five times while it was masked. What matters is that we recorded the first one and set the PENDING flag. That way we invoke the interrupt handler again and keep stuff rolling. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
In our system, there is one edge interrupt, and we want it to be
irq thread with IRQS_ONESHOT, and found in handle_edge_irq(),
even with IRQS_ONESHOT, the irq is still unmasked without care of
flag IRQS_ONESHOT.
It causes IRQS_ONESHOT can not work well for edge interrupt, but also
after the irq thread finished with flag IRQS_ONESHOT, the irq will be
possible to be unmasked again, it should be messing mask/unmask logic.
Signed-off-by: liu chuansheng
---
kernel/irq/chip.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 57d86d0..f23f524 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -497,7 +497,13 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)
kstat_incr_irqs_this_cpu(irq, desc);
/* Start handling the irq */
- desc->irq_data.chip->irq_ack(>irq_data);
+ if (desc->istate & IRQS_ONESHOT) {
+ mask_ack_irq(desc);
+ handle_irq_event(desc);
+ cond_unmask_irq(desc);
+ goto out_unlock;
+ } else
+ desc->irq_data.chip->irq_ack(>irq_data);
do {
if (unlikely(!desc->action)) {
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] genirq: for edge interrupt IRQS_ONESHOT support with irq thread
In our system, there is one edge interrupt, and we want it to be
irq thread with IRQS_ONESHOT, and found in handle_edge_irq(),
even with IRQS_ONESHOT, the irq is still unmasked without care of
flag IRQS_ONESHOT.
It causes IRQS_ONESHOT can not work well for edge interrupt, but also
after the irq thread finished with flag IRQS_ONESHOT, the irq will be
possible to be unmasked again, it should be messing mask/unmask logic.
Signed-off-by: liu chuansheng chuansheng@intel.com
---
kernel/irq/chip.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 57d86d0..f23f524 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -497,7 +497,13 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)
kstat_incr_irqs_this_cpu(irq, desc);
/* Start handling the irq */
- desc-irq_data.chip-irq_ack(desc-irq_data);
+ if (desc-istate IRQS_ONESHOT) {
+ mask_ack_irq(desc);
+ handle_irq_event(desc);
+ cond_unmask_irq(desc);
+ goto out_unlock;
+ } else
+ desc-irq_data.chip-irq_ack(desc-irq_data);
do {
if (unlikely(!desc-action)) {
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
