All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19  9:22 [PATCH] usb otg: use atomic notifier instead of blocking notifier Yang Ruirui
@ 2011-01-19  9:19 ` Dave Young
  2011-01-20  6:46 ` Felipe Balbi
  1 sibling, 0 replies; 14+ messages in thread
From: Dave Young @ 2011-01-19  9:19 UTC (permalink / raw)
  To: Yang Ruirui
  Cc: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Heikki Krogerus, Tejun Heo, Samuel Ortiz, Hema HK, linux-usb,
	linux-kernel

On Wed, Jan 19, 2011 at 5:22 PM, Yang Ruirui <ruirui.r.yang@tieto.com> 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@tieto.com>
> ---
>  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@vger.kernel.org
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] usb otg: use atomic notifier instead of blocking notifier
@ 2011-01-19  9:22 Yang Ruirui
  2011-01-19  9:19 ` Dave Young
  2011-01-20  6:46 ` Felipe Balbi
  0 siblings, 2 replies; 14+ messages in thread
From: Yang Ruirui @ 2011-01-19  9:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Heikki Krogerus, Tejun Heo, Samuel Ortiz, Hema HK, linux-usb,
	linux-kernel


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@tieto.com>
---
 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,

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19  9:22 [PATCH] usb otg: use atomic notifier instead of blocking notifier Yang Ruirui
  2011-01-19  9:19 ` Dave Young
@ 2011-01-20  6:46 ` Felipe Balbi
  1 sibling, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2011-01-20  6:46 UTC (permalink / raw)
  To: Yang Ruirui
  Cc: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Heikki Krogerus, Tejun Heo, Samuel Ortiz, Hema HK, linux-usb,
	linux-kernel

On Wed, Jan 19, 2011 at 05:22:35PM +0800, Yang Ruirui 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@tieto.com>

for twlx030-usb.c parts:

Acked-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19 10:33 ` Heikki Krogerus
@ 2011-01-21 15:34   ` Grazvydas Ignotas
  0 siblings, 0 replies; 14+ messages in thread
From: Grazvydas Ignotas @ 2011-01-21 15:34 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Felipe Balbi, Tejun Heo, Samuel Ortiz, Hema HK, linux-usb,
	linux-kernel, Yang Ruirui, David Brownell

On Wed, Jan 19, 2011 at 12:33 PM, Heikki Krogerus
<heikki.krogerus@nokia.com> wrote:
> On Wed, Jan 19, 2011 at 05:20:11PM +0800, ext Yang Ruirui wrote:
>> --- 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) */
>
> Is drivers/power/twl4030_charger.c ready for atomic notifiers?

No it is not, it's doing some i2c stuff. With twl4030 it's called from
threaded_irq in twl4030-usb instead of musb core and is working fine..

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19  9:20 Yang Ruirui
                   ` (2 preceding siblings ...)
  2011-01-19 15:18 ` Ming Lei
@ 2011-01-20 15:51 ` Mian Yousaf Kaukab
  3 siblings, 0 replies; 14+ messages in thread
From: Mian Yousaf Kaukab @ 2011-01-20 15:51 UTC (permalink / raw)
  To: Yang Ruirui
  Cc: Greg Kroah-Hartman, Linus WALLEIJ, Felipe Balbi, Heikki Krogerus,
	Tejun Heo, Samuel Ortiz, Hema HK, linux-usb, linux-kernel

On 01/19/2011 10:20 AM, Yang Ruirui 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@tieto.com>
>    

for ab8500-usb.c parts:

Acked-by: Mian Yousaf Kaukab<mian.yousaf.kaukab@stericsson.com>


--
Mian Yousaf Kaukab



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-20  2:06   ` Yang Ruirui
@ 2011-01-20  4:01     ` Felipe Balbi
  0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2011-01-20  4:01 UTC (permalink / raw)
  To: Yang Ruirui
  Cc: Felipe Balbi, Yang Ruirui R, Greg Kroah-Hartman,
	Mian Yousaf Kaukab, Linus Walleij, Felipe Balbi, Heikki Krogerus,
	Tejun Heo, Samuel Ortiz, Hema HK, linux-usb, linux-kernel

On Thu, Jan 20, 2011 at 10:06:38AM +0800, Yang Ruirui wrote:
> Actually I does not found where notifiers are used in these files except
> langwell_otg
> 
> For langwell_otg.c there's a wrapper struct named "intel mid_otg_xceiv"
> in include/linux/usb/intel_mid_otg.h, it use an atomic notifier for
> interrupt context.
> 
> So I think we'd better write another patch to make it use general
> otg_transceiver for notifier or just keep current code. What's your opinion?

I'm not the maintainer but I would rather see it using the notifier on
the otg_transceiver structure, and later add the support for the other
transceivers too.

-- 
balbi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19 10:40 ` Felipe Balbi
@ 2011-01-20  2:06   ` Yang Ruirui
  2011-01-20  4:01     ` Felipe Balbi
  0 siblings, 1 reply; 14+ messages in thread
