Re: [PATCH v4] ntfs: serialize attribute-list replacement with lookups

[Cen Zhang]

Hi, Hyunchul

>
> Could you summarize what issues were found and how they were resolved?
>
> There are so many changes in the v5 patch, it is difficult to understand the
> overall picture.
>

Thanks.

I attempted to create the following summary with the help of LLM.

Compared with v4, the main changes are not for a different original crash,
but for the locking problems I found while trying to validate the same
attr-list lifetime fix with xfstests.

The rough summary is:

1. generic/013 self-deadlock

   The stack you reported showed a path already holding attr_list_lock for
   writing and then re-entering ntfs_attr_get_search_ctx(), which tried to
   take the same rwsem again for reading.

   In v4 I tried to address that by adding LOCK_NONE/nolock paths for callers
   that already control attr_list_lock.

2. Nested index/$BITMAP locking

   Later, generic/467 reported a recursive locking problem.  The nested
   directory/index path propagated nested attr-list locking to the index
   search context, but the later $BITMAP attribute inode open still used the
   normal ntfs_attr_iget() path.

   v5 propagates the nested lock mode through the index open and the related
   $BITMAP attribute open.

3. attr_list_lock vs runlist.lock ordering

   Some paths reached attr-list lookup/update while runlist.lock was already
   held, or updated mapping pairs while still depending on the runlist lock.
   That conflicted with the intended order:

     mrec_lock => attr_list_lock => runlist.lock

   v5 adds explicit nolock/runlist-locked variants so callers that already
   hold one side do not acquire the locks again in the wrong order.

4. Mapping-pairs update paths

   Some mapping-pairs updates needed to be moved out of runlist-locked
   sections.  v5 snapshots the runlist where needed and defers mapping-pairs
   updates in write/fallocate/writeback paths until after runlist.lock is
   dropped.

5. Propagating the lock mode through helper layers

   Once the search-context lock mode was used to protect ctx->al_entry for the
   whole context lifetime, related helpers also had to carry the same mode:
   ntfs_attr_iget(), ntfs_index_iget(), iomap/aops, inode attr read/write, and
   some index/reparse paths.  This is the main reason v5 became much larger
   than v4.

So the high-level picture is: v4 fixed the first obvious re-entry/deadlock
case, but wider xfstests coverage exposed that the lock mode had to be carried
through more NTFS helper paths to keep the attr-list cursor lifetime and lock
ordering consistent.  That is why I sent v5 as RFC rather than as a final
patch.

Best regards
Cen Zhang.