Running zpool import -f tank: "[ INFO: possible recursive locking detected ]"

[prakashsurya]

When running the zpool import -f tank with CONFIG_LOCK_STAT enabled, I found the following message in the dmesg output:

=============================================
[ INFO: possible recursive locking detected ]
2.6.32-220.1chaos.ch5.x86_64.debug #1
---------------------------------------------
vdev_open/1/8480 is trying to acquire lock:
 (&zio->io_lock){+.+...}, at: [<ffffffffa051fac8>] zio_add_child+0x88/0x150 [zfs]

but task is already holding lock:
 (&zio->io_lock){+.+...}, at: [<ffffffffa051faa2>] zio_add_child+0x62/0x150 [zfs]

other info that might help us debug this:
1 lock held by vdev_open/1/8480:
 #0:  (&zio->io_lock){+.+...}, at: [<ffffffffa051faa2>] zio_add_child+0x62/0x150 [zfs]

stack backtrace:
Pid: 8480, comm: vdev_open/1 Tainted: G        W  ----------------   2.6.32-220.1chaos.ch5.x86_64.debug #1
Call Trace:
 [<ffffffff810b0610>] ? __lock_acquire+0x11c0/0x1570
 [<ffffffff8113a6cb>] ? __alloc_pages_nodemask+0x13b/0xa20
 [<ffffffff81013853>] ? native_sched_clock+0x13/0x60
 [<ffffffff81012d29>] ? sched_clock+0x9/0x10
 [<ffffffff810b0a64>] ? lock_acquire+0xa4/0x120
 [<ffffffffa051fac8>] ? zio_add_child+0x88/0x150 [zfs]
 [<ffffffff810aea33>] ? mark_held_locks+0x73/0xa0
 [<ffffffff815208a7>] ? __mutex_lock_common+0x2b7/0x400
 [<ffffffff81520633>] ? __mutex_lock_common+0x43/0x400
 [<ffffffffa051fac8>] ? zio_add_child+0x88/0x150 [zfs]
 [<ffffffffa051fac8>] ? zio_add_child+0x88/0x150 [zfs]
 [<ffffffff81520af8>] ? mutex_lock_nested+0x48/0x60
 [<ffffffffa051fac8>] ? zio_add_child+0x88/0x150 [zfs]
 [<ffffffffa051ff78>] ? zio_create+0x3e8/0x4b0 [zfs]
 [<ffffffffa05202fe>] ? zio_read_phys+0x6e/0x80 [zfs]
 [<ffffffffa04e1840>] ? vdev_probe_done+0x0/0x270 [zfs]
 [<ffffffffa04e090a>] ? vdev_probe+0x14a/0x250 [zfs]
 [<ffffffffa04e1840>] ? vdev_probe_done+0x0/0x270 [zfs]
 [<ffffffffa04fa4f5>] ? zfs_post_state_change+0x15/0x20 [zfs]
 [<ffffffffa04e0c70>] ? vdev_open+0x260/0x3e0 [zfs]
 [<ffffffffa04e1826>] ? vdev_open_child+0x26/0x40 [zfs]
 [<ffffffffa03c8da5>] ? taskq_thread+0x1e5/0x360 [spl]
 [<ffffffff810636d0>] ? default_wake_function+0x0/0x20
 [<ffffffffa03c8bc0>] ? taskq_thread+0x0/0x360 [spl]
 [<ffffffff81096986>] ? kthread+0x96/0xa0
 [<ffffffff8100c20a>] ? child_rip+0xa/0x20
 [<ffffffff8100bb50>] ? restore_args+0x0/0x30
 [<ffffffff810968f0>] ? kthread+0x0/0xa0
 [<ffffffff8100c200>] ? child_rip+0x0/0x20
SPL: Showing stack for process 11540
Pid: 11540, comm: mount Tainted: G        W  ----------------   2.6.32-220.1chaos.ch5.x86_64.debug #1
Call Trace:
 [<ffffffffa03c35b7>] ? spl_debug_dumpstack+0x27/0x40 [spl]
 [<ffffffffa03c7c0d>] ? kmem_alloc_debug+0x11d/0x130 [spl]
 [<ffffffffa050a8c8>] ? zfs_sb_create+0x48/0x5a0 [zfs]
 [<ffffffffa0526020>] ? zpl_fill_super+0x0/0x20 [zfs]
 [<ffffffffa050c36e>] ? zfs_domount+0x3e/0x350 [zfs]
 [<ffffffff81197457>] ? sget+0x3e7/0x460
 [<ffffffff81196550>] ? set_anon_super+0x0/0x110
 [<ffffffffa0526020>] ? zpl_fill_super+0x0/0x20 [zfs]
 [<ffffffffa052602e>] ? zpl_fill_super+0xe/0x20 [zfs]
 [<ffffffff8119799f>] ? get_sb_nodev+0x5f/0xa0
 [<ffffffffa0526015>] ? zpl_get_sb+0x25/0x30 [zfs]
 [<ffffffff811975fb>] ? vfs_kern_mount+0x7b/0x1b0
 [<ffffffff811977a2>] ? do_kern_mount+0x52/0x130
 [<ffffffff811b6402>] ? do_mount+0x2d2/0x8c0
 [<ffffffff811b3de2>] ? copy_mount_options+0xf2/0x1a0
 [<ffffffff811b6a80>] ? sys_mount+0x90/0xe0
 [<ffffffff8100b0b2>] ? system_call_fastpath+0x16/0x1b
lt-zpool used greatest stack depth: 1632 bytes left

[behlendorf]

Right, this is one of those false positives we were talking about. The issue here is that the zfs code hasn't yet been integrated with the Linux lock dependency checking code to suppress all these false positives. In this case, we really need to be using mutex_lock_nested() under the covers in the spl instead of just mutex_lock(). I've already put in place some infrastructure for this, but more is probably needed. For example, I've extended the Solaris mutex ABI to include the mutex_enter_nested() function which does take the subclass. We just need to update the ZFS code to use it appropriately.

[prakashsurya]

Right, I made a patch to fix this one specific case. Although, it may not be worth landing until all of the lockdep fixes are in place. Fixing this case only uncovers other warnings which need to be addressed before it's useful.

[behlendorf]

Great. Yes, I think it's probably worth keeping a development branch around with a stack of patches on it. It would be best to have one patch for each instance which explain how it was fixed for future reference. We'll probably end up with some generic infrastructure patches as well. Once we have everything fixed we can merge this whole branch.

[behlendorf]

Simply disabling the lockdep checker around these calls might be the simplest more portable thing we can do.

[behlendorf]

The relevant sections which trigger the lockdep warnings have had this check disabled. New issues have been opened for each of these remaining cases so they can be addressed independently as time permits.