From: Yang Ruirui @ 2011-01-20  2:06 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Yang Ruirui R, Greg Kroah-Hartman, Mian Yousaf Kaukab,
	Linus Walleij, Felipe Balbi, Heikki Krogerus, Tejun Heo,
	Samuel Ortiz, Hema HK, linux-usb, linux-kernel

On Wed, Jan 19, 2011 at 12:40:32PM +0200, Felipe Balbi wrote:
> hi,
> 
> On Wed, Jan 19, 2011 at 05:20:11PM +0800, Yang Ruirui wrote:
> > 
> > following bug happens with meego 2.6.35 kernel on nokia n900:
> 
> <snip>
> 
> > 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@tieto.com>
> > ---
> >  drivers/usb/otg/ab8500-usb.c  |    6 +++---
> >  drivers/usb/otg/twl4030-usb.c |    6 +++---
> >  drivers/usb/otg/twl6030-usb.c |    6 +++---
> 
> you're missing the other transceivers:
> drivers/usb/otg/langwell_otg.c
> drivers/usb/otg/msm72k_otg.c
> drivers/usb/otg/gpio_vbus.c
> drivers/usb/otg/nop-usb-xceiv.c
> drivers/usb/otg/ulpi.c
> drivers/usb/otg/isp1301_omap.c

Hi, thanks

Actually I does not found where notifiers are used in these files except
langwell_otg

For langwell_otg.c there's a wrapper struct named "intel mid_otg_xceiv"
in include/linux/usb/intel_mid_otg.h, it use an atomic notifier for
interrupt context.

So I think we'd better write another patch to make it use general
otg_transceiver for notifier or just keep current code. What's your opinion?

> 
> -- 
> balbi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19 15:38     ` Ming Lei
@ 2011-01-19 16:01       ` Felipe Balbi
  0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2011-01-19 16:01 UTC (permalink / raw)
  To: Ming Lei
  Cc: balbi, Yang Ruirui, Greg Kroah-Hartman, Mian Yousaf Kaukab,
	Linus Walleij, Felipe Balbi, Heikki Krogerus, Tejun Heo,
	Samuel Ortiz, Hema HK, linux-usb, linux-kernel

On Wed, Jan 19, 2011 at 11:38:44PM +0800, Ming Lei wrote:
> Hi,
> 
> 2011/1/19 Felipe Balbi <balbi@ti.com>:
> 
> > that's easy to guess since commit log says it's a problem on Meego
> > kernel.
> 
> OK, it is a specific issue on Meego kernel, so it is better to add
> "Meego " in the patch title.
> 
> >
> >> Are you sure the issue can be reproduced in 2.6.37 or -next tree?
> >>
> >> In mainline code(2.6.37), blocking_notifier_call_chain is only called in
> >> threaded irq handler of otg driver wrt. musb driver, so seems enough.
> >
> > true, but seems like Nokia is using to notify gadget states, which makes
> > a lot of sense when you start thinking about things such as battery
> > charging.
> 
> Please go ahead to submit and merge it into mainline first if it makes
> sense, :-)

isn't it what his patch is already intending to do ? Or do you want them
to send the initial buggy version and only afterwards send the bugfix ?
A bit weird decision wouldn't that be ?

