Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier

[Dave Young]

On Wed, Jan 19, 2011 at 5:22 PM, Yang Ruirui <ruirui.r.yang@xxxxxxxxx> wrote:
>
> following bug happens with meego 2.6.35 kernel on nokia n900:
>
> [ Â 28.693756] BUG: sleeping function called from invalid context at kernel/rwsem.c:21
> [ Â 28.693786] in_atomic(): 1, irqs_disabled(): 128, pid: 706, name: udisks-part-id
> [ Â 28.693817] 1 lock held by udisks-part-id/706:
> [ Â 28.693817] Â#0: Â(&(&musb->lock)->rlock){-.-...}, at: [<c0278590>] musb_g_disconnect+0x90/0x148
> [ Â 28.693908] irq event stamp: 1169
> [ Â 28.693908] hardirqs last Âenabled at (1168): [<c00c4b3c>] kmem_cache_alloc+0xd0/0x128
> [ Â 28.693969] hardirqs last disabled at (1169): [<c002da34>] __irq_svc+0x34/0xb4
> [ Â 28.694000] softirqs last Âenabled at (0): [<c0054bcc>] copy_process+0x304/0xe18
> [ Â 28.694030] softirqs last disabled at (0): [<(null)>] (null)
> [ Â 28.694091] [<c00326a0>] (unwind_backtrace+0x0/0xec) from [<c037e6b0>] (down_read+0x20/0x5c)
> [ Â 28.694152] [<c037e6b0>] (down_read+0x20/0x5c) from [<c0070770>] (__blocking_notifier_call_chain+0x2c/0x5c)
> [ Â 28.694183] [<c0070770>] (__blocking_notifier_call_chain+0x2c/0x5c) from [<c00707b4>] (blocking_notifier_call_chain+0x14/0x18)
> [ Â 28.694213] [<c00707b4>] (blocking_notifier_call_chain+0x14/0x18) from [<c02784d8>] (musb_gadget_vbus_draw+0x38/0x60)
> [ Â 28.694274] [<c02784d8>] (musb_gadget_vbus_draw+0x38/0x60) from [<c0276d10>] (musb_interrupt+0xb08/0xcb0)
> [ Â 28.694305] [<c0276d10>] (musb_interrupt+0xb08/0xcb0) from [<c0276f08>] (generic_interrupt+0x50/0x68)
> [ Â 28.694335] [<c0276f08>] (generic_interrupt+0x50/0x68) from [<c008fcf4>] (handle_IRQ_event+0x24/0xe8)
> [ Â 28.694396] [<c008fcf4>] (handle_IRQ_event+0x24/0xe8) from [<c0091924>] (handle_level_irq+0xac/0x128)
> [ Â 28.694427] [<c0091924>] (handle_level_irq+0xac/0x128) from [<c002d070>] (asm_do_IRQ+0x70/0x90)
> [ Â 28.694458] [<c002d070>] (asm_do_IRQ+0x70/0x90) from [<c002da4c>] (__irq_svc+0x4c/0xb4)
> [ Â 28.694488] Exception stack(0xcd835ed0 to 0xcd835f18)
> [ Â 28.694519] 5ec0: Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cfc0a240 c8282000 0000006b 0000006b
> [ Â 28.694549] 5ee0: cfc0a240 0000041a 00001000 00000000 c8282000 cd834000 c8282000 be80c464
> [ Â 28.694580] 5f00: 00000c1e cd835f18 c00c333c c00c297c 80000013 ffffffff
> [ Â 28.694610] [<c002da4c>] (__irq_svc+0x4c/0xb4) from [<c00c297c>] (check_poison_obj+0x24/0x194)
> [ Â 28.694641] [<c00c297c>] (check_poison_obj+0x24/0x194) from [<c00c333c>] (cache_alloc_debugcheck_after+0x28/0x188)
> [ Â 28.694671] [<c00c333c>] (cache_alloc_debugcheck_after+0x28/0x188) from [<c00c4b54>] (kmem_cache_alloc+0xe8/0x128)
> [ Â 28.694732] [<c00c4b54>] (kmem_cache_alloc+0xe8/0x128) from [<c00d79dc>] (getname+0x18/0xcc)
> [ Â 28.694763] [<c00d79dc>] (getname+0x18/0xcc) from [<c00cbca8>] (do_sys_open+0x18/0x10c)
> [ Â 28.694793] [<c00cbca8>] (do_sys_open+0x18/0x10c) from [<c002df40>] (ret_fast_syscall+0x0/0x3c)
> [ Â 28.694854]
> [ Â 28.694854] =================================
> [ Â 28.709014] [ INFO: inconsistent lock state ]
> [ Â 28.717254] 2.6.35.96.5-adaptation-n900 #1
> [ Â 28.725128] ---------------------------------
> [ Â 28.733123] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-R} usage.
> [ Â 28.742980] udisks-part-id/706 [HC1[1]:SC0[0]:HE0:SE1] takes:
> [ Â 28.752441] Â(&(&twl->otg.notifier)->rwsem){+-+...}, at: [<c0070770>] __blocking_notifier_call_chain+0x2c/0x5c
> [ Â 28.769836] {HARDIRQ-ON-W} state was registered at:
> [ Â 28.778320] Â [<c007c1dc>] __lock_acquire+0x618/0x1730
> [ Â 28.787109] Â [<c007d354>] lock_acquire+0x60/0x74
> [ Â 28.795227] Â [<c037e67c>] down_write+0x48/0x5c
> [ Â 28.803131] Â [<c00708a8>] blocking_notifier_chain_register+0x30/0x54
> [ Â 28.812896] Â [<bf0da9f8>] isp1704_charger_probe+0x268/0x39c [isp1704_charger]
> [ Â 28.823394] Â [<c0239410>] platform_drv_probe+0x18/0x1c
> [ Â 28.831787] Â [<c02385c8>] driver_probe_device+0xa8/0x158
> [ Â 28.840332] Â [<c02386e0>] __driver_attach+0x68/0x8c
> [ Â 28.848510] Â [<c0237e68>] bus_for_each_dev+0x44/0x74
> [ Â 28.856689] Â [<c02377c8>] bus_add_driver+0x9c/0x20c
> [ Â 28.864746] Â [<c02389b0>] driver_register+0xa8/0x138
> [ Â 28.872955] Â [<c002d340>] do_one_initcall+0x58/0x1ac
> [ Â 28.881134] Â [<c0085cdc>] sys_init_module+0x90/0x1b0
> [ Â 28.889343] Â [<c002df40>] ret_fast_syscall+0x0/0x3c
> [ Â 28.897491] irq event stamp: 1169
> [ Â 28.904083] hardirqs last Âenabled at (1168): [<c00c4b3c>] kmem_cache_alloc+0xd0/0x128
> [ Â 28.915557] hardirqs last disabled at (1169): [<c002da34>] __irq_svc+0x34/0xb4
> [ Â 28.926239] softirqs last Âenabled at (0): [<c0054bcc>] copy_process+0x304/0xe18
> [ Â 28.937194] softirqs last disabled at (0): [<(null)>] (null)
> [ Â 28.946258]
> [ Â 28.946258] other info that might help us debug this:
> [ Â 28.959381] 1 lock held by udisks-part-id/706:
> [ Â 28.967163] Â#0: Â(&(&musb->lock)->rlock){-.-...}, at: [<c0278590>] musb_g_disconnect+0x90/0x148
> [ Â 28.979705]
> [ Â 28.979705] stack backtrace:
> [ Â 28.990997] [<c00326a0>] (unwind_backtrace+0x0/0xec) from [<c007a39c>] (print_usage_bug+0x170/0x1b4)
> [ Â 29.007293] [<c007a39c>] (print_usage_bug+0x170/0x1b4) from [<c007a738>] (mark_lock+0x358/0x628)
> [ Â 29.019836] [<c007a738>] (mark_lock+0x358/0x628) from [<c007c118>] (__lock_acquire+0x554/0x1730)
> [ Â 29.032409] [<c007c118>] (__lock_acquire+0x554/0x1730) from [<c007d354>] (lock_acquire+0x60/0x74)
> [ Â 29.045166] [<c007d354>] (lock_acquire+0x60/0x74) from [<c037e6d8>] (down_read+0x48/0x5c)
> [ Â 29.057250] [<c037e6d8>] (down_read+0x48/0x5c) from [<c0070770>] (__blocking_notifier_call_chain+0x2c/0x5c)
> [ Â 29.074920] [<c0070770>] (__blocking_notifier_call_chain+0x2c/0x5c) from [<c00707b4>] (blocking_notifier_call_chain+0x14/0x18)
> [ Â 29.094573] [<c00707b4>] (blocking_notifier_call_chain+0x14/0x18) from [<c02784d8>] (musb_gadget_vbus_draw+0x38/0x60)
> [ Â 29.113891] [<c02784d8>] (musb_gadget_vbus_draw+0x38/0x60) from [<c0276d10>] (musb_interrupt+0xb08/0xcb0)
> [ Â 29.132629] [<c0276d10>] (musb_interrupt+0xb08/0xcb0) from [<c0276f08>] (generic_interrupt+0x50/0x68)
> [ Â 29.151580] [<c0276f08>] (generic_interrupt+0x50/0x68) from [<c008fcf4>] (handle_IRQ_event+0x24/0xe8)
> [ Â 29.171081] [<c008fcf4>] (handle_IRQ_event+0x24/0xe8) from [<c0091924>] (handle_level_irq+0xac/0x128)
> [ Â 29.190948] [<c0091924>] (handle_level_irq+0xac/0x128) from [<c002d070>] (asm_do_IRQ+0x70/0x90)
> [ Â 29.205291] [<c002d070>] (asm_do_IRQ+0x70/0x90) from [<c002da4c>] (__irq_svc+0x4c/0xb4)
> [ Â 29.218963] Exception stack(0xcd835ed0 to 0xcd835f18)
> [ Â 29.229644] 5ec0: Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cfc0a240 c8282000 0000006b 0000006b
> [ Â 29.243591] 5ee0: cfc0a240 0000041a 00001000 00000000 c8282000 cd834000 c8282000 be80c464
> [ Â 29.257629] 5f00: 00000c1e cd835f18 c00c333c c00c297c 80000013 ffffffff
> [ Â 29.270111] [<c002da4c>] (__irq_svc+0x4c/0xb4) from [<c00c297c>] (check_poison_obj+0x24/0x194)
> [ Â 29.284637] [<c00c297c>] (check_poison_obj+0x24/0x194) from [<c00c333c>] (cache_alloc_debugcheck_after+0x28/0x188)
> [ Â 29.306732] [<c00c333c>] (cache_alloc_debugcheck_after+0x28/0x188) from [<c00c4b54>] (kmem_cache_alloc+0xe8/0x128)
> [ Â 29.329071] [<c00c4b54>] (kmem_cache_alloc+0xe8/0x128) from [<c00d79dc>] (getname+0x18/0xcc)
> [ Â 29.343597] [<c00d79dc>] (getname+0x18/0xcc) from [<c00cbca8>] (do_sys_open+0x18/0x10c)
> [ Â 29.357696] [<c00cbca8>] (do_sys_open+0x18/0x10c) from [<c002df40>] (ret_fast_syscall+0x0/0x3c)
>
> Actually the blocking notifier chain runs in process context, so not fit for use here.
>
> For mainline kernel there's such issue as well.
> Here fix this problem by changing to use atomic_notifier.
>
> Signed-off-by: Yang Ruirui <ruirui.r.yang@xxxxxxxxx>
> ---
> Âdrivers/usb/otg/ab8500-usb.c Â| Â Â6 +++---
> Âdrivers/usb/otg/twl4030-usb.c | Â Â6 +++---
> Âdrivers/usb/otg/twl6030-usb.c | Â Â6 +++---
> Âinclude/linux/usb/otg.h    |  Â6 +++---
> Â4 files changed, 12 insertions(+), 12 deletions(-)
>
> --- linux-2.6.orig/include/linux/usb/otg.h   Â2011-01-17 09:39:11.000000000 +0800
> +++ linux-2.6/include/linux/usb/otg.h  2011-01-19 16:38:06.649546989 +0800
> @@ -74,7 +74,7 @@ struct otg_transceiver {
>    Âvoid __iomem          Â*io_priv;
>
> Â Â Â Â/* for notification of usb_xceiv_events */
> -    struct blocking_notifier_head  notifier;
> +    struct atomic_notifier_head   notifier;
>
> Â Â Â Â/* to pass extra port status to the root hub */
> Â Â Â Âu16 Â Â Â Â Â Â Â Â Â Â port_status;
> @@ -234,13 +234,13 @@ otg_start_srp(struct otg_transceiver *ot
> Âstatic inline int
> Âotg_register_notifier(struct otg_transceiver *otg, struct notifier_block *nb)
> Â{
> - Â Â Â return blocking_notifier_chain_register(&otg->notifier, nb);
> + Â Â Â return atomic_notifier_chain_register(&otg->notifier, nb);
> Â}
>
> Âstatic inline void
> Âotg_unregister_notifier(struct otg_transceiver *otg, struct notifier_block *nb)
> Â{
> - Â Â Â blocking_notifier_chain_unregister(&otg->notifier, nb);
> + Â Â Â atomic_notifier_chain_unregister(&otg->notifier, nb);
> Â}
>
> Â/* for OTG controller drivers (and maybe other stuff) */
> --- linux-2.6.orig/drivers/usb/otg/ab8500-usb.c 2011-01-17 09:39:11.000000000 +0800
> +++ linux-2.6/drivers/usb/otg/ab8500-usb.c   Â2011-01-19 16:50:21.942878632 +0800
> @@ -212,7 +212,7 @@ static int ab8500_usb_link_status_update
> Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â}
>
> - Â Â Â blocking_notifier_call_chain(&ab->otg.notifier, event, v);
> + Â Â Â atomic_notifier_call_chain(&ab->otg.notifier, event, v);
>
> Â Â Â Âreturn 0;
> Â}
> @@ -281,7 +281,7 @@ static int ab8500_usb_set_power(struct o
> Â Â Â Âab->vbus_draw = mA;
>
> Â Â Â Âif (mA)
> - Â Â Â Â Â Â Â blocking_notifier_call_chain(&ab->otg.notifier,
> + Â Â Â Â Â Â Â atomic_notifier_call_chain(&ab->otg.notifier,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂUSB_EVENT_ENUMERATED, ab->otg.gadget);
> Â Â Â Âreturn 0;
> Â}
> @@ -500,7 +500,7 @@ static int __devinit ab8500_usb_probe(st
>
> Â Â Â Âplatform_set_drvdata(pdev, ab);
>
> - Â Â Â BLOCKING_INIT_NOTIFIER_HEAD(&ab->otg.notifier);
> + Â Â Â ATOMIC_INIT_NOTIFIER_HEAD(&ab->otg.notifier);
>
> Â Â Â Â/* v1: Wait for link status to become stable.
> Â Â Â Â * all: Updates form set_host and set_peripheral as they are atomic.
> --- linux-2.6.orig/drivers/usb/otg/twl4030-usb.c    Â2011-01-17 09:39:11.000000000 +0800
> +++ linux-2.6/drivers/usb/otg/twl4030-usb.c   2011-01-19 16:50:05.929545335 +0800
> @@ -512,7 +512,7 @@ static irqreturn_t twl4030_usb_irq(int i
> Â Â Â Â Â Â Â Âelse
> Â Â Â Â Â Â Â Â Â Â Â Âtwl4030_phy_resume(twl);
>
> - Â Â Â Â Â Â Â blocking_notifier_call_chain(&twl->otg.notifier, status,
> + Â Â Â Â Â Â Â atomic_notifier_call_chain(&twl->otg.notifier, status,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âtwl->otg.gadget);
> Â Â Â Â}
> Â Â Â Âsysfs_notify(&twl->dev->kobj, NULL, "vbus");
> @@ -534,7 +534,7 @@ static void twl4030_usb_phy_init(struct
> Â Â Â Â Â Â Â Â Â Â Â Âtwl->asleep = 0;
> Â Â Â Â Â Â Â Â}
>
> - Â Â Â Â Â Â Â blocking_notifier_call_chain(&twl->otg.notifier, status,
> + Â Â Â Â Â Â Â atomic_notifier_call_chain(&twl->otg.notifier, status,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âtwl->otg.gadget);
> Â Â Â Â}
> Â Â Â Âsysfs_notify(&twl->dev->kobj, NULL, "vbus");
> @@ -623,7 +623,7 @@ static int __devinit twl4030_usb_probe(s
> Â Â Â Âif (device_create_file(&pdev->dev, &dev_attr_vbus))
> Â Â Â Â Â Â Â Âdev_warn(&pdev->dev, "could not create sysfs file\n");
>
> - Â Â Â BLOCKING_INIT_NOTIFIER_HEAD(&twl->otg.notifier);
> + Â Â Â ATOMIC_INIT_NOTIFIER_HEAD(&twl->otg.notifier);
>
> Â Â Â Â/* Our job is to use irqs and status from the power module
> Â Â Â Â * to keep the transceiver disabled when nothing's connected.
> --- linux-2.6.orig/drivers/usb/otg/twl6030-usb.c    Â2011-01-17 09:39:11.000000000 +0800
> +++ linux-2.6/drivers/usb/otg/twl6030-usb.c   2011-01-19 16:49:47.112878712 +0800
> @@ -269,7 +269,7 @@ static irqreturn_t twl6030_usb_irq(int i
> Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Âif (status >= 0) {
> Â Â Â Â Â Â Â Â Â Â Â Âtwl->linkstat = status;
> - Â Â Â Â Â Â Â Â Â Â Â blocking_notifier_call_chain(&twl->otg.notifier,
> + Â Â Â Â Â Â Â Â Â Â Â atomic_notifier_call_chain(&twl->otg.notifier,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstatus, twl->otg.gadget);
> Â Â Â Â Â Â Â Â}
> Â Â Â Â}
> @@ -294,7 +294,7 @@ static irqreturn_t twl6030_usbotg_irq(in
> Â Â Â Â Â Â Â Âstatus = USB_EVENT_ID;
> Â Â Â Â Â Â Â Âtwl->otg.default_a = true;
> Â Â Â Â Â Â Â Âtwl->otg.state = OTG_STATE_A_IDLE;
> - Â Â Â Â Â Â Â blocking_notifier_call_chain(&twl->otg.notifier, status,
> + Â Â Â Â Â Â Â atomic_notifier_call_chain(&twl->otg.notifier, status,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âtwl->otg.gadget);
> Â Â Â Â} else Â{
> Â Â Â Â Â Â Â Âtwl6030_writeb(twl, TWL_MODULE_USB, USB_ID_INT_EN_HI_CLR,
> @@ -411,7 +411,7 @@ static int __devinit twl6030_usb_probe(s
> Â Â Â Âif (device_create_file(&pdev->dev, &dev_attr_vbus))
> Â Â Â Â Â Â Â Âdev_warn(&pdev->dev, "could not create sysfs file\n");
>
> - Â Â Â BLOCKING_INIT_NOTIFIER_HEAD(&twl->otg.notifier);
> + Â Â Â ATOMIC_INIT_NOTIFIER_HEAD(&twl->otg.notifier);
>
> Â Â Â Âtwl->irq_enabled = true;
> Â Â Â Âstatus = request_threaded_irq(twl->irq1, NULL, twl6030_usbotg_irq,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
> Please read the FAQ at Âhttp://www.tux.org/lkml/
>

Sorry about send this twice


-- 
Regards
dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/