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@xxxxxxxxxxxxx