-- 
balbi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19 15:30   ` Felipe Balbi
@ 2011-01-19 15:38     ` Ming Lei
  2011-01-19 16:01       ` Felipe Balbi
  0 siblings, 1 reply; 14+ messages in thread
From: Ming Lei @ 2011-01-19 15:38 UTC (permalink / raw)
  To: balbi
  Cc: Yang Ruirui, Greg Kroah-Hartman, Mian Yousaf Kaukab,
	Linus Walleij, Felipe Balbi, Heikki Krogerus, Tejun Heo,
	Samuel Ortiz, Hema HK, linux-usb, linux-kernel

Hi,

2011/1/19 Felipe Balbi <balbi@ti.com>:

> that's easy to guess since commit log says it's a problem on Meego
> kernel.

OK, it is a specific issue on Meego kernel, so it is better to add
"Meego " in the patch title.

>
>> Are you sure the issue can be reproduced in 2.6.37 or -next tree?
>>
>> In mainline code(2.6.37), blocking_notifier_call_chain is only called in
>> threaded irq handler of otg driver wrt. musb driver, so seems enough.
>
> true, but seems like Nokia is using to notify gadget states, which makes
> a lot of sense when you start thinking about things such as battery
> charging.

Please go ahead to submit and merge it into mainline first if it makes
sense, :-)

thanks,
-- 
Lei Ming

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19 15:18 ` Ming Lei
@ 2011-01-19 15:30   ` Felipe Balbi
  2011-01-19 15:38     ` Ming Lei
  0 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2011-01-19 15:30 UTC (permalink / raw)
  To: Ming Lei
  Cc: Yang Ruirui, Greg Kroah-Hartman, Mian Yousaf Kaukab,
	Linus Walleij, Felipe Balbi, Heikki Krogerus, Tejun Heo,
	Samuel Ortiz, Hema HK, linux-usb, linux-kernel

Hi,

On Wed, Jan 19, 2011 at 11:18:49PM +0800, Ming Lei wrote:
> 2011/1/19 Yang Ruirui <ruirui.r.yang@tieto.com>:
> > [   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)
> 
> I am a little confused, why is blocking_notifier_call_chain called from
> musb_gadget_vbus_draw?  See source code of musb_gadget_vbus_draw:
> 
> 1587 static int musb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
> 1588 {
> 1589     struct musb *musb = gadget_to_musb(gadget);
> 1590
> 1591     if (!musb->xceiv->set_power)
> 1592         return -EOPNOTSUPP;
> 1593     return otg_set_power(musb->xceiv, mA);
> 1594 }

that's easy to guess since commit log says it's a problem on Meego
kernel.

> Are you sure the issue can be reproduced in 2.6.37 or -next tree?
> 
> In mainline code(2.6.37), blocking_notifier_call_chain is only called in
> threaded irq handler of otg driver wrt. musb driver, so seems enough.

true, but seems like Nokia is using to notify gadget states, which makes
a lot of sense when you start thinking about things such as battery
charging.

-- 
balbi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19  9:20 Yang Ruirui
  2011-01-19 10:33 ` Heikki Krogerus
  2011-01-19 10:40 ` Felipe Balbi
@ 2011-01-19 15:18 ` Ming Lei
  2011-01-19 15:30   ` Felipe Balbi
  2011-01-20 15:51 ` Mian Yousaf Kaukab
  3 siblings, 1 reply; 14+ messages in thread
From: Ming Lei @ 2011-01-19 15:18 UTC (permalink / raw)
  To: Yang Ruirui
  Cc: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Felipe Balbi, Heikki Krogerus, Tejun Heo, Samuel Ortiz, Hema HK,
	linux-usb, linux-kernel

Hi Yang,

2011/1/19 Yang Ruirui <ruirui.r.yang@tieto.com>:
> [   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)

I am a little confused, why is blocking_notifier_call_chain called from
musb_gadget_vbus_draw?  See source code of musb_gadget_vbus_draw:

1587 static int musb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
1588 {
1589     struct musb *musb = gadget_to_musb(gadget);
1590
1591     if (!musb->xceiv->set_power)
1592         return -EOPNOTSUPP;
1593     return otg_set_power(musb->xceiv, mA);
1594 }

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

Are you sure the issue can be reproduced in 2.6.37 or -next tree?

In mainline code(2.6.37), blocking_notifier_call_chain is only called in
threaded irq handler of otg driver wrt. musb driver, so seems enough.

thanks,
-- 
Lei Ming

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19  9:20 Yang Ruirui
  2011-01-19 10:33 ` Heikki Krogerus
@ 2011-01-19 10:40 ` Felipe Balbi
  2011-01-20  2:06   ` Yang Ruirui
  2011-01-19 15:18 ` Ming Lei
  2011-01-20 15:51 ` Mian Yousaf Kaukab
  3 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2011-01-19 10:40 UTC (permalink / raw)
  To: Yang Ruirui
  Cc: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Felipe Balbi, Heikki Krogerus, Tejun Heo, Samuel Ortiz, Hema HK,
	linux-usb, linux-kernel

hi,

On Wed, Jan 19, 2011 at 05:20:11PM +0800, Yang Ruirui wrote:
> 
> following bug happens with meego 2.6.35 kernel on nokia n900:

<snip>

> 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@tieto.com>
> ---
>  drivers/usb/otg/ab8500-usb.c  |    6 +++---
>  drivers/usb/otg/twl4030-usb.c |    6 +++---
>  drivers/usb/otg/twl6030-usb.c |    6 +++---

you're missing the other transceivers:
drivers/usb/otg/langwell_otg.c
drivers/usb/otg/msm72k_otg.c
drivers/usb/otg/gpio_vbus.c
drivers/usb/otg/nop-usb-xceiv.c
drivers/usb/otg/ulpi.c
drivers/usb/otg/isp1301_omap.c

-- 
balbi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb otg: use atomic notifier instead of blocking notifier
  2011-01-19  9:20 Yang Ruirui
@ 2011-01-19 10:33 ` Heikki Krogerus
  2011-01-21 15:34   ` Grazvydas Ignotas
  2011-01-19 10:40 ` Felipe Balbi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Heikki Krogerus @ 2011-01-19 10:33 UTC (permalink / raw)
  To: Grazvydas Ignotas
  Cc: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Felipe Balbi, Tejun Heo, Samuel Ortiz, Hema HK, linux-usb,
	linux-kernel, Yang Ruirui, David Brownell

On Wed, Jan 19, 2011 at 05:20:11PM +0800, ext Yang Ruirui wrote:
> --- 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) */

Is drivers/power/twl4030_charger.c ready for atomic notifiers?

-- 
heikki

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] usb otg: use atomic notifier instead of blocking notifier
@ 2011-01-19  9:20 Yang Ruirui
  2011-01-19 10:33 ` Heikki Krogerus
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Yang Ruirui @ 2011-01-19  9:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Mian Yousaf Kaukab, Linus Walleij,
	Felipe Balbi, Heikki Krogerus, Tejun Heo, Samuel Ortiz, Hema HK,
	linux-usb, linux-kernel


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@tieto.com>
---
 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,

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-01-21 15:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-19  9:22 [PATCH] usb otg: use atomic notifier instead of blocking notifier Yang Ruirui
2011-01-19  9:19 ` Dave Young
2011-01-20  6:46 ` Felipe Balbi
  -- strict thread matches above, loose matches on Subject: below --
2011-01-19  9:20 Yang Ruirui
2011-01-19 10:33 ` Heikki Krogerus
2011-01-21 15:34   ` Grazvydas Ignotas
2011-01-19 10:40 ` Felipe Balbi
2011-01-20  2:06   ` Yang Ruirui
2011-01-20  4:01     ` Felipe Balbi
2011-01-19 15:18 ` Ming Lei
2011-01-19 15:30   ` Felipe Balbi
2011-01-19 15:38     ` Ming Lei
2011-01-19 16:01       ` Felipe Balbi
2011-01-20 15:51 ` Mian Yousaf Kaukab

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.