From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Ungerer Subject: Re: [PATCH 1/2] fec: fix recursive locking of mii_lock Date: Thu, 03 Sep 2009 11:16:16 +1000 Message-ID: <4A9F18E0.3060307@snapgear.com> References: <20090831132200.GA21836@pengutronix.de> <1251882856-23549-1-git-send-email-u.kleine-koenig@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-rt-users@vger.kernel.org, Ben Hutchings , Patrick McHardy , Sascha Hauer , Matt Waddel , netdev@vger.kernel.org, Tim Sander To: =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= Return-path: Received: from rex.securecomputing.com ([203.24.151.4]:50528 "EHLO cyberguard.com.au" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753987AbZICBQt (ORCPT ); Wed, 2 Sep 2009 21:16:49 -0400 In-Reply-To: <1251882856-23549-1-git-send-email-u.kleine-koenig@pengutronix.de> Sender: netdev-owner@vger.kernel.org List-ID: Hi Uwe, Uwe Kleine-K=C3=B6nig wrote: > mii_discover_phy is only called by fec_enet_mii (via mip->mii_func). = So > &fep->mii_lock is already held and mii_discover_phy must not call > mii_queue which locks &fep->mii_lock, too. >=20 > This was noticed by lockdep: >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > [ INFO: possible recursive locking detected ] > 2.6.31-rc8-00038-g37d0892 #109 > --------------------------------------------- > swapper/1 is trying to acquire lock: > (&fep->mii_lock){-.....}, at: [] mii_queue+0x2c/0xcc >=20 > but task is already holding lock: > (&fep->mii_lock){-.....}, at: [] fec_enet_interrupt+0x78/= 0x460 >=20 > other info that might help us debug this: > 2 locks held by swapper/1: > #0: (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x18/0x20 > #1: (&fep->mii_lock){-.....}, at: [] fec_enet_interrupt+= 0x78/0x460 >=20 > stack backtrace: > Backtrace: > [] (dump_backtrace+0x0/0x108) from [] (dump_stac= k+0x18/0x1c) > r6:c781d118 r5:c03e41d8 r4:00000001 > [] (dump_stack+0x0/0x1c) from [] (__lock_acquire= +0x1a20/0x1a88) > [] (__lock_acquire+0x0/0x1a88) from [] (lock_acq= uire+0x60/0x74) > [] (lock_acquire+0x0/0x74) from [] (_spin_lock_i= rqsave+0x54/0x68) > r7:60000093 r6:c01569f8 r5:c785e468 r4:00000000 > [] (_spin_lock_irqsave+0x0/0x68) from [] (mii_qu= eue+0x2c/0xcc) > r7:c785e468 r6:c0156b24 r5:600a0000 r4:c785e000 > [] (mii_queue+0x0/0xcc) from [] (mii_discover_ph= y+0x54/0xa8) > r8:00000002 r7:00000032 r6:c785e000 r5:c785e360 r4:c785e000 > [] (mii_discover_phy+0x0/0xa8) from [] (fec_enet= _interrupt+0xa4/0x460) > r5:c785e360 r4:c077a170 > [] (fec_enet_interrupt+0x0/0x460) from [] (handl= e_IRQ_event+0x48/0x120) > [] (handle_IRQ_event+0x0/0x120) from [] (handle_= level_irq+0x94/0x11c) > ... >=20 > Signed-off-by: Uwe Kleine-K=C3=B6nig > Cc: Greg Ungerer Looks good too. Acked-by: Greg Ungerer > Cc: Ben Hutchings > Cc: Patrick McHardy > Cc: Sascha Hauer > Cc: Matt Waddel > Cc: netdev@vger.kernel.org > Cc: Tim Sander > --- > drivers/net/fec.c | 22 +++++++++++++++++----- > 1 files changed, 17 insertions(+), 5 deletions(-) >=20 > diff --git a/drivers/net/fec.c b/drivers/net/fec.c > index c9fd82d..ef82606 100644 > --- a/drivers/net/fec.c > +++ b/drivers/net/fec.c > @@ -637,16 +637,15 @@ unlock: > } > =20 > static int > -mii_queue(struct net_device *dev, int regval, void (*func)(uint, str= uct net_device *)) > +mii_queue_unlocked(struct net_device *dev, int regval, > + void (*func)(uint, struct net_device *)) > { > struct fec_enet_private *fep; > - unsigned long flags; > mii_list_t *mip; > int retval; > =20 > /* Add PHY address to register command */ > fep =3D netdev_priv(dev); > - spin_lock_irqsave(&fep->mii_lock, flags); > =20 > regval |=3D fep->phy_addr << 23; > retval =3D 0; > @@ -667,6 +666,19 @@ mii_queue(struct net_device *dev, int regval, vo= id (*func)(uint, struct net_devi > retval =3D 1; > } > =20 > + return retval; > +} > + > +static int > +mii_queue(struct net_device *dev, int regval, > + void (*func)(uint, struct net_device *)) > +{ > + struct fec_enet_private *fep; > + unsigned long flags; > + int retval; > + fep =3D netdev_priv(dev); > + spin_lock_irqsave(&fep->mii_lock, flags); > + retval =3D mii_queue_unlocked(dev, regval, func); > spin_unlock_irqrestore(&fep->mii_lock, flags); > return retval; > } > @@ -1373,11 +1385,11 @@ mii_discover_phy(uint mii_reg, struct net_dev= ice *dev) > =20 > /* Got first part of ID, now get remainder */ > fep->phy_id =3D phytype << 16; > - mii_queue(dev, mk_mii_read(MII_REG_PHYIR2), > + mii_queue_unlocked(dev, mk_mii_read(MII_REG_PHYIR2), > mii_discover_phy3); > } else { > fep->phy_addr++; > - mii_queue(dev, mk_mii_read(MII_REG_PHYIR1), > + mii_queue_unlocked(dev, mk_mii_read(MII_REG_PHYIR1), > mii_discover_phy); > } > } else { --=20 -----------------------------------------------------------------------= - Greg Ungerer -- Principal Engineer EMAIL: gerg@snapgear.co= m SnapGear Group, McAfee PHONE: +61 7 3435 288= 8 825 Stanley St, FAX: +61 7 3891 363= 0 Woolloongabba, QLD, 4102, Australia WEB: http://www.SnapGear.co= m