public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks
@ 2026-04-15  7:16 Ravi Singh
  2026-04-15  7:20 ` Christoph Hellwig
  0 siblings, 1 reply; 7+ messages in thread
From: Ravi Singh @ 2026-04-15  7:16 UTC (permalink / raw)
  To: linux-xfs, linux-fsdevel; +Cc: djwong, dchinner, esandeen, cem

When creating symlinks on a nearly-full XFS filesystem,
xfs_symlink() fails with ENOSPC even though space could be
reclaimed from speculative preallocations. xfs_create() already
handles this by flushing outstanding delalloc blocks via
xfs_flush_inodes() and retrying the transaction allocation, but
xfs_symlink() returns the error immediately.

Add the same ENOSPC flush+retry logic to xfs_symlink() so that
symlink creation behaves consistently with file creation.

Signed-off-by: Ravi Singh <ravising@redhat.com>
---
 fs/xfs/xfs_symlink.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index c4da624fb296..bf3cf23892eb 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -148,6 +148,12 @@ xfs_symlink(
 
 	error = xfs_trans_alloc_icreate(mp, &M_RES(mp)->tr_symlink, udqp, gdqp,
 			pdqp, resblks, &tp);
+	if (error == -ENOSPC) {
+		/* flush outstanding delalloc blocks and retry */
+		xfs_flush_inodes(mp);
+		error = xfs_trans_alloc_icreate(mp, &M_RES(mp)->tr_symlink,
+				udqp, gdqp, pdqp, resblks, &tp);
+	}
 	if (error)
 		goto out_parent;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks
  2026-04-15  7:16 [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks Ravi Singh
@ 2026-04-15  7:20 ` Christoph Hellwig
  2026-04-15 13:24   ` Ravi Singh
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2026-04-15  7:20 UTC (permalink / raw)
  To: Ravi Singh; +Cc: linux-xfs, linux-fsdevel, djwong, dchinner, esandeen, cem

On Wed, Apr 15, 2026 at 07:16:57AM +0000, Ravi Singh wrote:
> When creating symlinks on a nearly-full XFS filesystem,
> xfs_symlink() fails with ENOSPC even though space could be
> reclaimed from speculative preallocations. xfs_create() already
> handles this by flushing outstanding delalloc blocks via
> xfs_flush_inodes() and retrying the transaction allocation, but
> xfs_symlink() returns the error immediately.
> 
> Add the same ENOSPC flush+retry logic to xfs_symlink() so that
> symlink creation behaves consistently with file creation.

While we're at it - do we also want it for tempfiles and scrub?
i.e., should we move this logic into xfs_trans_alloc_icreate,
similar to the quota logic?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks
  2026-04-15  7:20 ` Christoph Hellwig
@ 2026-04-15 13:24   ` Ravi Singh
  2026-04-15 14:14     ` Eric Sandeen
  2026-04-16  5:13     ` Christoph Hellwig
  0 siblings, 2 replies; 7+ messages in thread
From: Ravi Singh @ 2026-04-15 13:24 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xfs, linux-fsdevel, djwong, dchinner, esandeen, cem

On Wed, Apr 15, 2026 at 12:51 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Apr 15, 2026 at 07:16:57AM +0000, Ravi Singh wrote:
> > When creating symlinks on a nearly-full XFS filesystem,
> > xfs_symlink() fails with ENOSPC even though space could be
> > reclaimed from speculative preallocations. xfs_create() already
> > handles this by flushing outstanding delalloc blocks via
> > xfs_flush_inodes() and retrying the transaction allocation, but
> > xfs_symlink() returns the error immediately.
> >
> > Add the same ENOSPC flush+retry logic to xfs_symlink() so that
> > symlink creation behaves consistently with file creation.
>
> While we're at it - do we also want it for tempfiles and scrub?
> i.e., should we move this logic into xfs_trans_alloc_icreate,
> similar to the quota logic?
>
tmpfiles and scrub tempfiles are created without
directory entries, so they don't need a new directory block
allocation and don't hit this ENOSPC path.

As for moving the logic into xfs_trans_alloc_icreate(),
xfs_flush_inodes() is a heavier operation (sync_inodes_sb).
Pushing it down into xfs_trans_alloc_icreate() would make every
caller pay for that flush, including paths that don't need it.

I think keeping it in the callers that actually need it
(xfs_create and xfs_symlink) is the right scope.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks
  2026-04-15 13:24   ` Ravi Singh
@ 2026-04-15 14:14     ` Eric Sandeen
  2026-04-16  5:13     ` Christoph Hellwig
  1 sibling, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2026-04-15 14:14 UTC (permalink / raw)
  To: Ravi Singh, Christoph Hellwig
  Cc: linux-xfs, linux-fsdevel, djwong, dchinner, cem

