* Re: [PATCH 02/17] sfc: Consistently report short MCDI responses as EIO
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482834.2549.18.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:27:14 +0100
> In some cases failing functions were returning 0 which is obviously wrong.
> In other cases they were returning inappropriate error codes.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 03/17] sfc: Handle serious errors in exactly one interrupt handler
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482856.2549.19.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:27:36 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> 'Fatal' errors set an interrupt flag associated with a specific event
> queue; only read the syndrome vector if we see that queue's flag set
> (legacy interrupts) or in the interrupt handler for that queue (MSI).
>
> Do not ignore an interrupt if the fatal error flag is set but specific
> error flags are all zero. Even if we don't schedule a reset, we must
> respect the queue mask and rearm the appropriate event queues.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 04/17] sfc: Stop masking out XGMII faults over reconfigures
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482874.2549.20.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:27:54 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> The aim of this code was to avoid a spurious XGMII fault over a MAC
> reconfigure. It's less relevant now that the PHY reconfigure isn't
> called from the MAC reconfigure.
>
> After applying this patch, our link stress test passed 48 hours of
> testing without ever resetting the PHY.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 05/17] sfc: Reconfigure the XAUI serdes after an EM reset
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482890.2549.21.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:28:10 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> Fix a regression introduced in d3245b28ef2a45ec4e115062a38100bd06229289
> "sfc: Refactor link configuration".
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 06/17] sfc: Extend the legacy interrupt workarounds
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482907.2549.22.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:28:27 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> Siena has two problems with legacy interrupts:
> 1. There is no synchronisation between the ISR read completion,
> and the interrupt deassert message.
> 2. A downstream read at the "wrong" moment can return 0, and
> suppress generating the next interrupt.
>
> Falcon should suffer from both of these, and it appears it does.
> Enable EFX_WORKAROUND_15783 on Falcon as well.
>
> Also, when we see queues == 0, ensure we always schedule or rearm
> every event queue.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 07/17] sfc: Log specific message for failure of NVRAM self-test
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482916.2549.23.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:28:36 +0100
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 08/17] sfc: Read MEM_STAT for SRM_PERR as well as MEM_PERR errors
From: David Miller @ 2010-04-28 19:45 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482932.2549.24.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:28:52 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> Parity errors in different blocks of SRAM may set one of two different
> interrupt flags.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 09/17] sfc: Enable IPv6 RSS using random key for Toeplitz hash
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482942.2549.25.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:29:02 +0100
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 10/17] sfc: Update MCDI protocol definitions
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482954.2549.26.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:29:14 +0100
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 11/17] sfc: Set PERIODIC_NOEVENT flag for MC_CMD_MAC_STATS
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482972.2549.27.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:29:32 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> When set, an event is not sent whenever periodic MAC statistics are
> raised. This avoids unnecessary wake-ups.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 12/17] sfc: Break NAPI processing after one ring-full of TX completions
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482982.2549.28.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:29:42 +0100
> Currently TX completions do not count towards the NAPI budget. This
> means a continuous stream of TX completions can cause the polling
> function to loop indefinitely with scheduling disabled. To avoid
> this, follow the common practice of reporting the budget spent after
> processing one ring-full of TX completions.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 13/17] sfc: Add necessary parentheses to macro definitions in net_driver.h
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272482990.2549.29.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:29:50 +0100
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 14/17] sfc: Clean up efx_nic::irq_zero_count
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272483000.2549.30.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:30:00 +0100
> There is no need for this to be unsigned long; make it unsigned int.
> It does need a line in kernel-doc, so add that.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 15/17] sfc: Add Siena PHY BIST and cable diagnostic support
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272483022.2549.31.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:30:22 +0100
> From: Steve Hodgson <shodgson@solarflare.com>
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 16/17] sfc: Test only the first pair of TX queues
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272483030.2549.32.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:30:30 +0100
> This makes no immediate difference, but we definitely do not want
> to test all TX queues once we allocate a pair of TX queues to each
> channel.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 17/17] sfc: Create multiple TX queues
From: David Miller @ 2010-04-28 19:46 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1272483043.2549.33.camel@achroite.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 28 Apr 2010 20:30:43 +0100
> Create a core TX queue and 2 hardware TX queues for each channel.
> If separate_tx_channels is set, create equal numbers of RX and TX
> channels instead.
>
> Rewrite the channel and queue iteration macros accordingly.
> Eliminate efx_channel::used_flags as redundant.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* [PATCH v2] net/sb1250: register mdio bus in probe
From: Sebastian Andrzej Siewior @ 2010-04-28 19:57 UTC (permalink / raw)
To: David Miller; +Cc: ralf, netdev
In-Reply-To: <20100427.155215.228945283.davem@davemloft.net>
"ifconfig eth0 up && ifconfig eth0 down" triggers:
| kobject (a8000000cfa5a480): tried to init an initialized object, something is seriously wrong.
| Call Trace:
| [<ffffffff8010aabc>] dump_stack+0x8/0x34
| [<ffffffff80293128>] kobject_init+0xe8/0xf0
| [<ffffffff802d922c>] device_initialize+0x2c/0x98
| [<ffffffff802d9cfc>] device_register+0x14/0x28
| [<ffffffff80312cd4>] mdiobus_register+0xdc/0x1e0
| [<ffffffff80314cf0>] sbmac_open+0x58/0x220
| [<ffffffff803519bc>] __dev_open+0x11c/0x180
| [<ffffffff8034d578>] __dev_change_flags+0x120/0x180
| [<ffffffff80351848>] dev_change_flags+0x20/0x78
| [<ffffffff803a753c>] devinet_ioctl+0x7cc/0x820
| [<ffffffff80339ac8>] sock_do_ioctl+0x38/0x90
| [<ffffffff8033a258>] compat_sock_ioctl_trans+0x408/0x1030
| [<ffffffff8033af30>] compat_sock_ioctl+0xb0/0xd0
| [<ffffffff80208b08>] compat_sys_ioctl+0xa0/0x18b8
| [<ffffffff80102f94>] handle_sys+0x114/0x130
|
| sb1250-mac-mdio: probed
mdiobus_register() calls device_register() which initializes the kobj of
the device. mdiobus_unregister() calls only device_del() so we have one
reference left. That one is leaving with mdiobus_free() which is only
called on remove.
Since I don't see any reason why mdiobus_register()/mdiobus_unregister()
should happen in ->open()/->close() I move them to probe & exit.
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
v2: register mdio bus before registering netdev device
drivers/net/sb1250-mac.c | 67 ++++++++++++++++++++++-----------------------
1 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/net/sb1250-mac.c b/drivers/net/sb1250-mac.c
index 3f882d5..b0161b4 100644
--- a/drivers/net/sb1250-mac.c
+++ b/drivers/net/sb1250-mac.c
@@ -2256,17 +2256,36 @@ static int sbmac_init(struct platform_device *pldev, long long base)
sc->mii_bus = mdiobus_alloc();
if (sc->mii_bus == NULL) {
- sbmac_uninitctx(sc);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto uninit_ctx;
}
+ sc->mii_bus->name = sbmac_mdio_string;
+ snprintf(sc->mii_bus->id, MII_BUS_ID_SIZE, "%x", idx);
+ sc->mii_bus->priv = sc;
+ sc->mii_bus->read = sbmac_mii_read;
+ sc->mii_bus->write = sbmac_mii_write;
+ sc->mii_bus->irq = sc->phy_irq;
+ for (i = 0; i < PHY_MAX_ADDR; ++i)
+ sc->mii_bus->irq[i] = SBMAC_PHY_INT;
+
+ sc->mii_bus->parent = &pldev->dev;
+ /*
+ * Probe PHY address
+ */
+ err = mdiobus_register(sc->mii_bus);
+ if (err) {
+ printk(KERN_ERR "%s: unable to register MDIO bus\n",
+ dev->name);
+ goto free_mdio;
+ }
+ dev_set_drvdata(&pldev->dev, sc->mii_bus);
+
err = register_netdev(dev);
if (err) {
printk(KERN_ERR "%s.%d: unable to register netdev\n",
sbmac_string, idx);
- mdiobus_free(sc->mii_bus);
- sbmac_uninitctx(sc);
- return err;
+ goto unreg_mdio;
}
pr_info("%s.%d: registered as %s\n", sbmac_string, idx, dev->name);
@@ -2282,19 +2301,15 @@ static int sbmac_init(struct platform_device *pldev, long long base)
pr_info("%s: SiByte Ethernet at 0x%08Lx, address: %pM\n",
dev->name, base, eaddr);
- sc->mii_bus->name = sbmac_mdio_string;
- snprintf(sc->mii_bus->id, MII_BUS_ID_SIZE, "%x", idx);
- sc->mii_bus->priv = sc;
- sc->mii_bus->read = sbmac_mii_read;
- sc->mii_bus->write = sbmac_mii_write;
- sc->mii_bus->irq = sc->phy_irq;
- for (i = 0; i < PHY_MAX_ADDR; ++i)
- sc->mii_bus->irq[i] = SBMAC_PHY_INT;
-
- sc->mii_bus->parent = &pldev->dev;
- dev_set_drvdata(&pldev->dev, sc->mii_bus);
-
return 0;
+unreg_mdio:
+ mdiobus_unregister(sc->mii_bus);
+ dev_set_drvdata(&pldev->dev, NULL);
+free_mdio:
+ mdiobus_free(sc->mii_bus);
+uninit_ctx:
+ sbmac_uninitctx(sc);
+ return err;
}
@@ -2320,16 +2335,6 @@ static int sbmac_open(struct net_device *dev)
goto out_err;
}
- /*
- * Probe PHY address
- */
- err = mdiobus_register(sc->mii_bus);
- if (err) {
- printk(KERN_ERR "%s: unable to register MDIO bus\n",
- dev->name);
- goto out_unirq;
- }
-
sc->sbm_speed = sbmac_speed_none;
sc->sbm_duplex = sbmac_duplex_none;
sc->sbm_fc = sbmac_fc_none;
@@ -2360,11 +2365,7 @@ static int sbmac_open(struct net_device *dev)
return 0;
out_unregister:
- mdiobus_unregister(sc->mii_bus);
-
-out_unirq:
free_irq(dev->irq, dev);
-
out_err:
return err;
}
@@ -2553,9 +2554,6 @@ static int sbmac_close(struct net_device *dev)
phy_disconnect(sc->phy_dev);
sc->phy_dev = NULL;
-
- mdiobus_unregister(sc->mii_bus);
-
free_irq(dev->irq, dev);
sbdma_emptyring(&(sc->sbm_txdma));
@@ -2663,6 +2661,7 @@ static int __exit sbmac_remove(struct platform_device *pldev)
unregister_netdev(dev);
sbmac_uninitctx(sc);
+ mdiobus_unregister(sc->mii_bus);
mdiobus_free(sc->mii_bus);
iounmap(sc->sbm_base);
free_netdev(dev);
--
1.6.6.1
^ permalink raw reply related
* Re: [PATCH net-next-2.6 1/7] caif: Ldisc add permission check and mem-alloc error check
From: David Miller @ 2010-04-28 20:02 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-1-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:34 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Changes:
> o Added permission checks for installing. CAP_SYS_ADMIN and
> CAP_SYS_TTY_CONFIG can install the ldisc.
> o Check if allocation of skb was successful.
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/7] caif: Rename functions in cfcnfg and caif_dev
From: David Miller @ 2010-04-28 20:03 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-2-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:35 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Changes:
> o Renamed cfcnfg_del_adapt_layer to cfcnfg_disconn_adapt_layer
> o Fixed typo cfcfg to cfcnfg
> o Renamed linkid to channel_id
> o Updated documentation in caif_dev.h
> o Minor formatting changes
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 3/7] caif: Add reference counting to service layer
From: David Miller @ 2010-04-28 20:03 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-3-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:36 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Changes:
> o Added functions cfsrvl_get and cfsrvl_put.
> o Added support release_client to use by socket and net device.
> o Increase reference counting for in-flight packets from cfmuxl
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 4/7] caif: Disconnect without waiting for response
From: David Miller @ 2010-04-28 20:03 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-4-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:37 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Changes:
> o Function cfcnfg_disconn_adapt_layer is changed to do asynchronous
> disconnect, not waiting for any response from the modem. Due to this
> the function cfcnfg_linkdestroy_rsp does nothing anymore.
> o Because disconnect may take down a connection before a connect response
> is received the function cfcnfg_linkup_rsp is checking if the client is
> still waiting for the response, if not a disconnect request is sent to
> the modem.
> o cfctrl is no longer keeping track of pending disconnect requests.
> o Added function cfctrl_cancel_req, which is used for deleting a pending
> connect request if disconnect is done before connect response is received.
> o Removed unused function cfctrl_insert_req2
> o Added better handling of connect reject from modem.
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 5/7] caif: Rewritten socket implementation
From: David Miller @ 2010-04-28 20:03 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-5-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:38 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Changes:
> This is a complete re-write of the socket layer. Making the socket
> implementation more aligned with the other socket layers and using more
> of the support functions available in sock.c. Lots of code is copied
> from af_unix (and some from af_irda).
> Non-blocking mode should be working as well.
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 6/7] caif: Bugfixes in CAIF netdevice for close and flow control
From: David Miller @ 2010-04-28 20:03 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-6-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:39 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Changes:
> o Bugfix: Flow control was causing the device to be destroyed.
> o Bugfix: Handle CAIF channel connect failures.
> o If the underlying link layer is gone the net-device is no longer removed,
> but closed.
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 7/7] Bugfix: Link selection was swapped in switch.
From: David Miller @ 2010-04-28 20:03 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson, sjurbr, linus.walleij
In-Reply-To: <1272480880-30672-7-git-send-email-sjur.brandeland@stericsson.com>
From: sjur.brandeland@stericsson.com
Date: Wed, 28 Apr 2010 20:54:40 +0200
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied.
^ permalink raw reply
* Re: 2.6.34-rc5-git7 (plus all patches) -- another suspicious rcu_dereference_check() usage.
From: Paul E. McKenney @ 2010-04-28 20:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: Miles Lane, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
Peter Zijlstra, LKML, nauman, netdev, Jens Axboe, Gui Jianfeng,
Li Zefan, Johannes Berg, shemminger
In-Reply-To: <1272483491.2201.9.camel@edumazet-laptop>
On Wed, Apr 28, 2010 at 09:38:11PM +0200, Eric Dumazet wrote:
> Le mercredi 28 avril 2010 à 10:54 -0700, Paul E. McKenney a écrit :
> > On Mon, Apr 26, 2010 at 08:51:06PM -0400, Miles Lane wrote:
> > > This one occurred during the wakeup from suspend to RAM.
> > >
> > > [ 984.724697] [ INFO: suspicious rcu_dereference_check() usage. ]
> > > [ 984.724700] ---------------------------------------------------
> > > [ 984.724703] include/linux/fdtable.h:88 invoked
> > > rcu_dereference_check() without protection!
> > > [ 984.724706]
> > > [ 984.724707] other info that might help us debug this:
> > > [ 984.724708]
> > > [ 984.724711]
> > > [ 984.724711] rcu_scheduler_active = 1, debug_locks = 1
> > > [ 984.724714] no locks held by dbus-daemon/4680.
> > > [ 984.724717]
> > > [ 984.724717] stack backtrace:
> > > [ 984.724721] Pid: 4680, comm: dbus-daemon Not tainted 2.6.34-rc5-git7 #33
> > > [ 984.724724] Call Trace:
> > > [ 984.724734] [<ffffffff81074556>] lockdep_rcu_dereference+0x9d/0xa6
> > > [ 984.724740] [<ffffffff810fc785>] fcheck_files+0xb1/0xc9
> > > [ 984.724745] [<ffffffff810fc7f5>] fget_light+0x35/0xab
> > > [ 984.724751] [<ffffffff81433e1b>] ? sock_poll_wait+0x13/0x18
> > > [ 984.724755] [<ffffffff81433e39>] ? unix_poll+0x19/0x95
> > > [ 984.724762] [<ffffffff8110aa95>] do_sys_poll+0x1ff/0x3e5
> > > [ 984.724766] [<ffffffff8110a19e>] ? __pollwait+0x0/0xc7
> > > [ 984.724771] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724776] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724780] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724784] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724788] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724793] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724797] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724802] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724806] [<ffffffff8110a265>] ? pollwake+0x0/0x4f
> > > [ 984.724812] [<ffffffff8110ae0f>] sys_poll+0x50/0xbb
> > > [ 984.724818] [<ffffffff81009d82>] system_call_fastpath+0x16/0x1b
> >
> > Hmmm... I am not convinced that this is a false positive. Couldn't
> > there be a multi-threaded process where one thread is invoking poll()
> > on a UNIX socket just as another thread is calling close() on it?
> >
> > The current fcheck_files() logic requires that the caller either (1) be in
> > an RCU read-side critical section, (2) hold ->files_lock, or (3) passing
> > in a files_struct with ->count equal to 1 (initialization or cleanup).
> >
> > So I don't feel comfortable just slapping an RCU read-side critical
> > section around this one, at least not unless someone who understands
> > the locking says that doing so is OK.
> >
> >
>
> Its a single threaded program.
>
> So fget_light() calls fcheck_files(files, fd); without rcu lock,
> but some /proc/pid/fd/... user temporarly raised files->count just
> before we perform the condition check.
So I should add a single-threaded check. My first thought was to use
current_is_single_threaded(), but the bit about scanning the full list
of processes does give me pause. However, thread_group_empty() looks
like a much lighter-weight alternative.
I believe that it is possible for a pair of single-threaded processes
to share a file descriptor, but that should not be a problem, as both
of them would need to close it for it to go away.
But what happens if someone does a clone() with CLONE_FILES, as some
of the AIO stuff seems to do? Won't that allow one of the resulting
processes to close the file for both of them, even though both are
otherwise single-threaded? And the ->count seems to be the only
distinction between these two cases.
And AIO does CLONE_VM as well as CLONE_FILES, but that seems to mean that
the check must scan the processes with current_is_single_threaded().
Besides which, a user could invoke clone() with only CLONE_FILES
specified, right?
Or am I just confused here?
Thanx, Paul
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox