All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out
@ 2021-02-01  2:05 Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 01/12] xfs: trigger all block gc scans when low on quota space Darrick J. Wong
                   ` (11 more replies)
  0 siblings, 12 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:05 UTC (permalink / raw)
  To: djwong; +Cc: Brian Foster, Christoph Hellwig, linux-xfs, hch, david, bfoster

Hi all,

Historically, when users ran out of space or quota when trying to write
to the filesystem, XFS didn't try very hard to reclaim space that it
might have speculatively allocated for the purpose of speeding up
front-end filesystem operations (appending writes, cow staging).  The
upcoming deferred inactivation series will greatly increase the amount
of allocated space that isn't actively being used to store user data.

Therefore, try to reduce the circumstances where we return EDQUOT or
ENOSPC to userspace by teaching the write paths to try to clear space
and retry the operation one time before giving up.

Previous iterations of this patchset made massive changes to the
codebase, but thanks to the transaction allocation helpers that Brian
pushed for (in the previous patchset) this is mostly no longer
necessary. :)

v2: clean up and rebase against 5.11.
v3: restructure the retry loops per dchinner suggestion
v4: simplify the calling convention of xfs_trans_reserve_quota_nblks
v5: constrain the open-coded 'goto retry' loops to the helpers created
    in the previous patchset
v6: move "xfs: try worst case space reservation upfront in
    xfs_reflink_remap_extent" to this series, open-code the qretry
    helpers in the (now very few) places they are used
v7: rebase chown stuff to use new helpers introduced in quota cleanups

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=reclaim-space-harder-5.12
---
 fs/xfs/xfs_file.c    |   24 +++----
 fs/xfs/xfs_icache.c  |  184 +++++++++++++++++++++++++++++++++-----------------
 fs/xfs/xfs_icache.h  |    7 +-
 fs/xfs/xfs_ioctl.c   |    2 +
 fs/xfs/xfs_reflink.c |   28 +++++++-
 fs/xfs/xfs_trace.c   |    1 
 fs/xfs/xfs_trace.h   |   42 +++++++++++
 fs/xfs/xfs_trans.c   |   38 ++++++++++
 8 files changed, 244 insertions(+), 82 deletions(-)


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

* [PATCH 01/12] xfs: trigger all block gc scans when low on quota space
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
@ 2021-02-01  2:05 ` Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 02/12] xfs: don't stall cowblocks scan if we can't take locks Darrick J. Wong
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:05 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

The functions to run an eof/cowblocks scan to try to reduce quota usage
are kind of a mess -- the logic repeatedly initializes an eofb structure
and there are logic bugs in the code that result in the cowblocks scan
never actually happening.

Replace all three functions with a single function that fills out an
eofb and runs both eof and cowblocks scans.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c   |   15 ++++++---------
 fs/xfs/xfs_icache.c |   46 ++++++++++++++++------------------------------
 fs/xfs/xfs_icache.h |    4 ++--
 3 files changed, 24 insertions(+), 41 deletions(-)


diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 5d4a66c72c78..69879237533b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -713,7 +713,7 @@ xfs_file_buffered_write(
 	struct inode		*inode = mapping->host;
 	struct xfs_inode	*ip = XFS_I(inode);
 	ssize_t			ret;
-	int			enospc = 0;
+	bool			cleared_space = false;
 	int			iolock;
 
 	if (iocb->ki_flags & IOCB_NOWAIT)
@@ -745,19 +745,16 @@ xfs_file_buffered_write(
 	 * also behaves as a filter to prevent too many eofblocks scans from
 	 * running at the same time.
 	 */
-	if (ret == -EDQUOT && !enospc) {
+	if (ret == -EDQUOT && !cleared_space) {
 		xfs_iunlock(ip, iolock);
-		enospc = xfs_inode_free_quota_eofblocks(ip);
-		if (enospc)
-			goto write_retry;
-		enospc = xfs_inode_free_quota_cowblocks(ip);
-		if (enospc)
+		cleared_space = xfs_inode_free_quota_blocks(ip);
+		if (cleared_space)
 			goto write_retry;
 		iolock = 0;
-	} else if (ret == -ENOSPC && !enospc) {
+	} else if (ret == -ENOSPC && !cleared_space) {
 		struct xfs_eofblocks eofb = {0};
 
-		enospc = 1;
+		cleared_space = true;
 		xfs_flush_inodes(ip->i_mount);
 
 		xfs_iunlock(ip, iolock);
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index deb99300d171..c71eb15e3835 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1397,33 +1397,31 @@ xfs_icache_free_eofblocks(
 }
 
 /*
- * Run eofblocks scans on the quotas applicable to the inode. For inodes with
- * multiple quotas, we don't know exactly which quota caused an allocation
+ * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
+ * with multiple quotas, we don't know exactly which quota caused an allocation
  * failure. We make a best effort by including each quota under low free space
  * conditions (less than 1% free space) in the scan.
  */
-static int
-__xfs_inode_free_quota_eofblocks(
-	struct xfs_inode	*ip,
-	int			(*execute)(struct xfs_mount *mp,
-					   struct xfs_eofblocks	*eofb))
+bool
+xfs_inode_free_quota_blocks(
+	struct xfs_inode	*ip)
 {
-	int scan = 0;
-	struct xfs_eofblocks eofb = {0};
-	struct xfs_dquot *dq;
+	struct xfs_eofblocks	eofb = {0};
+	struct xfs_dquot	*dq;
+	bool			do_work = false;
 
 	/*
 	 * Run a sync scan to increase effectiveness and use the union filter to
 	 * cover all applicable quotas in a single scan.
 	 */
-	eofb.eof_flags = XFS_EOF_FLAGS_UNION|XFS_EOF_FLAGS_SYNC;
+	eofb.eof_flags = XFS_EOF_FLAGS_UNION | XFS_EOF_FLAGS_SYNC;
 
 	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
 		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
 		if (dq && xfs_dquot_lowsp(dq)) {
 			eofb.eof_uid = VFS_I(ip)->i_uid;
 			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
-			scan = 1;
+			do_work = true;
 		}
 	}
 
@@ -1432,21 +1430,16 @@ __xfs_inode_free_quota_eofblocks(
 		if (dq && xfs_dquot_lowsp(dq)) {
 			eofb.eof_gid = VFS_I(ip)->i_gid;
 			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
-			scan = 1;
+			do_work = true;
 		}
 	}
 
-	if (scan)
-		execute(ip->i_mount, &eofb);
+	if (!do_work)
+		return false;
 
-	return scan;
-}
-
-int
-xfs_inode_free_quota_eofblocks(
-	struct xfs_inode *ip)
-{
-	return __xfs_inode_free_quota_eofblocks(ip, xfs_icache_free_eofblocks);
+	xfs_icache_free_eofblocks(ip->i_mount, &eofb);
+	xfs_icache_free_cowblocks(ip->i_mount, &eofb);
+	return true;
 }
 
 static inline unsigned long
@@ -1646,13 +1639,6 @@ xfs_icache_free_cowblocks(
 			XFS_ICI_COWBLOCKS_TAG);
 }
 
-int
-xfs_inode_free_quota_cowblocks(
-	struct xfs_inode *ip)
-{
-	return __xfs_inode_free_quota_eofblocks(ip, xfs_icache_free_cowblocks);
-}
-
 void
 xfs_inode_set_cowblocks_tag(
 	xfs_inode_t	*ip)
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 3a4c8b382cd0..3f7ddbca8638 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -54,17 +54,17 @@ long xfs_reclaim_inodes_nr(struct xfs_mount *mp, int nr_to_scan);
 
 void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
 
+bool xfs_inode_free_quota_blocks(struct xfs_inode *ip);
+
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
 void xfs_inode_clear_eofblocks_tag(struct xfs_inode *ip);
 int xfs_icache_free_eofblocks(struct xfs_mount *, struct xfs_eofblocks *);
-int xfs_inode_free_quota_eofblocks(struct xfs_inode *ip);
 void xfs_eofblocks_worker(struct work_struct *);
 void xfs_queue_eofblocks(struct xfs_mount *);
 
 void xfs_inode_set_cowblocks_tag(struct xfs_inode *ip);
 void xfs_inode_clear_cowblocks_tag(struct xfs_inode *ip);
 int xfs_icache_free_cowblocks(struct xfs_mount *, struct xfs_eofblocks *);
-int xfs_inode_free_quota_cowblocks(struct xfs_inode *ip);
 void xfs_cowblocks_worker(struct work_struct *);
 void xfs_queue_cowblocks(struct xfs_mount *);
 


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

* [PATCH 02/12] xfs: don't stall cowblocks scan if we can't take locks
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 01/12] xfs: trigger all block gc scans when low on quota space Darrick J. Wong
@ 2021-02-01  2:05 ` Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 03/12] xfs: xfs_inode_free_quota_blocks should scan project quota Darrick J. Wong
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:05 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Don't stall the cowblocks scan on a locked inode if we possibly can.
We'd much rather the background scanner keep moving.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_icache.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index c71eb15e3835..89f9e692fde7 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1605,17 +1605,31 @@ xfs_inode_free_cowblocks(
 	void			*args)
 {
 	struct xfs_eofblocks	*eofb = args;
+	bool			wait;
 	int			ret = 0;
 
+	wait = eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC);
+
 	if (!xfs_prep_free_cowblocks(ip))
 		return 0;
 
 	if (!xfs_inode_matches_eofb(ip, eofb))
 		return 0;
 
-	/* Free the CoW blocks */
-	xfs_ilock(ip, XFS_IOLOCK_EXCL);
-	xfs_ilock(ip, XFS_MMAPLOCK_EXCL);
+	/*
+	 * If the caller is waiting, return -EAGAIN to keep the background
+	 * scanner moving and revisit the inode in a subsequent pass.
+	 */
+	if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
+		if (wait)
+			return -EAGAIN;
+		return 0;
+	}
+	if (!xfs_ilock_nowait(ip, XFS_MMAPLOCK_EXCL)) {
+		if (wait)
+			ret = -EAGAIN;
+		goto out_iolock;
+	}
 
 	/*
 	 * Check again, nobody else should be able to dirty blocks or change
@@ -1625,6 +1639,7 @@ xfs_inode_free_cowblocks(
 		ret = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, false);
 
 	xfs_iunlock(ip, XFS_MMAPLOCK_EXCL);
+out_iolock:
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
 
 	return ret;


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

* [PATCH 03/12] xfs: xfs_inode_free_quota_blocks should scan project quota
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 01/12] xfs: trigger all block gc scans when low on quota space Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 02/12] xfs: don't stall cowblocks scan if we can't take locks Darrick J. Wong
@ 2021-02-01  2:05 ` Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 04/12] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts Darrick J. Wong
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:05 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Buffered writers who have run out of quota reservation call
xfs_inode_free_quota_blocks to try to free any space reservations that
might reduce the quota usage.  Unfortunately, the buffered write path
treats "out of project quota" the same as "out of overall space" so this
function has never supported scanning for space that might ease an "out
of project quota" condition.

