* [PATCH] stmmac: call local_irq_save other than disable_irq in poll_controller
@ 2015-05-05 7:55 Sonic Zhang
2015-05-05 23:36 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Sonic Zhang @ 2015-05-05 7:55 UTC (permalink / raw)
To: Giuseppe Cavallaro, David S. Miller
Cc: netdev, adi-buildroot-devel, Sonic Zhang
From: Sonic Zhang <sonic.zhang@analog.com>
It is possible that poll_controller is called from netpoll.c with irq
already disabled. Unexpected interrupt may be triggered if always
enable irq before returning from poll_controller.
Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 05c146f..830c008 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2465,9 +2465,11 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id)
*/
static void stmmac_poll_controller(struct net_device *dev)
{
- disable_irq(dev->irq);
+ unsigned long flags;
+
+ local_irq_save(flags);
stmmac_interrupt(dev->irq, dev);
- enable_irq(dev->irq);
+ local_irq_restore(flags);
}
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] stmmac: call local_irq_save other than disable_irq in poll_controller
2015-05-05 7:55 [PATCH] stmmac: call local_irq_save other than disable_irq in poll_controller Sonic Zhang
@ 2015-05-05 23:36 ` David Miller
2015-05-06 3:53 ` Sonic Zhang
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2015-05-05 23:36 UTC (permalink / raw)
To: sonic.adi; +Cc: peppe.cavallaro, netdev, adi-buildroot-devel, sonic.zhang
From: Sonic Zhang <sonic.adi@gmail.com>
Date: Tue, 5 May 2015 15:55:53 +0800
> From: Sonic Zhang <sonic.zhang@analog.com>
>
> It is possible that poll_controller is called from netpoll.c with irq
> already disabled. Unexpected interrupt may be triggered if always
> enable irq before returning from poll_controller.
>
> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Nope, this is wrong.
If you don't use disable_irq(), the interrupt handler can run in
parallel on another cpu and you definitely do not want that.
There is nothing wrong with the interrupt triggering on the
enable_irq() here. If that causes some problem, there is a
bug in this driver.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] stmmac: call local_irq_save other than disable_irq in poll_controller
2015-05-05 23:36 ` David Miller
@ 2015-05-06 3:53 ` Sonic Zhang
2015-05-06 14:02 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Sonic Zhang @ 2015-05-06 3:53 UTC (permalink / raw)
To: David Miller
Cc: Giuseppe CAVALLARO, netdev, adi-buildroot-devel, Zhang, Sonic
Hi David,
On Wed, May 6, 2015 at 7:36 AM, David Miller <davem@davemloft.net> wrote:
> From: Sonic Zhang <sonic.adi@gmail.com>
> Date: Tue, 5 May 2015 15:55:53 +0800
>
>> From: Sonic Zhang <sonic.zhang@analog.com>
>>
>> It is possible that poll_controller is called from netpoll.c with irq
>> already disabled. Unexpected interrupt may be triggered if always
>> enable irq before returning from poll_controller.
>>
>> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
>
> Nope, this is wrong.
>
> If you don't use disable_irq(), the interrupt handler can run in
> parallel on another cpu and you definitely do not want that.
How about change local_irq_save(flags) to
spin_lock_irqsave(&priv->lock, flags) ?
>
> There is nothing wrong with the interrupt triggering on the
> enable_irq() here. If that causes some problem, there is a
> bug in this driver.
When I debug Linux kernel via KGDB over Ethernet, KGDB calls netpoll
API and stmmac_poll_controller to send UDP packets with all interrupts
disabled. KGDB assumes all interrupts keeps disabled when it is active
and doing UDP data transmission. Enabling interrupts in
stmmac_poll_controller causes KGDB re-enter exception and system halt.
Although KGDB over Ethernet is not part of the mainline tree, I feel
this patch is useful in case other kernel functions want to call
netpoll API with interrupts disabled.
Regards,
Sonic
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] stmmac: call local_irq_save other than disable_irq in poll_controller
2015-05-06 3:53 ` Sonic Zhang
@ 2015-05-06 14:02 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2015-05-06 14:02 UTC (permalink / raw)
To: sonic.adi; +Cc: peppe.cavallaro, netdev, adi-buildroot-devel, sonic.zhang
From: Sonic Zhang <sonic.adi@gmail.com>
Date: Wed, 6 May 2015 11:53:53 +0800
> When I debug Linux kernel via KGDB over Ethernet, KGDB calls netpoll
> API and stmmac_poll_controller to send UDP packets with all interrupts
> disabled. KGDB assumes all interrupts keeps disabled when it is active
> and doing UDP data transmission. Enabling interrupts in
> stmmac_poll_controller causes KGDB re-enter exception and system halt.
If KGDB has disabled local cpu interrupts, that cannot happen.
What stmmac_poll_controller() is doing is enabling a specific interupt
source, not "interrupts" generally. The latter is what KGDB can
specifically control and should care about.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-05-06 14:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-05 7:55 [PATCH] stmmac: call local_irq_save other than disable_irq in poll_controller Sonic Zhang
2015-05-05 23:36 ` David Miller
2015-05-06 3:53 ` Sonic Zhang
2015-05-06 14:02 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.