From: Ben Hutchings <ben@decadent.org.uk>
To: David Miller <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>
Subject: [PATCH net-2.6] 3c59x: Remove incorrect locking; correct documented lock hierarchy
Date: Tue, 31 Aug 2010 01:15:33 +0100 [thread overview]
Message-ID: <1283213733.7653.180.camel@localhost> (raw)
vortex_ioctl() was grabbing vortex_private::lock around its call to
generic_mii_ioctl(). This is no longer necessary since there are more
specific locks which the mdio_{read,write}() functions will obtain.
Worse, those functions do not save and restore IRQ flags when locking
the MII state, so interrupts will be enabled when generic_mii_ioctl()
returns.
Since there is currently no need for any function to call
mdio_{read,write}() while holding another spinlock, do not change them
to save and restore IRQ flags but remove the specification of ordering
between vortex_private::lock and vortex_private::mii_lock.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
I've now borrowed a card to test 3c59x on. I've seen another regression
reported <http://bugs.debian.org/586967> after my locking changes, which
I can't reproduce.
Ben.
drivers/net/3c59x.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index c685a55..a045559 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -647,7 +647,7 @@ struct vortex_private {
u16 io_size; /* Size of PCI region (for release_region) */
/* Serialises access to hardware other than MII and variables below.
- * The lock hierarchy is rtnl_lock > lock > mii_lock > window_lock. */
+ * The lock hierarchy is rtnl_lock > {lock, mii_lock} > window_lock. */
spinlock_t lock;
spinlock_t mii_lock; /* Serialises access to MII */
@@ -2984,7 +2984,6 @@ static int vortex_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
int err;
struct vortex_private *vp = netdev_priv(dev);
- unsigned long flags;
pci_power_t state = 0;
if(VORTEX_PCI(vp))
@@ -2994,9 +2993,7 @@ static int vortex_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
if(state != 0)
pci_set_power_state(VORTEX_PCI(vp), PCI_D0);
- spin_lock_irqsave(&vp->lock, flags);
err = generic_mii_ioctl(&vp->mii, if_mii(rq), cmd, NULL);
- spin_unlock_irqrestore(&vp->lock, flags);
if(state != 0)
pci_set_power_state(VORTEX_PCI(vp), state);
--
1.7.1
next reply other threads:[~2010-08-31 0:15 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-31 0:15 Ben Hutchings [this message]
2010-09-01 21:38 ` [PATCH net-2.6] 3c59x: Remove incorrect locking; correct documented lock hierarchy David Miller
2010-09-01 21:47 ` Ben Hutchings
2010-09-01 22:08 ` Ben Hutchings
2010-09-02 0:37 ` David Miller
2010-09-02 0:42 ` Ben Hutchings
2010-09-02 1:02 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1283213733.7653.180.camel@localhost \
--to=ben@decadent.org.uk \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.