Re: moar weird metadata corruptions, this time on arm64

[Dave Chinner <david@fromorbit.com>]

On Thu, Nov 24, 2022 at 08:44:39AM -0800, Darrick J. Wong wrote:
> Also, last night's run produced this:
> 
> ino 0x140bb3 func xfs_bmapi_reserve_delalloc line 4164 data fork:
>     ino 0x140bb3 nr 0x0 nr_real 0x0 offset 0xb9 blockcount 0x1f startblock 0x935de2 state 1
>     ino 0x140bb3 nr 0x1 nr_real 0x1 offset 0xe6 blockcount 0xa startblock 0xffffffffe0007 state 0
>     ino 0x140bb3 nr 0x2 nr_real 0x1 offset 0xd8 blockcount 0xe startblock 0x935e01 state 0
> ino 0x140bb3 fork 0 oldoff 0xe6 oldlen 0x4 oldprealloc 0x6 isize 0xe6000
>     ino 0x140bb3 oldgotoff 0xea oldgotstart 0xfffffffffffffffe oldgotcount 0x0 oldgotstate 0
>     ino 0x140bb3 crapgotoff 0x0 crapgotstart 0x0 crapgotcount 0x0 crapgotstate 0
>     ino 0x140bb3 freshgotoff 0xd8 freshgotstart 0x935e01 freshgotcount 0xe freshgotstate 0
>     ino 0x140bb3 nowgotoff 0xe6 nowgotstart 0xffffffffe0007 nowgotcount 0xa nowgotstate 0
>     ino 0x140bb3 oldicurpos 1 oldleafnr 2 oldleaf 0xfffffc00f0609a00
>     ino 0x140bb3 crapicurpos 2 crapleafnr 2 crapleaf 0xfffffc00f0609a00
>     ino 0x140bb3 freshicurpos 1 freshleafnr 2 freshleaf 0xfffffc00f0609a00
>     ino 0x140bb3 newicurpos 1 newleafnr 3 newleaf 0xfffffc00f0609a00
> 
> The old/fresh/nowgot have the same meaning as yesterday.  "crapgot" is
> the results of duplicating the cursor at the start of the body of
> xfs_bmapi_reserve_delalloc and performing a fresh lookup at @off.
> I think @oldgot is a HOLESTARTBLOCK extent because the first lookup
> didn't find anything, so we filled in imap with "fake hole until the
> end".  At the time of the first lookup, I suspect that there's only one
> 32-block unwritten extent in the mapping (hence oldicurpos==1) but by
> the time we get to recording crapgot, crapicurpos==2.

Ok, that's much simpler to reason about, and implies the smoke is
coming from xfs_buffered_write_iomap_begin() or
xfs_bmapi_reserve_delalloc(). I suspect the former - it does a lot
of stuff with the ILOCK_EXCL held.....

.... including calling xfs_qm_dqattach_locked().

xfs_buffered_write_iomap_begin
  ILOCK_EXCL
  look up icur
  xfs_qm_dqattach_locked
    xfs_qm_dqattach_one
      xfs_qm_dqget_inode
        dquot cache miss
        xfs_iunlock(ip, XFS_ILOCK_EXCL);
        error = xfs_qm_dqread(mp, id, type, can_alloc, &dqp);
        xfs_ilock(ip, XFS_ILOCK_EXCL);
  ....
  xfs_bmapi_reserve_delalloc(icur)

Yup, that's what is letting the magic smoke out -
xfs_qm_dqattach_locked() can cycle the ILOCK. If that happens, we
can pass a stale icur to xfs_bmapi_reserve_delalloc() and it all
goes downhill from there.

> IOWS, I think I can safely eliminate FIEXCHANGE shenanigans and
> concentrate on finding an unlocked unwritten -> written extent
> conversion.  Or possibly a written -> unwritten extent conversion?
> 
> Anyway, long holiday weekend, so I won't get back to this until Monday.
> Just wanted to persist my notes to the mailing list so I can move on to
> testing the write race fixes with djwong-dev.

And I'm on PTO for the next couple of working days, too, so I'm not
going to write a patch for it right now, either.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com