{"action":"created","issue":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3681","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3681/labels{/name}","comments_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3681/comments","events_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3681/events","html_url":"https://github.com/zfsonlinux/zfs/issues/3681","id":100400629,"number":3681,"title":"lock order inversion between zvol_open() and dsl_pool_sync()...zvol_rename_minors()","user":{"login":"bprotopopov","id":2488505,"avatar_url":"https://avatars.githubusercontent.com/u/2488505?v=3","gravatar_id":"","url":"https://api.github.com/users/bprotopopov","html_url":"https://github.com/bprotopopov","followers_url":"https://api.github.com/users/bprotopopov/followers","following_url":"https://api.github.com/users/bprotopopov/following{/other_user}","gists_url":"https://api.github.com/users/bprotopopov/gists{/gist_id}","starred_url":"https://api.github.com/users/bprotopopov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bprotopopov/subscriptions","organizations_url":"https://api.github.com/users/bprotopopov/orgs","repos_url":"https://api.github.com/users/bprotopopov/repos","events_url":"https://api.github.com/users/bprotopopov/events{/privacy}","received_events_url":"https://api.github.com/users/bprotopopov/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/zfsonlinux/zfs/labels/Bug%20-%20Minor","name":"Bug - Minor","color":"e11d21"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-08-11T20:22:59Z","updated_at":"2015-08-25T15:43:42Z","closed_at":null,"body":"There appears to be a problem with the following two code paths executed concurrently:\r\n\r\n1) resulting from the zfs rename on zvol snapshot, taking place in txg_sync_thread()\r\n\r\n #0 [ffff8802a6e7ba20] schedule at ffffffff815299d0\r\n #1 [ffff8802a6e7baf8] __mutex_lock_slowpath at ffffffff8152b3a6\r\n #2 [ffff8802a6e7bb68] mutex_lock at ffffffff8152aecb\r\n #3 [ffff8802a6e7bb88] zvol_rename_minors at ffffffffa02fe634 [zfs]\r\n #4 [ffff8802a6e7bbd8] dsl_dir_rename_sync at ffffffffa027c04d [zfs]\r\n #5 [ffff8802a6e7bc48] dsl_sync_task_sync at ffffffffa02850f2 [zfs]\r\n #6 [ffff8802a6e7bc78] dsl_pool_sync at ffffffffa027d31b [zfs]\r\n #7 [ffff8802a6e7bcf8] spa_sync at ffffffffa0293587 [zfs]\r\n #8 [ffff8802a6e7bdc8] txg_sync_thread at ffffffffa02a890b [zfs]\r\n #9 [ffff8802a6e7beb8] thread_generic_wrapper at ffffffffa0169978 [spl]\r\n#10 [ffff8802a6e7bee8] kthread at ffffffff8109e78e\r\n#11 [ffff8802a6e7bf48] kernel_thread at ffffffff8100c28a\r\n\r\n2) resulting from opening zvol for I/O\r\n #0 [ffff88020e53d9b8] schedule at ffffffff815299d0\r\n #1 [ffff88020e53da90] cv_wait_common at ffffffffa0171e65 [spl]\r\n #2 [ffff88020e53db20] __cv_wait at ffffffffa0171f75 [spl]\r\n #3 [ffff88020e53db30] rrw_enter_read at ffffffffa028df7b [zfs]\r\n #4 [ffff88020e53db60] rrw_enter at ffffffffa028e0d0 [zfs]\r\n #5 [ffff88020e53db70] dsl_pool_config_enter at ffffffffa027c24d [zfs]\r\n #6 [ffff88020e53db80] dsl_pool_hold at ffffffffa027c2da [zfs]\r\n #7 [ffff88020e53dbc0] dmu_objset_own at ffffffffa025e1a0 [zfs]\r\n #8 [ffff88020e53dc20] zvol_open at ffffffffa02fd4fb [zfs]\r\n #9 [ffff88020e53dc90] __blkdev_get at ffffffff811cbc4e\r\n#10 [ffff88020e53dcf0] blkdev_get at ffffffff811cbf70\r\n#11 [ffff88020e53dd00] blkdev_open at ffffffff811cbff1\r\n#12 [ffff88020e53dd30] __dentry_open at ffffffff8118b1ba\r\n#13 [ffff88020e53dd90] nameidata_to_filp at ffffffff8118b524\r\n#14 [ffff88020e53ddb0] do_filp_open at ffffffff811a12d0\r\n#15 [ffff88020e53df20] do_sys_open at ffffffff8118af77\r\n#16 [ffff88020e53df70] sys_open at ffffffff8118b080\r\n#17 [ffff88020e53df80] system_call_fastpath at ffffffff8100b0f2\r\n\r\nThe first code path takes dsl pool config lock, then tries to take zvol_state_lock, while the second, takes the zvol_state_lock, and then tries to get the dsl pool config lock. \r\n\r\nIt is interesting that zvol_first_open() called from zvol_open() takes care to obtain spa_namespace_lock, and fails if it cannot, in order to avoid the locking order inversion between spa_namespace_lock and the zvol_state_lock. \r\n\r\nYet the txg_sync_thread does not seem to hold the spa_namespace_lock when calling dsl_pool_sync() and further, before trying to get the zvol_state_lock. That is why I believe this defensive check is not effective in this particular case. "},"comment":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/comments/134635389","html_url":"https://github.com/zfsonlinux/zfs/issues/3681#issuecomment-134635389","issue_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3681","id":134635389,"user":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"created_at":"2015-08-25T15:43:41Z","updated_at":"2015-08-25T15:43:41Z","body":"@bprotopopov perhaps the race would be smaller but I think your right.  It would still exist.  I'm definitely not opposed to doing the minor creation asynchronously I was just tossing out another possible idea.  I'd love to see your patch for this once your happy with it.\r\n\r\nIn fact, what would be ideal is if you could build up a small patch stack with this change and your other zvol improvements.  Then we could get them all reviewed in one go and merged."}}