On 4/15/26 8:24 AM, Ravi Singh wrote:
> On Wed, Apr 15, 2026 at 12:51 PM Christoph Hellwig <hch@infradead.org> wrote:
>>
>> On Wed, Apr 15, 2026 at 07:16:57AM +0000, Ravi Singh wrote:
>>> When creating symlinks on a nearly-full XFS filesystem,
>>> xfs_symlink() fails with ENOSPC even though space could be
>>> reclaimed from speculative preallocations. xfs_create() already
>>> handles this by flushing outstanding delalloc blocks via
>>> xfs_flush_inodes() and retrying the transaction allocation, but
>>> xfs_symlink() returns the error immediately.
>>>
>>> Add the same ENOSPC flush+retry logic to xfs_symlink() so that
>>> symlink creation behaves consistently with file creation.
>>
>> While we're at it - do we also want it for tempfiles and scrub?
>> i.e., should we move this logic into xfs_trans_alloc_icreate,
>> similar to the quota logic?
>>
> tmpfiles and scrub tempfiles are created without
> directory entries, so they don't need a new directory block
> allocation and don't hit this ENOSPC path.
> 
> As for moving the logic into xfs_trans_alloc_icreate(),
> xfs_flush_inodes() is a heavier operation (sync_inodes_sb).
> Pushing it down into xfs_trans_alloc_icreate() would make every
> caller pay for that flush, including paths that don't need it.
> 
> I think keeping it in the callers that actually need it
> (xfs_create and xfs_symlink) is the right scope.

I think that's correct. I had looked at this and came to the same
conclusions after also thinking i.e. tmpfiles needed the same
treatment, couldn't reproduce it, then realized it's because it's
directory block allocation that needs the freed up space, something
tmpfiles don't require.

Thanks,
-Eric


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks
  2026-04-15 13:24   ` Ravi Singh
  2026-04-15 14:14     ` Eric Sandeen
@ 2026-04-16  5:13     ` Christoph Hellwig
  2026-04-20  9:58       ` Ravi Singh
  2026-04-20 10:06       ` [PATCH v2] xfs: flush delalloc blocks on ENOSPC in xfs_trans_alloc_icreate Ravi Singh
  1 sibling, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-04-16  5:13 UTC (permalink / raw)
  To: Ravi Singh
  Cc: Christoph Hellwig, linux-xfs, linux-fsdevel, djwong, dchinner,
	esandeen, cem

On Wed, Apr 15, 2026 at 06:54:00PM +0530, Ravi Singh wrote:
> > While we're at it - do we also want it for tempfiles and scrub?
> > i.e., should we move this logic into xfs_trans_alloc_icreate,
> > similar to the quota logic?
> >
> tmpfiles and scrub tempfiles are created without
> directory entries, so they don't need a new directory block
> allocation and don't hit this ENOSPC path.


But we still ned the ialloc space reservation for all these cases.

> As for moving the logic into xfs_trans_alloc_icreate(),
> xfs_flush_inodes() is a heavier operation (sync_inodes_sb).
> Pushing it down into xfs_trans_alloc_icreate() would make every
> caller pay for that flush, including paths that don't need it.

Only if we git enospc and need to restart the underlying
xfs_trans_alloc call, similar to how it handles quotas.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks
  2026-04-16  5:13     ` Christoph Hellwig
@ 2026-04-20  9:58       ` Ravi Singh
  2026-04-20 10:06       ` [PATCH v2] xfs: flush delalloc blocks on ENOSPC in xfs_trans_alloc_icreate Ravi Singh
  1 sibling, 0 replies; 7+ messages in thread
From: Ravi Singh @ 2026-04-20  9:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xfs, linux-fsdevel, djwong, dchinner, esandeen, cem

On Thu, Apr 16, 2026 at 10:43 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Apr 15, 2026 at 06:54:00PM +0530, Ravi Singh wrote:
> > > While we're at it - do we also want it for tempfiles and scrub?
> > > i.e., should we move this logic into xfs_trans_alloc_icreate,
> > > similar to the quota logic?
> > >
> > tmpfiles and scrub tempfiles are created without
> > directory entries, so they don't need a new directory block
> > allocation and don't hit this ENOSPC path.
>
>
> But we still ned the ialloc space reservation for all these cases.
>

You're right - the ENOSPC comes from xfs_trans_alloc() failing
to reserve blocks via xfs_dec_fdblocks(), which is the same
 global counter for all callers. Even the smaller
XFS_IALLOC_SPACE_RES reservation for tmpfiles can fail there
when delalloc pressure has consumed the available fdblocks.

