potential zfs/vfs trouble in force umount

[Andriy Gapon]

on 14/10/2012 15:12 Andriy Gapon said the following:
> on 14/10/2012 14:25 Pawel Jakub Dawidek said the following:
>> On Sun, Oct 14, 2012 at 12:43:48PM +0300, Andriy Gapon wrote:
>>>
>>> I think that there is the following potentially troublesome scenario. One
>>> thread does zil_commit and obtains a znode pointer using zfs_zget.  At
>>> this point the thread doesn't have any locks on either the znode or its
>>> vnode.  the only thing that is supposed to keep them around is a
>>> reference on the vnode. If a force umount is going on in parallel, the
>>> one of the first things it does is calling vflush(FORCECLOSE) (this
>>> happens before closing down zil).  vflush force-reclaims all vnodes in
>>> this case (even when v_usecount > 0).  So the znode in question gets
>>> destroyed. Later, when the first thread tries to dereference the znode
>>> pointer it would crash.
>>
>> The z_teardown_lock lock is held for reading for every VOP and zfs_umount()
>> obtains this lock for writing before calling vflush(FORCECLOSE) and sets
>> z_unmounted to true. This in turn will make every new VOP to return with
>> EIO. This ensures that no VOP is in-progress when vflush() is called.
>>
> 
> What was/is not clear to me is whether zil operations are always called under
> z_teardown_lock (aka ZFS_ENTER)...
> 

OK, this appears to be true.
The only special case is zil_commit being called from zil_close in umount.
Because  zil_close is called after vflush all the znodes would already be
properly disposed of.  If there is anything in zil at that point, then I am not
sure what zfs_zget would do... Return an error and some the records would remain
uncommitted/lost?  Or create znodes (and vnodes) despite the unmount going on?
Something else?..

Umm, looks like there would be an error in insmntque and so zfs_zget would
return an error.
Not sure if losing some zil entries (non-sync operations) is something to worry
about for the forceful unmounting, though.

-- 
Andriy Gapon