We're about to start using this function for cases where we actually
/can/ tell if we're out of project quota, so add in this functionality.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_icache.c |    9 +++++++++
 1 file changed, 9 insertions(+)


diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 89f9e692fde7..10c1a0dee17d 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1434,6 +1434,15 @@ xfs_inode_free_quota_blocks(
 		}
 	}
 
+	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_prid = ip->i_d.di_projid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
+			do_work = true;
+		}
+	}
+
 	if (!do_work)
 		return false;
 


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

* [PATCH 04/12] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (2 preceding siblings ...)
  2021-02-01  2:05 ` [PATCH 03/12] xfs: xfs_inode_free_quota_blocks should scan project quota Darrick J. Wong
@ 2021-02-01  2:05 ` Darrick J. Wong
  2021-02-01  2:05 ` [PATCH 05/12] xfs: pass flags and return gc errors from xfs_blockgc_free_quota Darrick J. Wong
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:05 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Move this function further down in the file so that later cleanups won't
have to declare static functions.  Change the name because we're about
to rework all the code that performs garbage collection of speculatively
allocated file blocks.  No functional changes.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c   |    2 -
 fs/xfs/xfs_icache.c |  110 ++++++++++++++++++++++++++-------------------------
 fs/xfs/xfs_icache.h |    2 -
 3 files changed, 57 insertions(+), 57 deletions(-)


diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 69879237533b..d69e5abcc1b4 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -747,7 +747,7 @@ xfs_file_buffered_write(
 	 */
 	if (ret == -EDQUOT && !cleared_space) {
 		xfs_iunlock(ip, iolock);
-		cleared_space = xfs_inode_free_quota_blocks(ip);
+		cleared_space = xfs_blockgc_free_quota(ip);
 		if (cleared_space)
 			goto write_retry;
 		iolock = 0;
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 10c1a0dee17d..aba901d5637b 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1396,61 +1396,6 @@ xfs_icache_free_eofblocks(
 			XFS_ICI_EOFBLOCKS_TAG);
 }
 
-/*
- * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
- * with multiple quotas, we don't know exactly which quota caused an allocation
- * failure. We make a best effort by including each quota under low free space
- * conditions (less than 1% free space) in the scan.
- */
-bool
-xfs_inode_free_quota_blocks(
-	struct xfs_inode	*ip)
-{
-	struct xfs_eofblocks	eofb = {0};
-	struct xfs_dquot	*dq;
-	bool			do_work = false;
-
-	/*
-	 * Run a sync scan to increase effectiveness and use the union filter to
-	 * cover all applicable quotas in a single scan.
-	 */
-	eofb.eof_flags = XFS_EOF_FLAGS_UNION | XFS_EOF_FLAGS_SYNC;
-
-	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_uid = VFS_I(ip)->i_uid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
-			do_work = true;
-		}
-	}
-
-	if (XFS_IS_GQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_GROUP);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_gid = VFS_I(ip)->i_gid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
-			do_work = true;
-		}
-	}
-
-	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_prid = ip->i_d.di_projid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
-			do_work = true;
-		}
-	}
-
-	if (!do_work)
-		return false;
-
-	xfs_icache_free_eofblocks(ip->i_mount, &eofb);
-	xfs_icache_free_cowblocks(ip->i_mount, &eofb);
-	return true;
-}
-
 static inline unsigned long
 xfs_iflag_for_tag(
 	int		tag)
@@ -1699,3 +1644,58 @@ xfs_start_block_reaping(
 	xfs_queue_eofblocks(mp);
 	xfs_queue_cowblocks(mp);
 }
+
+/*
+ * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
+ * with multiple quotas, we don't know exactly which quota caused an allocation
+ * failure. We make a best effort by including each quota under low free space
+ * conditions (less than 1% free space) in the scan.
+ */
+bool
+xfs_blockgc_free_quota(
+	struct xfs_inode	*ip)
+{
+	struct xfs_eofblocks	eofb = {0};
+	struct xfs_dquot	*dq;
+	bool			do_work = false;
+
+	/*
+	 * Run a sync scan to increase effectiveness and use the union filter to
+	 * cover all applicable quotas in a single scan.
+	 */
+	eofb.eof_flags = XFS_EOF_FLAGS_UNION | XFS_EOF_FLAGS_SYNC;
+
+	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_uid = VFS_I(ip)->i_uid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
+			do_work = true;
+		}
+	}
+
+	if (XFS_IS_GQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_GROUP);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_gid = VFS_I(ip)->i_gid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
+			do_work = true;
+		}
+	}
+
+	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_prid = ip->i_d.di_projid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
+			do_work = true;
+		}
+	}
+
+	if (!do_work)
+		return false;
+
+	xfs_icache_free_eofblocks(ip->i_mount, &eofb);
+	xfs_icache_free_cowblocks(ip->i_mount, &eofb);
+	return true;
+}
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 3f7ddbca8638..21b726a05b0d 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -54,7 +54,7 @@ long xfs_reclaim_inodes_nr(struct xfs_mount *mp, int nr_to_scan);
 
 void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
 
-bool xfs_inode_free_quota_blocks(struct xfs_inode *ip);
+bool xfs_blockgc_free_quota(struct xfs_inode *ip);
 
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
 void xfs_inode_clear_eofblocks_tag(struct xfs_inode *ip);


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

* [PATCH 05/12] xfs: pass flags and return gc errors from xfs_blockgc_free_quota
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (3 preceding siblings ...)
  2021-02-01  2:05 ` [PATCH 04/12] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts Darrick J. Wong
@ 2021-02-01  2:05 ` Darrick J. Wong
  2021-02-01  2:06 ` [PATCH 06/12] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent Darrick J. Wong
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:05 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Change the signature of xfs_blockgc_free_quota in preparation for the
next few patches.  Callers can now pass EOF_FLAGS into the function to
control scan parameters; and the function will now pass back any
corruption errors seen while scanning, though for our retry loops we'll
just try again unconditionally.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c   |   10 +++++-----
 fs/xfs/xfs_icache.c |   26 +++++++++++++++++---------
 fs/xfs/xfs_icache.h |    2 +-
 3 files changed, 23 insertions(+), 15 deletions(-)


diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index d69e5abcc1b4..3be0b1d81325 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -743,14 +743,14 @@ xfs_file_buffered_write(
 	 * metadata space. This reduces the chances that the eofblocks scan
 	 * waits on dirty mappings. Since xfs_flush_inodes() is serialized, this
 	 * also behaves as a filter to prevent too many eofblocks scans from
-	 * running at the same time.
+	 * running at the same time.  Use a synchronous scan to increase the
+	 * effectiveness of the scan.
 	 */
 	if (ret == -EDQUOT && !cleared_space) {
 		xfs_iunlock(ip, iolock);
-		cleared_space = xfs_blockgc_free_quota(ip);
-		if (cleared_space)
-			goto write_retry;
-		iolock = 0;
+		xfs_blockgc_free_quota(ip, XFS_EOF_FLAGS_SYNC);
+		cleared_space = true;
+		goto write_retry;
 	} else if (ret == -ENOSPC && !cleared_space) {
 		struct xfs_eofblocks eofb = {0};
 
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index aba901d5637b..4a074aa12b52 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1650,20 +1650,26 @@ xfs_start_block_reaping(
  * with multiple quotas, we don't know exactly which quota caused an allocation
  * failure. We make a best effort by including each quota under low free space
  * conditions (less than 1% free space) in the scan.
+ *
+ * Callers must not hold any inode's ILOCK.  If requesting a synchronous scan
+ * (XFS_EOF_FLAGS_SYNC), the caller also must not hold any inode's IOLOCK or
+ * MMAPLOCK.
  */
-bool
+int
 xfs_blockgc_free_quota(
-	struct xfs_inode	*ip)
+	struct xfs_inode	*ip,
+	unsigned int		eof_flags)
 {
 	struct xfs_eofblocks	eofb = {0};
 	struct xfs_dquot	*dq;
 	bool			do_work = false;
+	int			error;
 
 	/*
-	 * Run a sync scan to increase effectiveness and use the union filter to
-	 * cover all applicable quotas in a single scan.
+	 * Run a scan to free blocks using the union filter to cover all
+	 * applicable quotas in a single scan.
 	 */
-	eofb.eof_flags = XFS_EOF_FLAGS_UNION | XFS_EOF_FLAGS_SYNC;
+	eofb.eof_flags = XFS_EOF_FLAGS_UNION | eof_flags;
 
 	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
 		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
@@ -1693,9 +1699,11 @@ xfs_blockgc_free_quota(
 	}
 
 	if (!do_work)
-		return false;
+		return 0;
 
-	xfs_icache_free_eofblocks(ip->i_mount, &eofb);
-	xfs_icache_free_cowblocks(ip->i_mount, &eofb);
-	return true;
+	error = xfs_icache_free_eofblocks(ip->i_mount, &eofb);
+	if (error)
+		return error;
+
+	return xfs_icache_free_cowblocks(ip->i_mount, &eofb);
 }
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 21b726a05b0d..d64ea8f5c589 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -54,7 +54,7 @@ long xfs_reclaim_inodes_nr(struct xfs_mount *mp, int nr_to_scan);
 
 void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
 
-bool xfs_blockgc_free_quota(struct xfs_inode *ip);
+int xfs_blockgc_free_quota(struct xfs_inode *ip, unsigned int eof_flags);
 
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
 void xfs_inode_clear_eofblocks_tag(struct xfs_inode *ip);


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

* [PATCH 06/12] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (4 preceding siblings ...)
  2021-02-01  2:05 ` [PATCH 05/12] xfs: pass flags and return gc errors from xfs_blockgc_free_quota Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-01 12:24   ` Christoph Hellwig
  2021-02-01  2:06 ` [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Now that we've converted xfs_reflink_remap_extent to use the new
xfs_trans_alloc_inode API, we can focus on its slightly unusual behavior
with regard to quota reservations.

Since it's valid to remap written blocks into a hole, we must be able to
increase the quota count by the number of blocks in the mapping.
However, the incore space reservation process requires us to supply an
asymptotic guess before we can gain exclusive access to resources.  We'd
like to reserve all the quota we need up front, but we also don't want
to fail a written -> allocated remap operation unnecessarily.

The solution is to make the remap_extents function call the transaction
allocation function twice.  The first time we ask to reserve enough
space and quota to handle the absolute worst case situation, but if that
fails, we can fall back to the old strategy: ask for the bare minimum
space reservation upfront and increase the quota reservation later if we
need to.

Later in this patchset we change the transaction and quota code to try
to reclaim space if we cannot reserve free space or quota.
Restructuring the remap_extent function in this manner means that if the
fallback increase fails, we can pass that back to the caller knowing
that the transaction allocation already tried freeing space.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_reflink.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 27f875fa7a0d..086866f6e71f 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -991,6 +991,7 @@ xfs_reflink_remap_extent(
 	xfs_off_t		newlen;
 	int64_t			qdelta = 0;
 	unsigned int		resblks;
+	bool			quota_reserved = true;
 	bool			smap_real;
 	bool			dmap_written = xfs_bmap_is_written_extent(dmap);
 	int			iext_delta = 0;
@@ -1006,10 +1007,26 @@ xfs_reflink_remap_extent(
 	 * the same index in the bmap btree, so we only need a reservation for
 	 * one bmbt split if either thing is happening.  However, we haven't
 	 * locked the inode yet, so we reserve assuming this is the case.
+	 *
+	 * The first allocation call tries to reserve enough space to handle
+	 * mapping dmap into a sparse part of the file plus the bmbt split.  We
+	 * haven't locked the inode or read the existing mapping yet, so we do
+	 * not know for sure that we need the space.  This should succeed most
+	 * of the time.
+	 *
+	 * If the first attempt fails, try again but reserving only enough
+	 * space to handle a bmbt split.  This is the hard minimum requirement,
+	 * and we revisit quota reservations later when we know more about what
+	 * we're remapping.
 	 */
 	resblks = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
-	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, resblks, 0,
-			false, &tp);
+	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
+			resblks + dmap->br_blockcount, 0, false, &tp);
+	if (error == -EDQUOT || error == -ENOSPC) {
+		quota_reserved = false;
+		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
+				resblks, 0, false, &tp);
+	}
 	if (error)
 		goto out;
 
@@ -1076,7 +1093,7 @@ xfs_reflink_remap_extent(
 	 * before we started.  That should have removed all the delalloc
 	 * reservations, but we code defensively.
 	 */
-	if (!smap_real && dmap_written) {
+	if (!quota_reserved && !smap_real && dmap_written) {
 		error = xfs_trans_reserve_quota_nblks(tp, ip,
 				dmap->br_blockcount, 0, false);
 		if (error)


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

* [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (5 preceding siblings ...)
  2021-02-01  2:06 ` [PATCH 06/12] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-01 12:32   ` Christoph Hellwig
  2021-02-02 15:38   ` Brian Foster
  2021-02-01  2:06 ` [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation Darrick J. Wong
                   ` (4 subsequent siblings)
  11 siblings, 2 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

If a fs modification (data write, reflink, xattr set, fallocate, etc.)
is unable to reserve enough quota to handle the modification, try
clearing whatever space the filesystem might have been hanging onto in
the hopes of speeding up the filesystem.  The flushing behavior will
become particularly important when we add deferred inode inactivation
because that will increase the amount of space that isn't actively tied
to user data.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_reflink.c |    5 +++++
 fs/xfs/xfs_trans.c   |   10 ++++++++++
 2 files changed, 15 insertions(+)


diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 086866f6e71f..725c7d8e4438 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1092,6 +1092,11 @@ xfs_reflink_remap_extent(
 	 * count.  This is suboptimal, but the VFS flushed the dest range
 	 * before we started.  That should have removed all the delalloc
 	 * reservations, but we code defensively.
+	 *
+	 * xfs_trans_alloc_inode above already tried to grab an even larger
+	 * quota reservation, and kicked off a blockgc scan if it couldn't.
+	 * If we can't get a potentially smaller quota reservation now, we're
+	 * done.
 	 */
 	if (!quota_reserved && !smap_real && dmap_written) {
 		error = xfs_trans_reserve_quota_nblks(tp, ip,
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 466e1c86767f..f62c1c5f210f 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -23,6 +23,7 @@
 #include "xfs_inode.h"
 #include "xfs_dquot_item.h"
 #include "xfs_dquot.h"
+#include "xfs_icache.h"
 
 kmem_zone_t	*xfs_trans_zone;
 
@@ -1046,8 +1047,10 @@ xfs_trans_alloc_inode(
 {
 	struct xfs_trans	*tp;
 	struct xfs_mount	*mp = ip->i_mount;
+	bool			retried = false;
 	int			error;
 
+retry:
 	error = xfs_trans_alloc(mp, resv, dblocks,
 			rblocks / mp->m_sb.sb_rextsize,
 			force ? XFS_TRANS_RESERVE : 0, &tp);
@@ -1065,6 +1068,13 @@ xfs_trans_alloc_inode(
 	}
 
 	error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force);
+	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
+		xfs_trans_cancel(tp);
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+		xfs_blockgc_free_quota(ip, 0);
+		retried = true;
+		goto retry;
+	}
 	if (error)
 		goto out_cancel;
 


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

* [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (6 preceding siblings ...)
  2021-02-01  2:06 ` [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-01 12:35   ` Christoph Hellwig
  2021-02-02 15:39   ` Brian Foster
  2021-02-01  2:06 ` [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown Darrick J. Wong
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

If an inode creation is unable to reserve enough quota to handle the
modification, try clearing whatever space the filesystem might have been
hanging onto in the hopes of speeding up the filesystem.  The flushing
behavior will become particularly important when we add deferred inode
inactivation because that will increase the amount of space that isn't
actively tied to user data.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_icache.c |   73 ++++++++++++++++++++++++++++++---------------------
 fs/xfs/xfs_icache.h |    2 +
 fs/xfs/xfs_trans.c  |    8 ++++++
 3 files changed, 53 insertions(+), 30 deletions(-)


diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 4a074aa12b52..cd369dd48818 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1646,64 +1646,77 @@ xfs_start_block_reaping(
 }
 
 /*
- * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
- * with multiple quotas, we don't know exactly which quota caused an allocation
- * failure. We make a best effort by including each quota under low free space
- * conditions (less than 1% free space) in the scan.
+ * Run cow/eofblocks scans on the supplied dquots.  We don't know exactly which
+ * quota caused an allocation failure, so we make a best effort by including
+ * each quota under low free space conditions (less than 1% free space) in the
+ * scan.
  *
  * Callers must not hold any inode's ILOCK.  If requesting a synchronous scan
  * (XFS_EOF_FLAGS_SYNC), the caller also must not hold any inode's IOLOCK or
  * MMAPLOCK.
  */
 int
-xfs_blockgc_free_quota(
-	struct xfs_inode	*ip,
+xfs_blockgc_free_dquots(
+	struct xfs_dquot	*udqp,
+	struct xfs_dquot	*gdqp,
+	struct xfs_dquot	*pdqp,
 	unsigned int		eof_flags)
 {
 	struct xfs_eofblocks	eofb = {0};
-	struct xfs_dquot	*dq;
+	struct xfs_mount	*mp = NULL;
 	bool			do_work = false;
 	int			error;
 
+	if (!udqp && !gdqp && !pdqp)
+		return 0;
+	if (udqp)
+		mp = udqp->q_mount;
+	if (!mp && gdqp)
+		mp = gdqp->q_mount;
+	if (!mp && pdqp)
+		mp = pdqp->q_mount;
+
 	/*
 	 * Run a scan to free blocks using the union filter to cover all
 	 * applicable quotas in a single scan.
 	 */
 	eofb.eof_flags = XFS_EOF_FLAGS_UNION | eof_flags;
 
-	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_uid = VFS_I(ip)->i_uid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
-			do_work = true;
-		}
+	if (XFS_IS_UQUOTA_ENFORCED(mp) && udqp && xfs_dquot_lowsp(udqp)) {
+		eofb.eof_uid = make_kuid(mp->m_super->s_user_ns, udqp->q_id);
+		eofb.eof_flags |= XFS_EOF_FLAGS_UID;
+		do_work = true;
 	}
 
-	if (XFS_IS_GQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_GROUP);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_gid = VFS_I(ip)->i_gid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
-			do_work = true;
-		}
+	if (XFS_IS_UQUOTA_ENFORCED(mp) && gdqp && xfs_dquot_lowsp(gdqp)) {
+		eofb.eof_gid = make_kgid(mp->m_super->s_user_ns, gdqp->q_id);
+		eofb.eof_flags |= XFS_EOF_FLAGS_GID;
+		do_work = true;
 	}
 
-	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_prid = ip->i_d.di_projid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
-			do_work = true;
-		}
+	if (XFS_IS_PQUOTA_ENFORCED(mp) && pdqp && xfs_dquot_lowsp(pdqp)) {
+		eofb.eof_prid = pdqp->q_id;
+		eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
+		do_work = true;
 	}
 
 	if (!do_work)
 		return 0;
 
-	error = xfs_icache_free_eofblocks(ip->i_mount, &eofb);
+	error = xfs_icache_free_eofblocks(mp, &eofb);
 	if (error)
 		return error;
 
-	return xfs_icache_free_cowblocks(ip->i_mount, &eofb);
+	return xfs_icache_free_cowblocks(mp, &eofb);
+}
+
+/* Run cow/eofblocks scans on the quotas attached to the inode. */
+int
+xfs_blockgc_free_quota(
+	struct xfs_inode	*ip,
+	unsigned int		eof_flags)
+{
+	return xfs_blockgc_free_dquots(xfs_inode_dquot(ip, XFS_DQTYPE_USER),
+			xfs_inode_dquot(ip, XFS_DQTYPE_GROUP),
+			xfs_inode_dquot(ip, XFS_DQTYPE_PROJ), eof_flags);
 }
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index d64ea8f5c589..5f520de637f6 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -54,6 +54,8 @@ long xfs_reclaim_inodes_nr(struct xfs_mount *mp, int nr_to_scan);
 
 void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
 
+int xfs_blockgc_free_dquots(struct xfs_dquot *udqp, struct xfs_dquot *gdqp,
+		struct xfs_dquot *pdqp, unsigned int eof_flags);
 int xfs_blockgc_free_quota(struct xfs_inode *ip, unsigned int eof_flags);
 
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index f62c1c5f210f..ee3cb916c5c9 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1102,13 +1102,21 @@ xfs_trans_alloc_icreate(
 	struct xfs_trans	**tpp)
 {
 	struct xfs_trans	*tp;
+	bool			retried = false;
 	int			error;
 
+retry:
 	error = xfs_trans_alloc(mp, resv, dblocks, 0, 0, &tp);
 	if (error)
 		return error;
 
 	error = xfs_trans_reserve_quota_icreate(tp, udqp, gdqp, pdqp, dblocks);
+	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
+		xfs_trans_cancel(tp);
+		xfs_blockgc_free_dquots(udqp, gdqp, pdqp, 0);
+		retried = true;
+		goto retry;
+	}
 	if (error) {
 		xfs_trans_cancel(tp);
 		return error;


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

* [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (7 preceding siblings ...)
  2021-02-01  2:06 ` [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-01 12:36   ` Christoph Hellwig
  2021-02-02 15:39   ` Brian Foster
  2021-02-01  2:06 ` [PATCH 10/12] xfs: add a tracepoint for blockgc scans Darrick J. Wong
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

If a file user, group, or project change is unable to reserve enough
quota to handle the modification, try clearing whatever space the
filesystem might have been hanging onto in the hopes of speeding up the
filesystem.  The flushing behavior will become particularly important
when we add deferred inode inactivation because that will increase the
amount of space that isn't actively tied to user data.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_trans.c |    9 +++++++++
 1 file changed, 9 insertions(+)


diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index ee3cb916c5c9..3203841ab19b 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1149,8 +1149,10 @@ xfs_trans_alloc_ichange(
 	struct xfs_dquot	*new_udqp;
 	struct xfs_dquot	*new_gdqp;
 	struct xfs_dquot	*new_pdqp;
+	bool			retried = false;
 	int			error;
 
+retry:
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp);
 	if (error)
 		return error;
@@ -1175,6 +1177,13 @@ xfs_trans_alloc_ichange(
 	if (new_udqp || new_gdqp || new_pdqp) {
 		error = xfs_trans_reserve_quota_chown(tp, ip, new_udqp,
 				new_gdqp, new_pdqp, force);
+		if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
+			xfs_trans_cancel(tp);
+			xfs_blockgc_free_dquots(new_udqp, new_gdqp, new_pdqp,
+					0);
+			retried = true;
+			goto retry;
+		}
 		if (error)
 			goto out_cancel;
 	}


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

* [PATCH 10/12] xfs: add a tracepoint for blockgc scans
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (8 preceding siblings ...)
  2021-02-01  2:06 ` [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-01  2:06 ` [PATCH 11/12] xfs: refactor xfs_icache_free_{eof,cow}blocks call sites Darrick J. Wong
  2021-02-01  2:06 ` [PATCH 12/12] xfs: flush speculative space allocations when we run out of space Darrick J. Wong
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Add some tracepoints so that we can observe when the speculative
preallocation garbage collector runs.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_ioctl.c |    2 ++
 fs/xfs/xfs_trace.c |    1 +
 fs/xfs/xfs_trace.h |   41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)


diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 38ee66d999d8..3ddefb685ac6 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -2339,6 +2339,8 @@ xfs_file_ioctl(
 		if (error)
 			return error;
 
+		trace_xfs_ioc_free_eofblocks(mp, &keofb, _RET_IP_);
+
 		sb_start_write(mp->m_super);
 		error = xfs_icache_free_eofblocks(mp, &keofb);
 		sb_end_write(mp->m_super);
diff --git a/fs/xfs/xfs_trace.c b/fs/xfs/xfs_trace.c
index 120398a37c2a..9b8d703dc9fd 100644
--- a/fs/xfs/xfs_trace.c
+++ b/fs/xfs/xfs_trace.c
@@ -29,6 +29,7 @@
 #include "xfs_filestream.h"
 #include "xfs_fsmap.h"
 #include "xfs_btree_staging.h"
+#include "xfs_icache.h"
 
 /*
  * We include this last to have the helpers above available for the trace
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 407c3a5208ab..38649e3341cb 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -37,6 +37,7 @@ struct xfs_trans_res;
 struct xfs_inobt_rec_incore;
 union xfs_btree_ptr;
 struct xfs_dqtrx;
+struct xfs_eofblocks;
 
 #define XFS_ATTR_FILTER_FLAGS \
 	{ XFS_ATTR_ROOT,	"ROOT" }, \
@@ -3888,6 +3889,46 @@ DEFINE_EVENT(xfs_timestamp_range_class, name, \
 DEFINE_TIMESTAMP_RANGE_EVENT(xfs_inode_timestamp_range);
 DEFINE_TIMESTAMP_RANGE_EVENT(xfs_quota_expiry_range);
 
+DECLARE_EVENT_CLASS(xfs_eofblocks_class,
+	TP_PROTO(struct xfs_mount *mp, struct xfs_eofblocks *eofb,
+		 unsigned long caller_ip),
+	TP_ARGS(mp, eofb, caller_ip),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(__u32, flags)
+		__field(uint32_t, uid)
+		__field(uint32_t, gid)
+		__field(prid_t, prid)
+		__field(__u64, min_file_size)
+		__field(unsigned long, caller_ip)
+	),
+	TP_fast_assign(
+		__entry->dev = mp->m_super->s_dev;
+		__entry->flags = eofb ? eofb->eof_flags : 0;
+		__entry->uid = eofb ? from_kuid(mp->m_super->s_user_ns,
+						eofb->eof_uid) : 0;
+		__entry->gid = eofb ? from_kgid(mp->m_super->s_user_ns,
+						eofb->eof_gid) : 0;
+		__entry->prid = eofb ? eofb->eof_prid : 0;
+		__entry->min_file_size = eofb ? eofb->eof_min_file_size : 0;
+		__entry->caller_ip = caller_ip;
+	),
+	TP_printk("dev %d:%d flags 0x%x uid %u gid %u prid %u minsize %llu caller %pS",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->flags,
+		  __entry->uid,
+		  __entry->gid,
+		  __entry->prid,
+		  __entry->min_file_size,
+		  (char *)__entry->caller_ip)
+);
+#define DEFINE_EOFBLOCKS_EVENT(name)	\
+DEFINE_EVENT(xfs_eofblocks_class, name,	\
+	TP_PROTO(struct xfs_mount *mp, struct xfs_eofblocks *eofb, \
+		 unsigned long caller_ip), \
+	TP_ARGS(mp, eofb, caller_ip))
+DEFINE_EOFBLOCKS_EVENT(xfs_ioc_free_eofblocks);
+
 #endif /* _TRACE_XFS_H */
 
 #undef TRACE_INCLUDE_PATH


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

* [PATCH 11/12] xfs: refactor xfs_icache_free_{eof,cow}blocks call sites
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (9 preceding siblings ...)
  2021-02-01  2:06 ` [PATCH 10/12] xfs: add a tracepoint for blockgc scans Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-01  2:06 ` [PATCH 12/12] xfs: flush speculative space allocations when we run out of space Darrick J. Wong
  11 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

In anticipation of more restructuring of the eof/cowblocks gc code,
refactor calling of those two functions into a single internal helper
function, then present a new standard interface to purge speculative
block preallocations and start shifting higher level code to use that.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c   |    3 +--
 fs/xfs/xfs_icache.c |   39 +++++++++++++++++++++++++++++++++------
 fs/xfs/xfs_icache.h |    1 +
 fs/xfs/xfs_trace.h  |    1 +
 4 files changed, 36 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 3be0b1d81325..dc91973c0b4f 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -759,8 +759,7 @@ xfs_file_buffered_write(
 
 		xfs_iunlock(ip, iolock);
 		eofb.eof_flags = XFS_EOF_FLAGS_SYNC;
-		xfs_icache_free_eofblocks(ip->i_mount, &eofb);
-		xfs_icache_free_cowblocks(ip->i_mount, &eofb);
+		xfs_blockgc_free_space(ip->i_mount, &eofb);
 		goto write_retry;
 	}
 
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index cd369dd48818..97c15fcdd6f7 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1645,6 +1645,38 @@ xfs_start_block_reaping(
 	xfs_queue_cowblocks(mp);
 }
 
+/* Scan all incore inodes for block preallocations that we can remove. */
+static inline int
+xfs_blockgc_scan(
+	struct xfs_mount	*mp,
+	struct xfs_eofblocks	*eofb)
+{
+	int			error;
+
+	error = xfs_icache_free_eofblocks(mp, eofb);
+	if (error)
+		return error;
+
+	error = xfs_icache_free_cowblocks(mp, eofb);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+/*
+ * Try to free space in the filesystem by purging eofblocks and cowblocks.
+ */
+int
+xfs_blockgc_free_space(
+	struct xfs_mount	*mp,
+	struct xfs_eofblocks	*eofb)
+{
+	trace_xfs_blockgc_free_space(mp, eofb, _RET_IP_);
+
+	return xfs_blockgc_scan(mp, eofb);
+}
+
 /*
  * Run cow/eofblocks scans on the supplied dquots.  We don't know exactly which
  * quota caused an allocation failure, so we make a best effort by including
@@ -1665,7 +1697,6 @@ xfs_blockgc_free_dquots(
 	struct xfs_eofblocks	eofb = {0};
 	struct xfs_mount	*mp = NULL;
 	bool			do_work = false;
-	int			error;
 
 	if (!udqp && !gdqp && !pdqp)
 		return 0;
@@ -1703,11 +1734,7 @@ xfs_blockgc_free_dquots(
 	if (!do_work)
 		return 0;
 
-	error = xfs_icache_free_eofblocks(mp, &eofb);
-	if (error)
-		return error;
-
-	return xfs_icache_free_cowblocks(mp, &eofb);
+	return xfs_blockgc_free_space(mp, &eofb);
 }
 
 /* Run cow/eofblocks scans on the quotas attached to the inode. */
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 5f520de637f6..583c132ae0fb 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -57,6 +57,7 @@ void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
 int xfs_blockgc_free_dquots(struct xfs_dquot *udqp, struct xfs_dquot *gdqp,
 		struct xfs_dquot *pdqp, unsigned int eof_flags);
 int xfs_blockgc_free_quota(struct xfs_inode *ip, unsigned int eof_flags);
+int xfs_blockgc_free_space(struct xfs_mount *mp, struct xfs_eofblocks *eofb);
 
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
 void xfs_inode_clear_eofblocks_tag(struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 38649e3341cb..27929c6ca43a 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3928,6 +3928,7 @@ DEFINE_EVENT(xfs_eofblocks_class, name,	\
 		 unsigned long caller_ip), \
 	TP_ARGS(mp, eofb, caller_ip))
 DEFINE_EOFBLOCKS_EVENT(xfs_ioc_free_eofblocks);
+DEFINE_EOFBLOCKS_EVENT(xfs_blockgc_free_space);
 
 #endif /* _TRACE_XFS_H */
 


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

* [PATCH 12/12] xfs: flush speculative space allocations when we run out of space
  2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
                   ` (10 preceding siblings ...)
  2021-02-01  2:06 ` [PATCH 11/12] xfs: refactor xfs_icache_free_{eof,cow}blocks call sites Darrick J. Wong
@ 2021-02-01  2:06 ` Darrick J. Wong
  2021-02-02 15:39   ` Brian Foster
  11 siblings, 1 reply; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01  2:06 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

If a fs modification (creation, file write, reflink, etc.) is unable to
reserve enough space to handle the modification, try clearing whatever
space the filesystem might have been hanging onto in the hopes of
speeding up the filesystem.  The flushing behavior will become
particularly important when we add deferred inode inactivation because
that will increase the amount of space that isn't actively tied to user
data.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trans.c |   11 +++++++++++
 1 file changed, 11 insertions(+)


diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 3203841ab19b..973354647298 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -289,6 +289,17 @@ xfs_trans_alloc(
 	tp->t_firstblock = NULLFSBLOCK;
 
 	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
+	if (error == -ENOSPC) {
+		/*
+		 * We weren't able to reserve enough space for the transaction.
+		 * Flush the other speculative space allocations to free space.
+		 * Do not perform a synchronous scan because callers can hold
+		 * other locks.
+		 */
+		error = xfs_blockgc_free_space(mp, NULL);
+		if (!error)
+			error = xfs_trans_reserve(tp, resp, blocks, rtextents);
+	}
 	if (error) {
 		xfs_trans_cancel(tp);
 		return error;


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

* Re: [PATCH 06/12] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent
  2021-02-01  2:06 ` [PATCH 06/12] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent Darrick J. Wong
@ 2021-02-01 12:24   ` Christoph Hellwig
  0 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2021-02-01 12:24 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Brian Foster, linux-xfs, hch, david

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
  2021-02-01  2:06 ` [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
@ 2021-02-01 12:32   ` Christoph Hellwig
  2021-02-01 19:01     ` Darrick J. Wong
  2021-02-02 15:38   ` Brian Foster
  1 sibling, 1 reply; 26+ messages in thread
From: Christoph Hellwig @ 2021-02-01 12:32 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch, david, bfoster

On Sun, Jan 31, 2021 at 06:06:06PM -0800, Darrick J. Wong wrote:
> @@ -1046,8 +1047,10 @@ xfs_trans_alloc_inode(
>  {
>  	struct xfs_trans	*tp;
>  	struct xfs_mount	*mp = ip->i_mount;
> +	bool			retried = false;
>  	int			error;
>  
> +retry:
>  	error = xfs_trans_alloc(mp, resv, dblocks,
>  			rblocks / mp->m_sb.sb_rextsize,
>  			force ? XFS_TRANS_RESERVE : 0, &tp);
> @@ -1065,6 +1068,13 @@ xfs_trans_alloc_inode(
>  	}
>  
>  	error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force);
> +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {

Nit: writing this as

	if ((error == -EDQUOT || error == -ENOSPC) && !retried) {

would make reading the line a little bit easier at least to me because
it checks the variable assigned in the line above first.

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation
  2021-02-01  2:06 ` [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation Darrick J. Wong
@ 2021-02-01 12:35   ` Christoph Hellwig
  2021-02-01 19:03     ` Darrick J. Wong
  2021-02-02 15:39   ` Brian Foster
  1 sibling, 1 reply; 26+ messages in thread
From: Christoph Hellwig @ 2021-02-01 12:35 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch, david, bfoster

> +xfs_blockgc_free_dquots(
> +	struct xfs_dquot	*udqp,
> +	struct xfs_dquot	*gdqp,
> +	struct xfs_dquot	*pdqp,
>  	unsigned int		eof_flags)
>  {
>  	struct xfs_eofblocks	eofb = {0};
> -	struct xfs_dquot	*dq;
> +	struct xfs_mount	*mp = NULL;
>  	bool			do_work = false;
>  	int			error;
>  
> +	if (!udqp && !gdqp && !pdqp)
> +		return 0;
> +	if (udqp)
> +		mp = udqp->q_mount;
> +	if (!mp && gdqp)
> +		mp = gdqp->q_mount;
> +	if (!mp && pdqp)
> +		mp = pdqp->q_mount;

I think just passing the xfs_mount as the first argument would be a
little simpler and produce better code.

>  	error = xfs_trans_reserve_quota_icreate(tp, udqp, gdqp, pdqp, dblocks);
> +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {

Same minor nit as for the last patch.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown
  2021-02-01  2:06 ` [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown Darrick J. Wong
@ 2021-02-01 12:36   ` Christoph Hellwig
  2021-02-01 19:12     ` Darrick J. Wong
  2021-02-02 15:39   ` Brian Foster
  1 sibling, 1 reply; 26+ messages in thread
From: Christoph Hellwig @ 2021-02-01 12:36 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch, david, bfoster

On Sun, Jan 31, 2021 at 06:06:17PM -0800, Darrick J. Wong wrote:
> @@ -1175,6 +1177,13 @@ xfs_trans_alloc_ichange(
>  	if (new_udqp || new_gdqp || new_pdqp) {
>  		error = xfs_trans_reserve_quota_chown(tp, ip, new_udqp,
>  				new_gdqp, new_pdqp, force);
> +		if (!retried && (error == -EDQUOT || error == -ENOSPC)) {

One more :)

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
  2021-02-01 12:32   ` Christoph Hellwig
@ 2021-02-01 19:01     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01 19:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, david, bfoster

On Mon, Feb 01, 2021 at 12:32:45PM +0000, Christoph Hellwig wrote:
> On Sun, Jan 31, 2021 at 06:06:06PM -0800, Darrick J. Wong wrote:
> > @@ -1046,8 +1047,10 @@ xfs_trans_alloc_inode(
> >  {
> >  	struct xfs_trans	*tp;
> >  	struct xfs_mount	*mp = ip->i_mount;
> > +	bool			retried = false;
> >  	int			error;
> >  
> > +retry:
> >  	error = xfs_trans_alloc(mp, resv, dblocks,
> >  			rblocks / mp->m_sb.sb_rextsize,
> >  			force ? XFS_TRANS_RESERVE : 0, &tp);
> > @@ -1065,6 +1068,13 @@ xfs_trans_alloc_inode(
> >  	}
> >  
> >  	error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force);
> > +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> 
> Nit: writing this as
> 
> 	if ((error == -EDQUOT || error == -ENOSPC) && !retried) {
> 
> would make reading the line a little bit easier at least to me because
> it checks the variable assigned in the line above first.

Will fix in all three.

--D

> Otherwise this looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation
  2021-02-01 12:35   ` Christoph Hellwig
@ 2021-02-01 19:03     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01 19:03 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, david, bfoster

On Mon, Feb 01, 2021 at 12:35:40PM +0000, Christoph Hellwig wrote:
> > +xfs_blockgc_free_dquots(
> > +	struct xfs_dquot	*udqp,
> > +	struct xfs_dquot	*gdqp,
> > +	struct xfs_dquot	*pdqp,
> >  	unsigned int		eof_flags)
> >  {
> >  	struct xfs_eofblocks	eofb = {0};
> > -	struct xfs_dquot	*dq;
> > +	struct xfs_mount	*mp = NULL;
> >  	bool			do_work = false;
> >  	int			error;
> >  
> > +	if (!udqp && !gdqp && !pdqp)
> > +		return 0;
> > +	if (udqp)
> > +		mp = udqp->q_mount;
> > +	if (!mp && gdqp)
> > +		mp = gdqp->q_mount;
> > +	if (!mp && pdqp)
> > +		mp = pdqp->q_mount;
> 
> I think just passing the xfs_mount as the first argument would be a
> little simpler and produce better code.

Changed.

> >  	error = xfs_trans_reserve_quota_icreate(tp, udqp, gdqp, pdqp, dblocks);
> > +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> 
> Same minor nit as for the last patch.

This too.

--D

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown
  2021-02-01 12:36   ` Christoph Hellwig
@ 2021-02-01 19:12     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-01 19:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, david, bfoster

On Mon, Feb 01, 2021 at 12:36:37PM +0000, Christoph Hellwig wrote:
> On Sun, Jan 31, 2021 at 06:06:17PM -0800, Darrick J. Wong wrote:
> > @@ -1175,6 +1177,13 @@ xfs_trans_alloc_ichange(
> >  	if (new_udqp || new_gdqp || new_pdqp) {
> >  		error = xfs_trans_reserve_quota_chown(tp, ip, new_udqp,
> >  				new_gdqp, new_pdqp, force);
> > +		if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> 
> One more :)
> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Cool, thanks for reviewing!

--D

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

* Re: [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
  2021-02-01  2:06 ` [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
  2021-02-01 12:32   ` Christoph Hellwig
@ 2021-02-02 15:38   ` Brian Foster
  2021-02-02 16:53     ` Darrick J. Wong
  1 sibling, 1 reply; 26+ messages in thread
From: Brian Foster @ 2021-02-02 15:38 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch, david

On Sun, Jan 31, 2021 at 06:06:06PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If a fs modification (data write, reflink, xattr set, fallocate, etc.)
> is unable to reserve enough quota to handle the modification, try
> clearing whatever space the filesystem might have been hanging onto in
> the hopes of speeding up the filesystem.  The flushing behavior will
> become particularly important when we add deferred inode inactivation
> because that will increase the amount of space that isn't actively tied
> to user data.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

(FWIW, I'm reviewing the patches from your reclaim-space-harder-5.12
branch as of this morning, which look like they have some deltas from
the posted versions based on Christoph's feedback.)

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_reflink.c |    5 +++++
>  fs/xfs/xfs_trans.c   |   10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 086866f6e71f..725c7d8e4438 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1092,6 +1092,11 @@ xfs_reflink_remap_extent(
>  	 * count.  This is suboptimal, but the VFS flushed the dest range
>  	 * before we started.  That should have removed all the delalloc
>  	 * reservations, but we code defensively.
> +	 *
> +	 * xfs_trans_alloc_inode above already tried to grab an even larger
> +	 * quota reservation, and kicked off a blockgc scan if it couldn't.
> +	 * If we can't get a potentially smaller quota reservation now, we're
> +	 * done.
>  	 */
>  	if (!quota_reserved && !smap_real && dmap_written) {
>  		error = xfs_trans_reserve_quota_nblks(tp, ip,
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 466e1c86767f..f62c1c5f210f 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -23,6 +23,7 @@
>  #include "xfs_inode.h"
>  #include "xfs_dquot_item.h"
>  #include "xfs_dquot.h"
> +#include "xfs_icache.h"
>  
>  kmem_zone_t	*xfs_trans_zone;
>  
> @@ -1046,8 +1047,10 @@ xfs_trans_alloc_inode(
>  {
>  	struct xfs_trans	*tp;
>  	struct xfs_mount	*mp = ip->i_mount;
> +	bool			retried = false;
>  	int			error;
>  
> +retry:
>  	error = xfs_trans_alloc(mp, resv, dblocks,
>  			rblocks / mp->m_sb.sb_rextsize,
>  			force ? XFS_TRANS_RESERVE : 0, &tp);
> @@ -1065,6 +1068,13 @@ xfs_trans_alloc_inode(
>  	}
>  
>  	error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force);
> +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> +		xfs_trans_cancel(tp);
> +		xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +		xfs_blockgc_free_quota(ip, 0);
> +		retried = true;
> +		goto retry;
> +	}
>  	if (error)
>  		goto out_cancel;
>  
> 


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

* Re: [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation
  2021-02-01  2:06 ` [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation Darrick J. Wong
  2021-02-01 12:35   ` Christoph Hellwig
@ 2021-02-02 15:39   ` Brian Foster
  1 sibling, 0 replies; 26+ messages in thread
From: Brian Foster @ 2021-02-02 15:39 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch, david

On Sun, Jan 31, 2021 at 06:06:11PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If an inode creation is unable to reserve enough quota to handle the
> modification, try clearing whatever space the filesystem might have been
> hanging onto in the hopes of speeding up the filesystem.  The flushing
> behavior will become particularly important when we add deferred inode
> inactivation because that will increase the amount of space that isn't
> actively tied to user data.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_icache.c |   73 ++++++++++++++++++++++++++++++---------------------
>  fs/xfs/xfs_icache.h |    2 +
>  fs/xfs/xfs_trans.c  |    8 ++++++
>  3 files changed, 53 insertions(+), 30 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 4a074aa12b52..cd369dd48818 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1646,64 +1646,77 @@ xfs_start_block_reaping(
>  }
>  
>  /*
> - * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
> - * with multiple quotas, we don't know exactly which quota caused an allocation
> - * failure. We make a best effort by including each quota under low free space
> - * conditions (less than 1% free space) in the scan.
> + * Run cow/eofblocks scans on the supplied dquots.  We don't know exactly which
> + * quota caused an allocation failure, so we make a best effort by including
> + * each quota under low free space conditions (less than 1% free space) in the
> + * scan.
>   *
>   * Callers must not hold any inode's ILOCK.  If requesting a synchronous scan
>   * (XFS_EOF_FLAGS_SYNC), the caller also must not hold any inode's IOLOCK or
>   * MMAPLOCK.
>   */
>  int
> -xfs_blockgc_free_quota(
> -	struct xfs_inode	*ip,
> +xfs_blockgc_free_dquots(
> +	struct xfs_dquot	*udqp,
> +	struct xfs_dquot	*gdqp,
> +	struct xfs_dquot	*pdqp,
>  	unsigned int		eof_flags)
>  {
>  	struct xfs_eofblocks	eofb = {0};
> -	struct xfs_dquot	*dq;
> +	struct xfs_mount	*mp = NULL;
>  	bool			do_work = false;
>  	int			error;
>  
> +	if (!udqp && !gdqp && !pdqp)
> +		return 0;
> +	if (udqp)
> +		mp = udqp->q_mount;
> +	if (!mp && gdqp)
> +		mp = gdqp->q_mount;
> +	if (!mp && pdqp)
> +		mp = pdqp->q_mount;
> +
>  	/*
>  	 * Run a scan to free blocks using the union filter to cover all
>  	 * applicable quotas in a single scan.
>  	 */
>  	eofb.eof_flags = XFS_EOF_FLAGS_UNION | eof_flags;
>  
> -	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
> -		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
> -		if (dq && xfs_dquot_lowsp(dq)) {
> -			eofb.eof_uid = VFS_I(ip)->i_uid;
> -			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
> -			do_work = true;
> -		}
> +	if (XFS_IS_UQUOTA_ENFORCED(mp) && udqp && xfs_dquot_lowsp(udqp)) {
> +		eofb.eof_uid = make_kuid(mp->m_super->s_user_ns, udqp->q_id);
> +		eofb.eof_flags |= XFS_EOF_FLAGS_UID;
> +		do_work = true;
>  	}
>  
> -	if (XFS_IS_GQUOTA_ENFORCED(ip->i_mount)) {
> -		dq = xfs_inode_dquot(ip, XFS_DQTYPE_GROUP);
> -		if (dq && xfs_dquot_lowsp(dq)) {
> -			eofb.eof_gid = VFS_I(ip)->i_gid;
> -			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
> -			do_work = true;
> -		}
> +	if (XFS_IS_UQUOTA_ENFORCED(mp) && gdqp && xfs_dquot_lowsp(gdqp)) {
> +		eofb.eof_gid = make_kgid(mp->m_super->s_user_ns, gdqp->q_id);
> +		eofb.eof_flags |= XFS_EOF_FLAGS_GID;
> +		do_work = true;
>  	}
>  
> -	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
> -		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
> -		if (dq && xfs_dquot_lowsp(dq)) {
> -			eofb.eof_prid = ip->i_d.di_projid;
> -			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
> -			do_work = true;
> -		}
> +	if (XFS_IS_PQUOTA_ENFORCED(mp) && pdqp && xfs_dquot_lowsp(pdqp)) {
> +		eofb.eof_prid = pdqp->q_id;
> +		eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
> +		do_work = true;
>  	}
>  
>  	if (!do_work)
>  		return 0;
>  
> -	error = xfs_icache_free_eofblocks(ip->i_mount, &eofb);
> +	error = xfs_icache_free_eofblocks(mp, &eofb);
>  	if (error)
>  		return error;
>  
> -	return xfs_icache_free_cowblocks(ip->i_mount, &eofb);
> +	return xfs_icache_free_cowblocks(mp, &eofb);
> +}
> +
> +/* Run cow/eofblocks scans on the quotas attached to the inode. */
> +int
> +xfs_blockgc_free_quota(
> +	struct xfs_inode	*ip,
> +	unsigned int		eof_flags)
> +{
> +	return xfs_blockgc_free_dquots(xfs_inode_dquot(ip, XFS_DQTYPE_USER),
> +			xfs_inode_dquot(ip, XFS_DQTYPE_GROUP),
> +			xfs_inode_dquot(ip, XFS_DQTYPE_PROJ), eof_flags);
>  }
> diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
> index d64ea8f5c589..5f520de637f6 100644
> --- a/fs/xfs/xfs_icache.h
> +++ b/fs/xfs/xfs_icache.h
> @@ -54,6 +54,8 @@ long xfs_reclaim_inodes_nr(struct xfs_mount *mp, int nr_to_scan);
>  
>  void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
>  
> +int xfs_blockgc_free_dquots(struct xfs_dquot *udqp, struct xfs_dquot *gdqp,
> +		struct xfs_dquot *pdqp, unsigned int eof_flags);
>  int xfs_blockgc_free_quota(struct xfs_inode *ip, unsigned int eof_flags);
>  
>  void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index f62c1c5f210f..ee3cb916c5c9 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1102,13 +1102,21 @@ xfs_trans_alloc_icreate(
>  	struct xfs_trans	**tpp)
>  {
>  	struct xfs_trans	*tp;
> +	bool			retried = false;
>  	int			error;
>  
> +retry:
>  	error = xfs_trans_alloc(mp, resv, dblocks, 0, 0, &tp);
>  	if (error)
>  		return error;
>  
>  	error = xfs_trans_reserve_quota_icreate(tp, udqp, gdqp, pdqp, dblocks);
> +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> +		xfs_trans_cancel(tp);
> +		xfs_blockgc_free_dquots(udqp, gdqp, pdqp, 0);
> +		retried = true;
> +		goto retry;
> +	}
>  	if (error) {
>  		xfs_trans_cancel(tp);
>  		return error;
> 


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

* Re: [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown
  2021-02-01  2:06 ` [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown Darrick J. Wong
  2021-02-01 12:36   ` Christoph Hellwig
@ 2021-02-02 15:39   ` Brian Foster
  1 sibling, 0 replies; 26+ messages in thread
From: Brian Foster @ 2021-02-02 15:39 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, hch, david

On Sun, Jan 31, 2021 at 06:06:17PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If a file user, group, or project change is unable to reserve enough
> quota to handle the modification, try clearing whatever space the
> filesystem might have been hanging onto in the hopes of speeding up the
> filesystem.  The flushing behavior will become particularly important
> when we add deferred inode inactivation because that will increase the
> amount of space that isn't actively tied to user data.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_trans.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index ee3cb916c5c9..3203841ab19b 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1149,8 +1149,10 @@ xfs_trans_alloc_ichange(
>  	struct xfs_dquot	*new_udqp;
>  	struct xfs_dquot	*new_gdqp;
>  	struct xfs_dquot	*new_pdqp;
> +	bool			retried = false;
>  	int			error;
>  
> +retry:
>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp);
>  	if (error)
>  		return error;
> @@ -1175,6 +1177,13 @@ xfs_trans_alloc_ichange(
>  	if (new_udqp || new_gdqp || new_pdqp) {
>  		error = xfs_trans_reserve_quota_chown(tp, ip, new_udqp,
>  				new_gdqp, new_pdqp, force);
> +		if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> +			xfs_trans_cancel(tp);
> +			xfs_blockgc_free_dquots(new_udqp, new_gdqp, new_pdqp,
> +					0);
> +			retried = true;
> +			goto retry;
> +		}
>  		if (error)
>  			goto out_cancel;
>  	}
> 


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

* Re: [PATCH 12/12] xfs: flush speculative space allocations when we run out of space
  2021-02-01  2:06 ` [PATCH 12/12] xfs: flush speculative space allocations when we run out of space Darrick J. Wong
@ 2021-02-02 15:39   ` Brian Foster
  0 siblings, 0 replies; 26+ messages in thread
From: Brian Foster @ 2021-02-02 15:39 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs, hch, david

On Sun, Jan 31, 2021 at 06:06:34PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If a fs modification (creation, file write, reflink, etc.) is unable to
> reserve enough space to handle the modification, try clearing whatever
> space the filesystem might have been hanging onto in the hopes of
> speeding up the filesystem.  The flushing behavior will become
> particularly important when we add deferred inode inactivation because
> that will increase the amount of space that isn't actively tied to user
> data.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_trans.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 3203841ab19b..973354647298 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -289,6 +289,17 @@ xfs_trans_alloc(
>  	tp->t_firstblock = NULLFSBLOCK;
>  
>  	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
> +	if (error == -ENOSPC) {
> +		/*
> +		 * We weren't able to reserve enough space for the transaction.
> +		 * Flush the other speculative space allocations to free space.
> +		 * Do not perform a synchronous scan because callers can hold
> +		 * other locks.
> +		 */
> +		error = xfs_blockgc_free_space(mp, NULL);
> +		if (!error)
> +			error = xfs_trans_reserve(tp, resp, blocks, rtextents);
> +	}
>  	if (error) {
>  		xfs_trans_cancel(tp);
>  		return error;
> 


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

* Re: [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks
  2021-02-02 15:38   ` Brian Foster
@ 2021-02-02 16:53     ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-02-02 16:53 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs, hch, david

On Tue, Feb 02, 2021 at 10:38:59AM -0500, Brian Foster wrote:
> On Sun, Jan 31, 2021 at 06:06:06PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > If a fs modification (data write, reflink, xattr set, fallocate, etc.)
> > is unable to reserve enough quota to handle the modification, try
> > clearing whatever space the filesystem might have been hanging onto in
> > the hopes of speeding up the filesystem.  The flushing behavior will
> > become particularly important when we add deferred inode inactivation
> > because that will increase the amount of space that isn't actively tied
> > to user data.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> (FWIW, I'm reviewing the patches from your reclaim-space-harder-5.12
> branch as of this morning, which look like they have some deltas from
> the posted versions based on Christoph's feedback.)

Yes, it does. :(

I pushed the branches and had sent the first of the two patchsets
last night, and then the power went out so I gave up and went to bed.

But thanks for the review. :)

--D

> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> >  fs/xfs/xfs_reflink.c |    5 +++++
> >  fs/xfs/xfs_trans.c   |   10 ++++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > 
> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index 086866f6e71f..725c7d8e4438 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -1092,6 +1092,11 @@ xfs_reflink_remap_extent(
> >  	 * count.  This is suboptimal, but the VFS flushed the dest range
> >  	 * before we started.  That should have removed all the delalloc
> >  	 * reservations, but we code defensively.
> > +	 *
> > +	 * xfs_trans_alloc_inode above already tried to grab an even larger
> > +	 * quota reservation, and kicked off a blockgc scan if it couldn't.
> > +	 * If we can't get a potentially smaller quota reservation now, we're
> > +	 * done.
> >  	 */
> >  	if (!quota_reserved && !smap_real && dmap_written) {
> >  		error = xfs_trans_reserve_quota_nblks(tp, ip,
> > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> > index 466e1c86767f..f62c1c5f210f 100644
> > --- a/fs/xfs/xfs_trans.c
> > +++ b/fs/xfs/xfs_trans.c
> > @@ -23,6 +23,7 @@
> >  #include "xfs_inode.h"
> >  #include "xfs_dquot_item.h"
> >  #include "xfs_dquot.h"
> > +#include "xfs_icache.h"
> >  
> >  kmem_zone_t	*xfs_trans_zone;
> >  
> > @@ -1046,8 +1047,10 @@ xfs_trans_alloc_inode(
> >  {
> >  	struct xfs_trans	*tp;
> >  	struct xfs_mount	*mp = ip->i_mount;
> > +	bool			retried = false;
> >  	int			error;
> >  
> > +retry:
> >  	error = xfs_trans_alloc(mp, resv, dblocks,
> >  			rblocks / mp->m_sb.sb_rextsize,
> >  			force ? XFS_TRANS_RESERVE : 0, &tp);
> > @@ -1065,6 +1068,13 @@ xfs_trans_alloc_inode(
> >  	}
> >  
> >  	error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks, force);
> > +	if (!retried && (error == -EDQUOT || error == -ENOSPC)) {
> > +		xfs_trans_cancel(tp);
> > +		xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > +		xfs_blockgc_free_quota(ip, 0);
> > +		retried = true;
> > +		goto retry;
> > +	}
> >  	if (error)
> >  		goto out_cancel;
> >  
> > 
> 

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

* [PATCH 04/12] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts
  2021-01-29  2:17 [PATCHSET v6 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
@ 2021-01-29  2:18 ` Darrick J. Wong
  0 siblings, 0 replies; 26+ messages in thread
From: Darrick J. Wong @ 2021-01-29  2:18 UTC (permalink / raw)
  To: djwong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs, hch, david, bfoster

From: Darrick J. Wong <djwong@kernel.org>

Move this function further down in the file so that later cleanups won't
have to declare static functions.  Change the name because we're about
to rework all the code that performs garbage collection of speculatively
allocated file blocks.  No functional changes.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c   |    2 -
 fs/xfs/xfs_icache.c |  110 ++++++++++++++++++++++++++-------------------------
 fs/xfs/xfs_icache.h |    2 -
 3 files changed, 57 insertions(+), 57 deletions(-)


diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 69879237533b..d69e5abcc1b4 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -747,7 +747,7 @@ xfs_file_buffered_write(
 	 */
 	if (ret == -EDQUOT && !cleared_space) {
 		xfs_iunlock(ip, iolock);
-		cleared_space = xfs_inode_free_quota_blocks(ip);
+		cleared_space = xfs_blockgc_free_quota(ip);
 		if (cleared_space)
 			goto write_retry;
 		iolock = 0;
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 10c1a0dee17d..aba901d5637b 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1396,61 +1396,6 @@ xfs_icache_free_eofblocks(
 			XFS_ICI_EOFBLOCKS_TAG);
 }
 
-/*
- * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
- * with multiple quotas, we don't know exactly which quota caused an allocation
- * failure. We make a best effort by including each quota under low free space
- * conditions (less than 1% free space) in the scan.
- */
-bool
-xfs_inode_free_quota_blocks(
-	struct xfs_inode	*ip)
-{
-	struct xfs_eofblocks	eofb = {0};
-	struct xfs_dquot	*dq;
-	bool			do_work = false;
-
-	/*
-	 * Run a sync scan to increase effectiveness and use the union filter to
-	 * cover all applicable quotas in a single scan.
-	 */
-	eofb.eof_flags = XFS_EOF_FLAGS_UNION | XFS_EOF_FLAGS_SYNC;
-
-	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_uid = VFS_I(ip)->i_uid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
-			do_work = true;
-		}
-	}
-
-	if (XFS_IS_GQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_GROUP);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_gid = VFS_I(ip)->i_gid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
-			do_work = true;
-		}
-	}
-
-	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
-		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
-		if (dq && xfs_dquot_lowsp(dq)) {
-			eofb.eof_prid = ip->i_d.di_projid;
-			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
-			do_work = true;
-		}
-	}
-
-	if (!do_work)
-		return false;
-
-	xfs_icache_free_eofblocks(ip->i_mount, &eofb);
-	xfs_icache_free_cowblocks(ip->i_mount, &eofb);
-	return true;
-}
-
 static inline unsigned long
 xfs_iflag_for_tag(
 	int		tag)
@@ -1699,3 +1644,58 @@ xfs_start_block_reaping(
 	xfs_queue_eofblocks(mp);
 	xfs_queue_cowblocks(mp);
 }
+
+/*
+ * Run cow/eofblocks scans on the quotas applicable to the inode. For inodes
+ * with multiple quotas, we don't know exactly which quota caused an allocation
+ * failure. We make a best effort by including each quota under low free space
+ * conditions (less than 1% free space) in the scan.
+ */
+bool
+xfs_blockgc_free_quota(
+	struct xfs_inode	*ip)
+{
+	struct xfs_eofblocks	eofb = {0};
+	struct xfs_dquot	*dq;
+	bool			do_work = false;
+
+	/*
+	 * Run a sync scan to increase effectiveness and use the union filter to
+	 * cover all applicable quotas in a single scan.
+	 */
+	eofb.eof_flags = XFS_EOF_FLAGS_UNION | XFS_EOF_FLAGS_SYNC;
+
+	if (XFS_IS_UQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_USER);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_uid = VFS_I(ip)->i_uid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_UID;
+			do_work = true;
+		}
+	}
+
+	if (XFS_IS_GQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_GROUP);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_gid = VFS_I(ip)->i_gid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_GID;
+			do_work = true;
+		}
+	}
+
+	if (XFS_IS_PQUOTA_ENFORCED(ip->i_mount)) {
+		dq = xfs_inode_dquot(ip, XFS_DQTYPE_PROJ);
+		if (dq && xfs_dquot_lowsp(dq)) {
+			eofb.eof_prid = ip->i_d.di_projid;
+			eofb.eof_flags |= XFS_EOF_FLAGS_PRID;
+			do_work = true;
+		}
+	}
+
+	if (!do_work)
+		return false;
+
+	xfs_icache_free_eofblocks(ip->i_mount, &eofb);
+	xfs_icache_free_cowblocks(ip->i_mount, &eofb);
+	return true;
+}
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 3f7ddbca8638..21b726a05b0d 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -54,7 +54,7 @@ long xfs_reclaim_inodes_nr(struct xfs_mount *mp, int nr_to_scan);
 
 void xfs_inode_set_reclaim_tag(struct xfs_inode *ip);
 
-bool xfs_inode_free_quota_blocks(struct xfs_inode *ip);
+bool xfs_blockgc_free_quota(struct xfs_inode *ip);
 
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
 void xfs_inode_clear_eofblocks_tag(struct xfs_inode *ip);


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

end of thread, other threads:[~2021-02-02 16:56 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01  2:05 [PATCHSET v7 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
2021-02-01  2:05 ` [PATCH 01/12] xfs: trigger all block gc scans when low on quota space Darrick J. Wong
2021-02-01  2:05 ` [PATCH 02/12] xfs: don't stall cowblocks scan if we can't take locks Darrick J. Wong
2021-02-01  2:05 ` [PATCH 03/12] xfs: xfs_inode_free_quota_blocks should scan project quota Darrick J. Wong
2021-02-01  2:05 ` [PATCH 04/12] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts Darrick J. Wong
2021-02-01  2:05 ` [PATCH 05/12] xfs: pass flags and return gc errors from xfs_blockgc_free_quota Darrick J. Wong
2021-02-01  2:06 ` [PATCH 06/12] xfs: try worst case space reservation upfront in xfs_reflink_remap_extent Darrick J. Wong
2021-02-01 12:24   ` Christoph Hellwig
2021-02-01  2:06 ` [PATCH 07/12] xfs: flush eof/cowblocks if we can't reserve quota for file blocks Darrick J. Wong
2021-02-01 12:32   ` Christoph Hellwig
2021-02-01 19:01     ` Darrick J. Wong
2021-02-02 15:38   ` Brian Foster
2021-02-02 16:53     ` Darrick J. Wong
2021-02-01  2:06 ` [PATCH 08/12] xfs: flush eof/cowblocks if we can't reserve quota for inode creation Darrick J. Wong
2021-02-01 12:35   ` Christoph Hellwig
2021-02-01 19:03     ` Darrick J. Wong
2021-02-02 15:39   ` Brian Foster
2021-02-01  2:06 ` [PATCH 09/12] xfs: flush eof/cowblocks if we can't reserve quota for chown Darrick J. Wong
2021-02-01 12:36   ` Christoph Hellwig
2021-02-01 19:12     ` Darrick J. Wong
2021-02-02 15:39   ` Brian Foster
2021-02-01  2:06 ` [PATCH 10/12] xfs: add a tracepoint for blockgc scans Darrick J. Wong
2021-02-01  2:06 ` [PATCH 11/12] xfs: refactor xfs_icache_free_{eof,cow}blocks call sites Darrick J. Wong
2021-02-01  2:06 ` [PATCH 12/12] xfs: flush speculative space allocations when we run out of space Darrick J. Wong
2021-02-02 15:39   ` Brian Foster
  -- strict thread matches above, loose matches on Subject: below --
2021-01-29  2:17 [PATCHSET v6 00/12] xfs: try harder to reclaim space when we run out Darrick J. Wong
2021-01-29  2:18 ` [PATCH 04/12] xfs: move and rename xfs_inode_free_quota_blocks to avoid conflicts Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.