I did try several runs to reproduce the ENOSPC for tmpfile
creation, but wasn't able to trigger it in practice - the
smaller reservation makes it much harder to hit. That said,
the code path is the same and it makes sense to handle it
centrally rather than leave the gap.

> > As for moving the logic into xfs_trans_alloc_icreate(),
> > xfs_flush_inodes() is a heavier operation (sync_inodes_sb).
> > Pushing it down into xfs_trans_alloc_icreate() would make every
> > caller pay for that flush, including paths that don't need it.
>
> Only if we git enospc and need to restart the underlying
> xfs_trans_alloc call, similar to how it handles quotas.
>
>

v2 moves the xfs_flush_inodes() retry into
xfs_trans_alloc_icreate() and removes the existing caller-level
retry from xfs_create()


Thanks,
Ravi


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] xfs: flush delalloc blocks on ENOSPC in xfs_trans_alloc_icreate
  2026-04-16  5:13     ` Christoph Hellwig
  2026-04-20  9:58       ` Ravi Singh
@ 2026-04-20 10:06       ` Ravi Singh
  1 sibling, 0 replies; 7+ messages in thread
From: Ravi Singh @ 2026-04-20 10:06 UTC (permalink / raw)
  To: linux-xfs, linux-fsdevel; +Cc: djwong, dchinner, esandeen, cem

xfs_trans_alloc_icreate() can fail with ENOSPC when delalloc
reservations have consumed most of the available block count
(fdblocks).  xfs_trans_alloc() already retries internally with
xfs_blockgc_flush_all(), but that only trims post-EOF speculative
preallocation and may not free enough space for the transaction
reservation.

Add a retry with xfs_flush_inodes() when xfs_trans_alloc() returns
ENOSPC.  This forces writeback of all dirty inodes via
sync_inodes_sb(), converting delalloc reservations to real
allocations and freeing the over-reserved portion back to fdblocks.

This fixes all callers of xfs_trans_alloc_icreate() and removes
the existing caller-level retry from xfs_create(), which is now
handled centrally.

Signed-off-by: Ravi Singh <ravising@redhat.com>
---

v2:
 - Moved xfs_flush_inodes() retry into xfs_trans_alloc_icreate()
   instead of xfs_symlink() (Christoph)
 - Removed caller-level retry from xfs_create()
 - All callers now covered: xfs_create, xfs_symlink,
   xfs_create_tmpfile, xcrep_tempfile_create

 fs/xfs/xfs_inode.c | 6 ------
 fs/xfs/xfs_trans.c | 9 +++++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index beaa26ec62da..9978ac1422fc 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -699,12 +699,6 @@ xfs_create(
 	 */
 	error = xfs_trans_alloc_icreate(mp, tres, udqp, gdqp, pdqp, resblks,
 			&tp);
-	if (error == -ENOSPC) {
-		/* flush outstanding delalloc blocks and retry */
-		xfs_flush_inodes(mp);
-		error = xfs_trans_alloc_icreate(mp, tres, udqp, gdqp, pdqp,
-				resblks, &tp);
-	}
 	if (error)
 		goto out_parent;
 
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index bcc470f56e46..2796634b7d2b 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1186,6 +1186,9 @@ xfs_trans_reserve_more_inode(
 /*
  * Allocate an transaction in preparation for inode creation by reserving quota
  * against the given dquots.  Callers are not required to hold any inode locks.
+ * If the block reservation fails with ENOSPC, flush all delalloc blocks to
+ * reclaim space from speculative preallocation and retry.  This is similar to
+ * the quota retry but targets FS-wide ENOSPC.
  */
 int
 xfs_trans_alloc_icreate(
@@ -1199,10 +1202,16 @@ xfs_trans_alloc_icreate(
 {
 	struct xfs_trans	*tp;
 	bool			retried = false;
+	bool			flushed = false;
 	int			error;
 
 retry:
 	error = xfs_trans_alloc(mp, resv, dblocks, 0, 0, &tp);
+	if (error == -ENOSPC && !flushed) {
+		xfs_flush_inodes(mp);
+		flushed = true;
+		goto retry;
+	}
 	if (error)
 		return error;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-20 10:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15  7:16 [PATCH] xfs: flush delalloc blocks on ENOSPC when creating symlinks Ravi Singh
2026-04-15  7:20 ` Christoph Hellwig
2026-04-15 13:24   ` Ravi Singh
2026-04-15 14:14     ` Eric Sandeen
2026-04-16  5:13     ` Christoph Hellwig
2026-04-20  9:58       ` Ravi Singh
2026-04-20 10:06       ` [PATCH v2] xfs: flush delalloc blocks on ENOSPC in xfs_trans_alloc_icreate Ravi Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox