On Tue, Sep 01, Alan Stern wrote:
> On Tue, 1 Sep 2009, Jan Blunck wrote:
>
> > On Tue, Sep 01, Kay Sievers wrote:
> >
> > > On Tue, 2009-09-01 at 15:21 +0200, Jan Blunck wrote:
> > > > I get this when I enable lockdep:
> > > >
> > > > [ 10.658753] =============================================
> > > > [ 10.659008] [ INFO: possible recursive locking detected ]
> > > > [ 10.659008] 2.6.31-rc8-rt9-rt_trace #5
> > > > [ 10.659008] ---------------------------------------------
> > > > [ 10.659008] swapper/1 is trying to acquire lock:
> > > > [ 10.659008] (&dev->mutex){+.+...}, at: [<ffffffff813519fd>] __driver_attach+0x7d/0xc0
> > > > [ 10.659008]
> > > > [ 10.659008] but task is already holding lock:
> > > > [ 10.659008] (&dev->mutex){+.+...}, at: [<ffffffff813519f1>] __driver_attach+0x71/0xc0
> > > > [ 10.659008]
> > > > [ 10.659008] other info that might help us debug this:
> > > > [ 10.659008] 1 lock held by swapper/1:
> > > > [ 10.659008] #0: (&dev->mutex){+.+...}, at: [<ffffffff813519f1>] __driver_attach+0x71/0xc0
> > > > [ 10.659008]
> > > > [ 10.659008] stack backtrace:
> > > > [ 10.659008] Pid: 1, comm: swapper Not tainted 2.6.31-rc8-rt9-rt_trace #5
> > > > [ 10.659008] Call Trace:
> > > > [ 10.659008] [<ffffffff810119c1>] try_stack_unwind+0x171/0x1c0
> > > > [ 10.659008] [<ffffffff81010241>] dump_trace+0xb1/0x300
> > > > [ 10.659008] [<ffffffff81011489>] show_trace_log_lvl+0x69/0xa0
> > > > [ 10.659008] [<ffffffff810114e8>] show_trace+0x28/0x50
> > > > [ 10.659008] [<ffffffff814d60f3>] dump_stack+0x86/0xa3
> > > > [ 10.659008] [<ffffffff810b1171>] __lock_acquire+0x1391/0x13e0
> > > > [ 10.659008] [<ffffffff810b12e7>] lock_acquire+0x127/0x190
> > > > [ 10.659008] [<ffffffff814da2e3>] _mutex_lock+0x43/0x70
> > > > [ 10.659008] [<ffffffff813519fd>] __driver_attach+0x7d/0xc0
> > > > [ 10.659008] [<ffffffff81350abb>] bus_for_each_dev+0x7b/0xc0
> > > > [ 10.659008] [<ffffffff81351454>] driver_attach+0x34/0x50
> > > > [ 10.659008] [<ffffffff813500c8>] bus_add_driver+0x2a8/0x360
> > > > [ 10.659008] [<ffffffff81351e94>] driver_register+0x94/0x190
> > > > [ 10.659008] [<ffffffff812db610>] acpi_bus_register_driver+0x56/0x6e
> > > > [ 10.659008] [<ffffffff81acf075>] acpi_pci_root_init+0x2c/0x4f
> > > > [ 10.659008] [<ffffffff8100908b>] do_one_initcall+0x4b/0x1e0
> > > > [ 10.659008] [<ffffffff81a95665>] kernel_init+0x210/0x370
> > > > [ 10.659008] [<ffffffff8100d9ba>] child_rip+0xa/0x20
> > > >
> > > > The code leading to is is from __driver_attach():
> > > >
> > > > if (!driver_match_device(drv, dev))
> > > > return 0;
> > > >
> > > > if (dev->parent) /* Needed for USB */
> > > > mutex_lock(&dev->parent->mutex);
> > > > mutex_lock(&dev->mutex);
> > > > if (!dev->driver)
> > > > driver_probe_device(drv, dev);
> > > > mutex_unlock(&dev->mutex);
> > > > if (dev->parent)
> > > > mutex_unlock(&dev->parent->mutex);
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > I think that you need to use mutex_lock_nested() and teach lockdep the
> > > > parent/child relation of the mutexs in this case (e.g. like it is done in
> > > > fs/namei.c).
> > > >
> > > > What do you think?
> > >
> > > Would be good to have Alan Stern having a look at this, he had a few
> > > things with the nested locks recently. Maybe Cc: him and linux-usb?
> > >
> >
> > Ok, will do that.
>
> This is semaphore->mutex conversion madness from tglx. What he tried
> to do just doesn't work with lockdep.
>
If this is a parent->child relationship and the parent is always locked before
the child this works perfectly with lockdep. The inode->i_mutex is doing
it. How is the lock in your code different from that?
Thanks,
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
