Re: [bug report] fstests generic/774 hang again

[Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Sep 21, 2026 / 17:28, Darrick J. Wong wrote:
> On Thu, Sep 17, 2026 at 07:45:04AM +1000, Dave Chinner wrote:
[...]
> > A bunch of threads waiting on the ILOCK here:
> > 
> >  down_write_nested+0x1c0/0x1f0
> >  xfs_reflink_end_atomic_cow+0x2f3/0x560 [xfs]
> >  xfs_dio_write_end_io+0x4b7/0x650 [xfs]
> >  iomap_dio_complete+0x140/0xb20
> >  iomap_dio_complete_work+0x58/0x90
> >  process_one_work+0x947/0x1760
> > 
> > And the holder:
> > 
> >  schedule+0xe5/0x2e0
> >  xlog_grant_head_wait+0x175/0xac0 [xfs]
> >  xlog_grant_head_check+0x312/0x3f0 [xfs]
> >  xfs_log_regrant+0x380/0x7d0 [xfs]
> >  xfs_trans_roll+0x2d9/0x420 [xfs]
> >  xfs_defer_trans_roll+0x11e/0x4b0 [xfs]
> >  xfs_defer_finish_noroll+0x460/0xe70 [xfs]
> >  xfs_trans_commit+0xfc/0x180 [xfs]
> >  xfs_reflink_end_atomic_cow+0x3b2/0x560 [xfs]
> >  xfs_dio_write_end_io+0x4b7/0x650 [xfs]
> >  iomap_dio_complete+0x140/0xb20
> >  iomap_dio_complete_work+0x58/0x90
> >  process_one_work+0x947/0x1760
> > 
> > is waiting on log space whilst holding the ILOCK.
> > 
> > This looks to me like the test runs out of log space because of all
> > the IO completions holding transaction reservations waiting on the
> > ILOCK, whilst the ILOCK holder can't get enough log space to regrant
> > on transaction roll to continue the transaction.
> 
> Agreed.
> 
> Why are we calling xlog_grant_head_check from within
> xfs_reflink_end_atomic_cow?  I think the reason for doing that is
> because we've exhausted t_cnt in the ticket (i.e. we've already rolled
> more than tr_logcount times).
> 
> Oh.  tr_atomic_ioend.tr_logcount is 5 on a rmap+reflink filesystem,
> which it inherits from tr_itruncate.  However, tr_itruncate is only
> intended to remove two extents from a file, so it only needs 5 rolls.
> Coincidentally we calculate 5 rolls for each atomic extent remapping
> operation.
> 
> For atomic write ioends, what if we increased tr_logcount to 5x the
> number of remappings that would have to occur to finish the write?
> That would preallocate all the permanent reservation we'd need before we
> take the ILOCK, which avoids the situation of needing to obtain more log
> space while holding ILOCK.
> 
> The downside is that you'd have to limit the software awu_max even
> further, perhaps to 40% of the log size divided by
> (tr_logres*tr_logcount).  However, we'd still be able to handle
> concurrent atomic writes to different parts of the file, at least until
> fragmentation got bad.

Darrick, thanks for the idea. I wanted to try out the idea, so I cooled
the experimental patch below. Is this what you meant? It is a very rough
patch, and does not reflect the "limit the software awu_max even further"
part, it is just for experiment.

Anyway, I applied this patch on top of the kernel xfs/for-next branch git
hash 0ca15a1a1151, and repeated the test case g774. It did not hang after
100 times repeat, so it looks working.


From c90ec246b0b9e14097b0ad1d1ef5661261dd61db Mon Sep 17 00:00:00 2001
From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Date: Thu, 24 Sep 2026 19:08:32 +0900
Subject: [PATCH] xfs: try out Darrick's idea for g774 hang

[experimental patch]

Darrick's idea from
https://lore.kernel.org/linux-xfs/20260922002814.GK2705364@frogsfrogsfrogs/

The deadlock of g774 caused by grant space while holding ILOCK. To avoid the
deadlock, allocate enough space before taking the ILOCK by increasing
tr_logcount. 5 rolls are required for each extent, then multiply tr_logcount
by 5.
---
 fs/xfs/xfs_reflink.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 48013613663..9d3da713a01 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1020,6 +1020,28 @@ xfs_reflink_end_cow(
 	return error;
 }
 
+/*
+ * Number of transaction rolls needed to remap a single extent during an atomic
+ * write ioend.
+ */
+#define XFS_ATOMIC_IOEND_ROLLS	5
+
+/*
+ * Compute the number of log operations to reserve for an atomic write ioend
+ * that remaps @blockcount blocks with a @logres byte reservation.
+ */
+STATIC int
+xfs_calc_atomic_write_ioend_logcount(
+	struct xfs_mount	*mp,
+	xfs_extlen_t		blockcount,
+	unsigned int		logres)
+{
+	if (blockcount == 0 || logres == 0)
+		return M_RES(mp)->tr_atomic_ioend.tr_logcount;
+
+	return blockcount * XFS_ATOMIC_IOEND_ROLLS;
+}
+
 /*
  * Fully remap all of the file's data fork at once, which is the critical part
  * in achieving atomic behaviour.
@@ -1036,6 +1058,7 @@ xfs_reflink_end_atomic_cow(
 	xfs_fileoff_t			end_fsb;
 	int				error = 0;
 	struct xfs_mount		*mp = ip->i_mount;
+	struct xfs_trans_res		resv = M_RES(mp)->tr_atomic_ioend;
 	struct xfs_trans		*tp;
 	unsigned int			resblks;
 
@@ -1051,7 +1074,15 @@ xfs_reflink_end_atomic_cow(
 	resblks = (end_fsb - offset_fsb) *
 			XFS_NEXTENTADD_SPACE_RES(mp, 1, XFS_DATA_FORK);
 
-	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_atomic_ioend, resblks, 0,
+	/*
+	 * Reserve enough log operations to remap every block of this write
+	 * before we take ILOCK, so that no roll below has to wait for grant
+	 * space while holding ILOCK.
+	 */
+	resv.tr_logcount = xfs_calc_atomic_write_ioend_logcount(mp,
+			end_fsb - offset_fsb, resv.tr_logres);
+
+	error = xfs_trans_alloc(mp, &resv, resblks, 0,
 			XFS_TRANS_RESERVE, &tp);
 	if (error)
 		return error;
-- 
2.55.0