Re: [PATCH] forcedeth: fix unilateral interrupt disabling in netpoll path

[David Miller]

From: Neil Horman 
Date: Mon, 26 Oct 2015 12:24:22 -0400

> Forcedeth currently uses disable_irq_lockdep and enable_irq_lockdep, which in
> some configurations simply calls local_irq_disable.  This causes errant 
> warnings
> in the netpoll path as in netpoll_send_skb_on_dev, where we disable irqs using
> local_irq_save, leading to the following warning:
 ...
> Fix it by modifying the forcedeth code to use
> disable_irq_nosync_lockdep_irqsavedisable_irq_nosync_lockdep_irqsave instead,
> which saves and restores irq state properly.  This also saves us a little code
> in the process
> 
> Tested by the reporter, with successful restuls
> 
> Patch applies to the head of the net tree
> 
> Signed-off-by: Neil Horman 
> CC: "David S. Miller" 
> Reported-by: Vasily Averin 

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] forcedeth: fix unilateral interrupt disabling in netpoll path

[Neil Horman]

Forcedeth currently uses disable_irq_lockdep and enable_irq_lockdep, which in
some configurations simply calls local_irq_disable.  This causes errant warnings
in the netpoll path as in netpoll_send_skb_on_dev, where we disable irqs using
local_irq_save, leading to the following warning:

WARNING: at net/core/netpoll.c:352 netpoll_send_skb_on_dev+0x243/0x250() (Not
tainted)
Hardware name:
netpoll_send_skb_on_dev(): eth0 enabled interrupts in poll
(nv_start_xmit_optimized+0x0/0x860 [forcedeth])
Modules linked in: netconsole(+) configfs ipv6 iptable_filter ip_tables ppdev
parport_pc parport sg microcode serio_raw edac_core edac_mce_amd k8temp
snd_hda_codec_realtek snd_hda_codec_generic forcedeth snd_hda_intel
snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore
snd_page_alloc i2c_nforce2 i2c_core shpchp ext4 jbd2 mbcache sr_mod cdrom sd_mod
crc_t10dif pata_amd ata_generic pata_acpi sata_nv dm_mirror dm_region_hash
dm_log dm_mod [last unloaded: scsi_wait_scan]
Pid: 1940, comm: modprobe Not tainted 2.6.32-573.7.1.el6.x86_64.debug #1
Call Trace:
 [] ? warn_slowpath_common+0x91/0xe0
 [] ? warn_slowpath_fmt+0x46/0x60
 [] ? nv_start_xmit_optimized+0x0/0x860 [forcedeth]
 [] ? netpoll_send_skb_on_dev+0x243/0x250
 [] ? netpoll_send_udp+0x229/0x270
 [] ? write_msg+0x39/0x110 [netconsole]
 [] ? write_msg+0xbb/0x110 [netconsole]
 [] ? __call_console_drivers+0x75/0x90
 [] ? _call_console_drivers+0x4a/0x80
 [] ? release_console_sem+0xe5/0x250
 [] ? register_console+0x190/0x3e0
 [] ? init_netconsole+0x1a6/0x216 [netconsole]
 [] ? init_netconsole+0x0/0x216 [netconsole]
 [] ? do_one_initcall+0xc0/0x280
 [] ? sys_init_module+0xe3/0x260
 [] ? system_call_fastpath+0x16/0x1b
---[ end trace f349c7af88e6a6d5 ]---
console [netcon0] enabled
netconsole: network logging started

Fix it by modifying the forcedeth code to use
disable_irq_nosync_lockdep_irqsavedisable_irq_nosync_lockdep_irqsave instead,
which saves and restores irq state properly.  This also saves us a little code
in the process

Tested by the reporter, with successful restuls

Patch applies to the head of the net tree

Signed-off-by: Neil Horman 
CC: "David S. Miller" 
Reported-by: Vasily Averin 
---
 drivers/net/ethernet/nvidia/forcedeth.c | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c 
b/drivers/net/ethernet/nvidia/forcedeth.c
index a41bb5e..75e88f4 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -4076,6 +4076,8 @@ static void nv_do_nic_poll(unsigned long data)
struct fe_priv *np = netdev_priv(dev);
u8 __iomem *base = get_hwbase(dev);
u32 mask = 0;
+   unsigned long flags;
+   unsigned int irq = 0;
 
/*
 * First disable irq(s) and then
@@ -4085,25 +4087,27 @@ static void nv_do_nic_poll(unsigned long data)
 
if (!using_multi_irqs(dev)) {
if (np->msi_flags & NV_MSI_X_ENABLED)
-   
disable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
+   irq = np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector;
else
-   disable_irq_lockdep(np->pci_dev->irq);
+   irq = np->pci_dev->irq;
mask = np->irqmask;
} else {
if (np->nic_poll_irq & NVREG_IRQ_RX_ALL) {
-   
disable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
+   irq = np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector;
mask |= NVREG_IRQ_RX_ALL;
}
if (np->nic_poll_irq & NVREG_IRQ_TX_ALL) {
-   
disable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector);
+   irq = np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector;
mask |= NVREG_IRQ_TX_ALL;
}
if (np->nic_poll_irq & NVREG_IRQ_OTHER) {
-   
disable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector);
+   irq = np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector;
mask |= NVREG_IRQ_OTHER;
}
}
-   /* disable_irq() contains synchronize_irq, thus no irq handler can run 
now */
+
+   disable_irq_nosync_lockdep_irqsave(irq, );
+   synchronize_irq(irq);
 
if (np->recover_error) {
np->recover_error = 0;
@@ -4156,28 +4160,22 @@ static void nv_do_nic_poll(unsigned long data)
nv_nic_irq_optimized(0, dev);
else
nv_nic_irq(0, dev);
-   if (np->msi_flags & NV_MSI_X_ENABLED)
-   
enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
-   else
-   enable_irq_lockdep(np->pci_dev->irq);
} else {
if