Re: [BUG] ntfs: task hung in find_inode due to self-deadlock in ntfs_attrlist_update() during unmount

[Hyunchul Lee]

Hi Peiyang,

> ==========
> Root cause
> ==========
>
> A = the NTFS base inode whose delayed allocation / mapping pairs are being
>     written back.
>
> B = the fake NTFS attribute inode for A's unnamed $ATTRIBUTE_LIST attribute,
>     i.e. the inode returned by ntfs_attr_iget(VFS_I(A), AT_ATTRIBUTE_LIST, AT_UNNAMED, 0)
>
> Timeflow (all code should be executed within one task):
>
> 1. The task starts unmounting the NTFS filesystem that contains A:
>
>         cleanup_mnt()                    fs/namespace.c:1292
>         --> deactivate_super()           fs/super.c:505
>         --> deactivate_locked_super()    fs/super.c:471
>             --> fs->kill_sb(s)           fs/super.c:476
>                 /* ntfs_fs_type.kill_sb is kill_block_super. */
>                                            fs/ntfs/super.c:2645
>         --> kill_block_super()           fs/super.c:1721
>         --> generic_shutdown_super()     fs/super.c:618
>
>    generic_shutdown_super() clears SB_ACTIVE and then evicts cached inodes:
>
>         sb->s_flags &= ~SB_ACTIVE        fs/super.c:626
>         evict_inodes(sb)                 fs/super.c:632
>
> 2. evict_inodes(sb) selects B, marks it I_FREEING, and the same task starts
>    evicting B:
>
>         evict_inodes(sb)                 fs/inode.c:897
>         --> inode_state_set(B, I_FREEING)
>                                            fs/inode.c:918
>         --> dispose_list(&dispose)       fs/inode.c:875
>         --> evict(B)                     fs/inode.c:883
>         --> ntfs_evict_big_inode(B)      fs/ntfs/inode.c:2261
>
>    B is a fake attribute inode.  During its eviction, NTFS drops B's reference
>    on its base inode A:
>
>         iput(VFS_I(B->ext.base_ntfs_ino))
>                                            fs/ntfs/inode.c:2307
>
>    At this point B is still in the inode hash.  VFS removes B from the hash
>    and wakes __wait_on_freeing_inode() waiters only after
>    ntfs_evict_big_inode(B) returns:
>
>         remove_inode_hash(B)             fs/inode.c:849
>         inode_wake_up_bit(B, __I_NEW)    fs/inode.c:862
>
> 3. The iput(A) from ntfs_evict_big_inode(B) starts synchronous writeback of A:
>
>         iput(A)                         fs/inode.c:1972
>         --> iput_final(A)               fs/inode.c:1916
>         --> write_inode_now(A, 1)       fs/fs-writeback.c:2961
>         --> ntfs_writepages(A->i_mapping)
>                                           fs/ntfs/aops.c:244
>         --> ntfs_writeback_range(... A ...)
>                                           fs/ntfs/iomap.c:861
>
>    SB_ACTIVE is already clear, so iput_final(A) cannot keep A on the inode LRU.
>    It has to write and evict A.
>
> 4. A's writeback updates mapping pairs and then updates A's attribute list:
>
>         __ntfs_write_iomap_begin(A, ...)
>                                           fs/ntfs/iomap.c:868
>         --> ntfs_attr_map_cluster(A, ...)
>                                           fs/ntfs/attrib.c:4948
>         --> ntfs_attr_update_mapping_pairs(A, 0)
>                                           fs/ntfs/attrib.c:3535
>         --> ntfs_attrlist_update(A)      fs/ntfs/attrib.c:3712
>         --> ntfs_attr_iget(VFS_I(A),
>                            AT_ATTRIBUTE_LIST, AT_UNNAMED, 0)
>                                           fs/ntfs/attrlist.c:60
>                                           fs/ntfs/inode.c:209
>
>    This attempts to get B *again* while B is still being evicted.
>
> 5. ntfs_attr_iget() reaches find_inode().  find_inode() finds B, sees that B
>    already has I_FREEING set, and sleeps waiting for B's eviction to finish:
>
>         iget5_locked()                  fs/ntfs/inode.c:224
>                                          fs/inode.c:1375
>         --> find_inode()                fs/inode.c:1040
>         --> __wait_on_freeing_inode(B)  fs/inode.c:1061
>                                          fs/inode.c:2537
>         --> schedule()                  fs/inode.c:2560
>
>    This is a self-deadlock.  The task waits for B's eviction to complete, but
>    the only code that can complete B's eviction is the later part of evict(B),
>    after ntfs_evict_big_inode(B) returns.  The same task cannot return there
>    because it is now sleeping in the nested find_inode() path.

Thank you for the detailed root cause analysis.

Your analysis looks correct. However we would like to discuss whether
there is another way to handle this situation.

The call stack you provided seems unlikely to occur under normal conditions,
because generic_shutdown_super() unconditionally runs sync_filesystems()
before calling evict_inodes().

Does this issue occur when sync_filesystem() fails, for example due to
I/O errors or disk removal?

If so, would it make sense to mark volume as shutdown (NVolShutdown) or
switch the filesystem to read-only when a writeback error is encountered?