* 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).