Re: [PATCH v2 3/3] ntfs: do not convert a named $DATA stream when making room in a record

[Wahid Khan]

Hi,

You were right, v3 deadlocks. I reproduced your diagram: with a debug
delay before the folio lock in ntfs_attr_make_non_resident(), a reader
of the stream and the make-room loop hang the kernel (hung task, lockdep
shows the writer holding the unnamed inode's i_rwsem and mrec_lock and
waiting for folio 0, the reader holding the stream's folio 0 and waiting
for mrec_lock). The same direct conversion of the inode being written
is safe because the reader of that inode waits on its i_rwsem first.

An automated review of v3 also listed other problems with converting a
stream from this loop. I could reproduce most of them, with a forced
make-room pass and debug delays:

- a write to the stream that passed its residency check before the
  conversion carries on with a non-resident attribute (iomap_iter
  WARNINGs, "possible recursive locking", hang)
- an unlinked file with an open stream: "mft 0x1b, type 0x80 is
  corrupt. Run chkdsk." and the next write to the stream fails
- a stream that is being removed is converted under
  ntfs_stream_finish_remove() (unlink returned 1)
- two streams whose names differ only in case: WARN in
  ntfs_attr_make_non_resident(), the lookup ignores case

v4 keeps ntfs_attr_iget() for the named stream and adds what is needed:
FGP_NOWAIT for the folio in this loop (skip when busy), convert a
stream only if it is not in the inode cache and its file is not being
deleted or unlinked, and compare the stream name exactly. All of the
cases above fail on v3 and pass with v4. The WARN from the original
image is fixed.

I could not show data loss on unlink, only that the unsafe state is
reached. The debug delays are mine, so these windows are narrow without
them. If you think this is more than the loop should
carry, the one-line v2 (skip named streams) also fixes the WARN.

Thanks,
Wahid