{"action":"created","issue":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3225","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3225/labels{/name}","comments_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3225/comments","events_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3225/events","html_url":"https://github.com/zfsonlinux/zfs/pull/3225","id":64274443,"number":3225,"title":"Fix deadlocks resulting from the post-kmem rework merge","user":{"login":"dweeezil","id":1432005,"avatar_url":"https://avatars.githubusercontent.com/u/1432005?v=3","gravatar_id":"","url":"https://api.github.com/users/dweeezil","html_url":"https://github.com/dweeezil","followers_url":"https://api.github.com/users/dweeezil/followers","following_url":"https://api.github.com/users/dweeezil/following{/other_user}","gists_url":"https://api.github.com/users/dweeezil/gists{/gist_id}","starred_url":"https://api.github.com/users/dweeezil/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dweeezil/subscriptions","organizations_url":"https://api.github.com/users/dweeezil/orgs","repos_url":"https://api.github.com/users/dweeezil/repos","events_url":"https://api.github.com/users/dweeezil/events{/privacy}","received_events_url":"https://api.github.com/users/dweeezil/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/zfsonlinux/zfs/labels/Bug%20-%20Major","name":"Bug - Major","color":"e11d21"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/zfsonlinux/zfs/milestones/24","html_url":"https://github.com/zfsonlinux/zfs/milestones/0.6.4","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/milestones/24/labels","id":175735,"number":24,"title":"0.6.4","description":"ZFS on Linux v0.6.4 - Functionality / Integration","creator":{"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},"open_issues":3,"closed_issues":427,"state":"open","created_at":"2012-09-16T18:38:17Z","updated_at":"2015-03-27T23:57:28Z","due_on":null,"closed_at":null},"comments":16,"created_at":"2015-03-25T13:52:48Z","updated_at":"2015-03-30T17:12:44Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/zfsonlinux/zfs/pulls/3225","html_url":"https://github.com/zfsonlinux/zfs/pull/3225","diff_url":"https://github.com/zfsonlinux/zfs/pull/3225.diff","patch_url":"https://github.com/zfsonlinux/zfs/pull/3225.patch"},"body":"Put the following functions under PF_FSTRANS to prevent deadlocks\r\ndue to re-entering the filesystem during reclaim:\r\n\r\n\tdbuf_hold()\r\n\tdmu_prefetch()\r\n\tsa_buf_hold()\r\n\r\nTesting this one now:\r\n\tdmu_zfetch_fetch() - z_hold_mtx[] via zget and zf_rwlock via dmu_zfetch"},"comment":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/comments/87754392","html_url":"https://github.com/zfsonlinux/zfs/pull/3225#issuecomment-87754392","issue_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3225","id":87754392,"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-03-30T17:12:44Z","updated_at":"2015-03-30T17:12:44Z","body":"> I'm concerned that @tuxoko's observation about the ordering issue with MUTEX_FSTRANS may be happening along the sync path because I'm seeing reclaim entered from txg_sync_thread in my live-locks:\r\n\r\n@tuxoko sorry about the long delayed comment on https://github.com/zfsonlinux/zfs/issues/3183#issuecomment-85425528.  You're exactly right, that's a legitimate case which can happen and I should have commented on it explicitly.  At the time I didn't think  that could occur in the existing code but given the stacks observed by @dweeezil it seems it may be possible.  Direct reclaim should never ever ever be possible in the context of the `txg_sync_thread()`.\r\n\r\nYou mentioned the only way to handle this transparently is to keep a reference count in the task.  Unfortunately, that's not really something we can do because we can't add a field to the task struct.  We could add a bit of thread specific data (TSD) but I suspect the performance hit there would be unacceptable.\r\n\r\nWe may not be able to transparently hide this case.  Which is a shame because I'd really like to keep the Linux specific bits to a minimum.  I completely agree with @dweeezil's comments above:\r\n\r\n> At this rate, the entire system will be reduced to non-sleeping allocations just as it mostly was before the kmem-rework merge. Furthermore, all these semi-gratuitous changes cause even more divergence with upstream code than KM_PUSHPAGE did in the past. I'm going to keep plugging away at it for awhile but a little voice in the back of my head is telling me we need a different approach.\r\n\r\nSo with that in mind let me throw out a couple possible alternate approaches we could take.  Better ideas are welcome!  I'd love to put this issue behind us once and for all.\r\n\r\n* Continue adding targeted calls to `spl_fstrans_mark()` throughout the common ZFS code.  One possible clean way to minimize the footprint here would be to extend the `ZFS_OBJ_HOLD_ENTER/ZFS_OBJ_HOLD_EXIT` macro to take a third `fstrans_cookie_t` argument.  Then we could hide the mark/unmark in the macro for Linux, and a patch could be pushed upstream to add the argument and just ignore it.  I'd have to see how this works out in practice but at the moment this seems like the best approach to me.\r\n\r\n* Alternately, we _could_ disable direct reclaim in larger chunks of the code by adding the mark/unmark to the Linux specific `zpl_*` functions.  That would keep the changes in Linux specific code and I think should cover most common cases.  Although it's heavy handed.\r\n\r\n* Even more heavy handed would be to just add GFP_NOFS to all KM_SLEEP allocations.  This would be even more comprehensive than the KM_PUSHPAGE solution but it would be straight forward.  We'd definitely want to somehow measure what the performance / stability impact is.  I could see this potentially causing issues on a machine which is starved for memory, but perhaps not.  It's hard to say for sure.\r\n\r\n"}}