* phylib: kernel oops on resume with 2.6.33-rc4
@ 2010-01-16 16:08 Nori, Sekhar
2010-01-16 22:37 ` Anton Vorontsov
0 siblings, 1 reply; 6+ messages in thread
From: Nori, Sekhar @ 2010-01-16 16:08 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: avorontsov@ru.mvista.com
Hello all,
I am hitting a kernel oops when resuming from suspend-to-RAM on 2.6.33-rc4
kernel.
The MAC is TI DaVinci EMAC and PHY is SMSC LAN8710. However, I also hit the
issue even when using generic PHY driver (CONFIG_SMSC_PHY turned off).
On reverting commit 541cd3e "phylib: Fix deadlock on resume" which was
introduced in 2.6.33-rc4, the issue doesn't come about. So far, I haven't
figured out how this particular commit is causing the oops. Full oops report
is attached. Appreciate any help in debugging this.
Thanks,
Sekhar
-----8<---------
root@da850-omapl138-evm:~# rtcwake -m mem -d /dev/rtc0 -s 15
wakeup from "mem" at Sat Jan 1 00:02:01 2000
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.01 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
Suspending console(s) (use no_console_suspend to debug)
PM: suspend of devices complete after 212.456 msecs
PM: late suspend of devices complete after 0.697 msecs
PM: early resume of devices complete after 0.220 msecs
eth0: attached PHY driver [SMSC LAN8710/LAN8720] (mii_bus:phy_addr=1:00, id=7c0f1)
PM: resume of devices complete after 237.098 msecs
Restarting tasks ... done.
kernel BUG at kernel/workqueue.c:354!
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
Internal error: Oops: 817 [#1] PREEMPT
last sysfs file: /sys/power/state
Modules linked in:
CPU: 0 Not tainted (2.6.33-rc4 #1830)
PC is at __bug+0x20/0x2c
LR is at release_console_sem+0xf0/0xfc
pc : [<c002c5b8>] lr : [<c003ed68>] psr: 60000013
sp : c7833ef0 ip : c7833e30 fp : c7833efc
r10: 00000000 r9 : 00000000 r8 : c78dfb30
r7 : 00000000 r6 : 00000064 r5 : ffffffff r4 : c78dfb04
r3 : 00000000 r2 : 00000000 r1 : 0000182b r0 : 0000002c
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: c7b64000 DAC: 00000017
Process events/0 (pid: 4, stack limit = 0xc7832270)
Stack: (0xc7833ef0 to 0xc7834000)
3ee0: c7833f1c c7833f00 c0052a54 c002c5a8
3f00: 00000000 c78dfb04 c78dfa00 00000000 c7833f2c c7833f20 c0052b30 c00529f0
3f20: c7833f3c c7833f30 c0052b58 c0052b14 c7833f64 c7833f40 c01cf8b4 c0052b44
3f40: c78dfb08 c7832000 c78dfb04 c7800380 c01cf4e0 00000000 c7833f8c c7833f68
3f60: c0052578 c01cf4f0 c7832000 c7800388 c7832000 c7800380 00000000 00000000
3f80: c7833fc4 c7833f90 c0052730 c00524c4 c02a6440 00000000 c7820960 c0056428
3fa0: c7833fa0 c7833fa0 c7833fcc c7823f18 c7800380 c005263c c7833ff4 c7833fc8
3fc0: c00560c0 c005264c 00000000 00000000 c7833fd0 c7833fd0 00000000 00000000
3fe0: 00000000 00000000 00000000 c7833ff8 c0042218 c0056048 44004012 00300000
Backtrace:
[<c002c598>] (__bug+0x0/0x2c) from [<c0052a54>] (queue_delayed_work_on+0x74/0xf8)
[<c00529e0>] (queue_delayed_work_on+0x0/0xf8) from [<c0052b30>] (queue_delayed_work+0x2c/0x30)
r7:00000000 r6:c78dfa00 r5:c78dfb04 r4:00000000
[<c0052b04>] (queue_delayed_work+0x0/0x30) from [<c0052b58>] (schedule_delayed_work+0x24/0x2c)
[<c0052b34>] (schedule_delayed_work+0x0/0x2c) from [<c01cf8b4>] (phy_state_machine+0x3d4/0x3e8)
[<c01cf4e0>] (phy_state_machine+0x0/0x3e8) from [<c0052578>] (run_workqueue+0xc4/0x188)
r9:00000000 r8:c01cf4e0 r7:c7800380 r6:c78dfb04 r5:c7832000
r4:c78dfb08
[<c00524b4>] (run_workqueue+0x0/0x188) from [<c0052730>] (worker_thread+0xf4/0x108)
r9:00000000 r8:00000000 r7:c7800380 r6:c7832000 r5:c7800388
r4:c7832000
[<c005263c>] (worker_thread+0x0/0x108) from [<c00560c0>] (kthread+0x88/0x90)
r7:c005263c r6:c7800380 r5:c7823f18 r4:c7833fcc
[<c0056038>] (kthread+0x0/0x90) from [<c0042218>] (do_exit+0x0/0x280)
r7:00000000 r6:00000000 r5:00000000 r4:00000000
Code: e1a01000 e59f000c eb09e6f5 e3a03000 (e5833000)
---[ end trace e88b3bd0ec06cbd0 ]---
nfs: server 172.24.190.197 not responding, still trying
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: phylib: kernel oops on resume with 2.6.33-rc4
2010-01-16 16:08 phylib: kernel oops on resume with 2.6.33-rc4 Nori, Sekhar
@ 2010-01-16 22:37 ` Anton Vorontsov
2010-01-17 8:28 ` Nori, Sekhar
0 siblings, 1 reply; 6+ messages in thread
From: Anton Vorontsov @ 2010-01-16 22:37 UTC (permalink / raw)
To: Nori, Sekhar; +Cc: netdev@vger.kernel.org
Hello,
On Sat, Jan 16, 2010 at 09:38:34PM +0530, Nori, Sekhar wrote:
> Hello all,
>
> I am hitting a kernel oops when resuming from suspend-to-RAM on 2.6.33-rc4
> kernel.
>
> The MAC is TI DaVinci EMAC and PHY is SMSC LAN8710. However, I also hit the
> issue even when using generic PHY driver (CONFIG_SMSC_PHY turned off).
>
> On reverting commit 541cd3e "phylib: Fix deadlock on resume" which was
> introduced in 2.6.33-rc4, the issue doesn't come about. So far, I haven't
> figured out how this particular commit is causing the oops. Full oops report
> is attached. Appreciate any help in debugging this.
It seems phy_start_machine() needlessly reinitialized work
struct. This is OK for ucc_geth and gianfar driver because
these drivers do not detach PHYs on suspend, but TI davinci
does. :-/
Can you try the patch down below?
Thanks!
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index b0e9f9c..0295097 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -410,7 +410,6 @@ EXPORT_SYMBOL(phy_start_aneg);
static void phy_change(struct work_struct *work);
-static void phy_state_machine(struct work_struct *work);
/**
* phy_start_machine - start PHY state machine tracking
@@ -430,7 +429,6 @@ void phy_start_machine(struct phy_device *phydev,
{
phydev->adjust_state = handler;
- INIT_DELAYED_WORK(&phydev->state_queue, phy_state_machine);
schedule_delayed_work(&phydev->state_queue, HZ);
}
@@ -761,7 +759,7 @@ EXPORT_SYMBOL(phy_start);
* phy_state_machine - Handle the state machine
* @work: work_struct that describes the work to be done
*/
-static void phy_state_machine(struct work_struct *work)
+void phy_state_machine(struct work_struct *work)
{
struct delayed_work *dwork = to_delayed_work(work);
struct phy_device *phydev =
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8212b2b..adbc0fd 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -177,6 +177,7 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
dev->state = PHY_DOWN;
mutex_init(&dev->lock);
+ INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
return dev;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7968def..6a7eb40 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -485,6 +485,7 @@ void phy_driver_unregister(struct phy_driver *drv);
int phy_driver_register(struct phy_driver *new_driver);
void phy_prepare_link(struct phy_device *phydev,
void (*adjust_link)(struct net_device *));
+void phy_state_machine(struct work_struct *work);
void phy_start_machine(struct phy_device *phydev,
void (*handler)(struct net_device *));
void phy_stop_machine(struct phy_device *phydev);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: phylib: kernel oops on resume with 2.6.33-rc4
2010-01-16 22:37 ` Anton Vorontsov
@ 2010-01-17 8:28 ` Nori, Sekhar
2010-01-18 3:23 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Nori, Sekhar @ 2010-01-17 8:28 UTC (permalink / raw)
To: avorontsov@ru.mvista.com; +Cc: netdev@vger.kernel.org
On Sun, Jan 17, 2010 at 04:07:03, Anton Vorontsov wrote:
> Hello,
>
> On Sat, Jan 16, 2010 at 09:38:34PM +0530, Nori, Sekhar wrote:
> > Hello all,
> >
> > I am hitting a kernel oops when resuming from suspend-to-RAM on 2.6.33-rc4
> > kernel.
> >
> > The MAC is TI DaVinci EMAC and PHY is SMSC LAN8710. However, I also hit the
> > issue even when using generic PHY driver (CONFIG_SMSC_PHY turned off).
> >
> > On reverting commit 541cd3e "phylib: Fix deadlock on resume" which was
> > introduced in 2.6.33-rc4, the issue doesn't come about. So far, I haven't
> > figured out how this particular commit is causing the oops. Full oops report
> > is attached. Appreciate any help in debugging this.
>
> It seems phy_start_machine() needlessly reinitialized work
> struct. This is OK for ucc_geth and gianfar driver because
> these drivers do not detach PHYs on suspend, but TI davinci
> does. :-/
>
> Can you try the patch down below?
This seems to have fixed the problem. Thanks! I hope this can be
included for the next rc. FWIW,
Reported-and-Tested-by: Sekhar Nori <nsekhar@ti.com>
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: phylib: kernel oops on resume with 2.6.33-rc4
2010-01-17 8:28 ` Nori, Sekhar
@ 2010-01-18 3:23 ` David Miller
2010-01-18 15:37 ` [PATCH] phylib: Move workqueue initialization to a proper place Anton Vorontsov
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2010-01-18 3:23 UTC (permalink / raw)
To: nsekhar; +Cc: avorontsov, netdev
From: "Nori, Sekhar" <nsekhar@ti.com>
Date: Sun, 17 Jan 2010 13:58:39 +0530
> On Sun, Jan 17, 2010 at 04:07:03, Anton Vorontsov wrote:
>> Hello,
>>
>> On Sat, Jan 16, 2010 at 09:38:34PM +0530, Nori, Sekhar wrote:
>> > Hello all,
>> >
>> > I am hitting a kernel oops when resuming from suspend-to-RAM on 2.6.33-rc4
>> > kernel.
>> >
>> > The MAC is TI DaVinci EMAC and PHY is SMSC LAN8710. However, I also hit the
>> > issue even when using generic PHY driver (CONFIG_SMSC_PHY turned off).
>> >
>> > On reverting commit 541cd3e "phylib: Fix deadlock on resume" which was
>> > introduced in 2.6.33-rc4, the issue doesn't come about. So far, I haven't
>> > figured out how this particular commit is causing the oops. Full oops report
>> > is attached. Appreciate any help in debugging this.
>>
>> It seems phy_start_machine() needlessly reinitialized work
>> struct. This is OK for ucc_geth and gianfar driver because
>> these drivers do not detach PHYs on suspend, but TI davinci
>> does. :-/
>>
>> Can you try the patch down below?
>
> This seems to have fixed the problem. Thanks! I hope this can be
> included for the next rc. FWIW,
>
> Reported-and-Tested-by: Sekhar Nori <nsekhar@ti.com>
Anton, please resubmit with your signoff.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] phylib: Move workqueue initialization to a proper place
2010-01-18 3:23 ` David Miller
@ 2010-01-18 15:37 ` Anton Vorontsov
2010-01-19 10:00 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Anton Vorontsov @ 2010-01-18 15:37 UTC (permalink / raw)
To: David Miller; +Cc: nsekhar, netdev
commit 541cd3ee00a4fe975b22fac6a3bc846bacef37f7 ("phylib: Fix deadlock
on resume") caused TI DaVinci EMAC ethernet driver to oops upon resume:
PM: resume of devices complete after 237.098 msecs
Restarting tasks ... done.
kernel BUG at kernel/workqueue.c:354!
Unable to handle kernel NULL pointer dereference at virtual address 00000000
[...]
Backtrace:
[<c002c598>] (__bug+0x0/0x2c) from [<c0052a54>] (queue_delayed_work_on+0x74/0xf8)
[<c00529e0>] (queue_delayed_work_on+0x0/0xf8) from [<c0052b30>] (queue_delayed_work+0x2c/0x30)
The oops pops up because TI DaVinci EMAC driver detaches PHY on
suspend and attaches it back on resume. Attaching makes phylib call
phy_start_machine() that initializes a workqueue. On the other hand,
PHY's resume routine will call phy_start_machine() again, and that
will cause the oops since we just destroyed the already scheduled
workqueue.
This patch fixes the issue by moving workqueue initialization to
phy_device_create().
p.s. We don't see this oops with ucc_geth and gianfar drivers because
they perform a fine-grained suspend, i.e. they just stop the PHYs
without detaching.
Reported-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Tested-by: Sekhar Nori <nsekhar@ti.com>
---
On Sun, Jan 17, 2010 at 07:23:53PM -0800, David Miller wrote:
[...]
> >> It seems phy_start_machine() needlessly reinitialized work
> >> struct. This is OK for ucc_geth and gianfar driver because
> >> these drivers do not detach PHYs on suspend, but TI davinci
> >> does. :-/
> >>
> >> Can you try the patch down below?
> >
> > This seems to have fixed the problem. Thanks! I hope this can be
> > included for the next rc. FWIW,
> >
> > Reported-and-Tested-by: Sekhar Nori <nsekhar@ti.com>
>
> Anton, please resubmit with your signoff.
Here it is. Thanks!
drivers/net/phy/phy.c | 4 +---
drivers/net/phy/phy_device.c | 1 +
include/linux/phy.h | 1 +
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index b0e9f9c..0295097 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -410,7 +410,6 @@ EXPORT_SYMBOL(phy_start_aneg);
static void phy_change(struct work_struct *work);
-static void phy_state_machine(struct work_struct *work);
/**
* phy_start_machine - start PHY state machine tracking
@@ -430,7 +429,6 @@ void phy_start_machine(struct phy_device *phydev,
{
phydev->adjust_state = handler;
- INIT_DELAYED_WORK(&phydev->state_queue, phy_state_machine);
schedule_delayed_work(&phydev->state_queue, HZ);
}
@@ -761,7 +759,7 @@ EXPORT_SYMBOL(phy_start);
* phy_state_machine - Handle the state machine
* @work: work_struct that describes the work to be done
*/
-static void phy_state_machine(struct work_struct *work)
+void phy_state_machine(struct work_struct *work)
{
struct delayed_work *dwork = to_delayed_work(work);
struct phy_device *phydev =
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8212b2b..adbc0fd 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -177,6 +177,7 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
dev->state = PHY_DOWN;
mutex_init(&dev->lock);
+ INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
return dev;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7968def..6a7eb40 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -485,6 +485,7 @@ void phy_driver_unregister(struct phy_driver *drv);
int phy_driver_register(struct phy_driver *new_driver);
void phy_prepare_link(struct phy_device *phydev,
void (*adjust_link)(struct net_device *));
+void phy_state_machine(struct work_struct *work);
void phy_start_machine(struct phy_device *phydev,
void (*handler)(struct net_device *));
void phy_stop_machine(struct phy_device *phydev);
--
1.6.5.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] phylib: Move workqueue initialization to a proper place
2010-01-18 15:37 ` [PATCH] phylib: Move workqueue initialization to a proper place Anton Vorontsov
@ 2010-01-19 10:00 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-01-19 10:00 UTC (permalink / raw)
To: avorontsov; +Cc: nsekhar, netdev
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Mon, 18 Jan 2010 18:37:16 +0300
> commit 541cd3ee00a4fe975b22fac6a3bc846bacef37f7 ("phylib: Fix deadlock
> on resume") caused TI DaVinci EMAC ethernet driver to oops upon resume:
...
> The oops pops up because TI DaVinci EMAC driver detaches PHY on
> suspend and attaches it back on resume. Attaching makes phylib call
> phy_start_machine() that initializes a workqueue. On the other hand,
> PHY's resume routine will call phy_start_machine() again, and that
> will cause the oops since we just destroyed the already scheduled
> workqueue.
>
> This patch fixes the issue by moving workqueue initialization to
> phy_device_create().
>
> p.s. We don't see this oops with ucc_geth and gianfar drivers because
> they perform a fine-grained suspend, i.e. they just stop the PHYs
> without detaching.
>
> Reported-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Tested-by: Sekhar Nori <nsekhar@ti.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-19 10:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-16 16:08 phylib: kernel oops on resume with 2.6.33-rc4 Nori, Sekhar
2010-01-16 22:37 ` Anton Vorontsov
2010-01-17 8:28 ` Nori, Sekhar
2010-01-18 3:23 ` David Miller
2010-01-18 15:37 ` [PATCH] phylib: Move workqueue initialization to a proper place Anton Vorontsov
2010-01-19 10:00 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).