speculative preallocation inode tracking

Add the XFS_ICI_EOFBLOCKS_TAG inode tag to identify inodes with speculatively preallocated blocks beyond EOF. An inode is tagged when speculative preallocation occurs and untagged either via truncate down or when post-EOF blocks are freed via release or reclaim.

The tag management is intentionally not aggressive to prefer simplicity over the complexity of handling all the corner cases under which post-EOF blocks could be freed (i.e., forward truncation, fallocate, write error conditions, etc.). This means that a tagged inode may or may not have post-EOF blocks after a period of time. The tag is eventually cleared when the inode is released or reclaimed.

Signed-off-by Brian Foster <[email protected]>

Quoted 14 lines · Brian Foster

Add the XFS_ICI_EOFBLOCKS_TAG inode tag to identify inodes with speculatively preallocated blocks beyond EOF. An inode is tagged when speculative preallocation occurs and untagged either via truncate down or when post-EOF blocks are freed via release or reclaim.

The tag management is intentionally not aggressive to prefer simplicity over the complexity of handling all the corner cases under which post-EOF blocks could be freed (i.e., forward truncation, fallocate, write error conditions, etc.). This means that a tagged inode may or may not have post-EOF blocks after a period of time. The tag is eventually cleared when the inode is released or reclaimed.

Quoted 1 line

Signed-off-by Brian Foster <[email protected]>

Apart from the fact this conflicts with my xfssyncd killing patchset and xfs_sync.c no longer exists, it looks fine.

Can you rebase this on top of my patch series? Mostly it is simply making your changes to xfs_icache.c rather than xfs_sync.c as that's where all this inode cache radix tree walking code is now.....

Cheers,

Dave.

Genericize xfs_inode_ag_walk() to support an optional radix tree tag and args argument for the execute function. Create a new wrapper called xfs_inode_ag_iterator_tag() that performs a tag based walk of perag's and inodes.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_qm_syscalls.c |    5 ++-
 fs/xfs/xfs_sync.c        |   61 +++++++++++++++++++++++++++++++++++++++-------
 fs/xfs/xfs_sync.h        |    7 ++++-
 3 files changed, 60 insertions(+), 13 deletions(-)

This check is used in multiple places to determine whether we should check for (and potentially free) post EOF blocks on an inode. Add a helper to consolidate the check.

Note that when we remove an inode from the cache (xfs_inactive()), we are required to trim post-EOF blocks even if the inode is marked preallocated or append-only to maintain correct space accounting. The 'force' parameter to xfs_can_free_eofblocks() specifies whether we should ignore the prealloc/append-only status of the inode.

Signed-off-by Brian Foster <[email protected]>

fs/xfs/xfs_vnodeops.h +0 -0

index 447e146..d5701e3 100644

fs/xfs/xfs_vnodeops.h +25 -1

@@ -9,8 +13,42 @@ struct iovec;
 struct kiocb;
 struct pipe_inode_info;
 struct uio;
-struct xfs_inode;
 
+/*
+ * Test whether it is appropriate to check an inode for and free post EOF
+ * blocks. The 'force' parameter determines whether we should also consider
+ * regular files that are marked preallocated or append-only.
+ */
+static inline bool
+xfs_can_free_eofblocks(struct xfs_inode *ip, bool force)
+{
+	/* prealloc/delalloc exists only on regular files */
+	if (!S_ISREG(ip->i_d.di_mode))
+		return false;
+
+	/*
+	 * Zero sized files with no cached pages and delalloc blocks will not
+	 * have speculative prealloc/delalloc blocks to remove.
+	 */
+	if (VFS_I(ip)->i_size == 0 &&
+	    VN_CACHED(VFS_I(ip)) == 0 &&
+	    ip->i_delayed_blks == 0)
+		return false;
+
+	/* If we haven't read in the extent list, then don't do it now. */
+	if (!(ip->i_df.if_flags & XFS_IFEXTENTS))
+		return false;
+

Quoted 11 lines · Brian Foster

This check is used in multiple places to determine whether we should check for (and potentially free) post EOF blocks on an inode. Add a helper to consolidate the check.

Note that when we remove an inode from the cache (xfs_inactive()), we are required to trim post-EOF blocks even if the inode is marked preallocated or append-only to maintain correct space accounting. The 'force' parameter to xfs_can_free_eofblocks() specifies whether we should ignore the prealloc/append-only status of the inode.

Signed-off-by Brian Foster <[email protected]>

....

Quoted 8 lines

fs/xfs/xfs_vnodeops.h +0 -0

index 447e146..d5701e3 100644
@@ -1,6 +1,10 @@
 #ifndef _XFS_VNODEOPS_H
 #define _XFS_VNODEOPS_H 1
 

fs/xfs/xfs_vnodeops.h line 10

Quoted 7 lines

fs/xfs/xfs_vnodeops.h +4 -0

+#include "xfs_bmap_btree.h"
+#include "xfs_dinode.h"
+#include "xfs_inode.h"
+
 struct attrlist_cursor_kern;
 struct file;
 struct iattr;

Generally we try to avoid including XFS header files in XFS header files, and instead order the header files in the .c files appropriately.

Quoted 38 lines

fs/xfs/xfs_vnodeops.h +32 -1

@@ -9,8 +13,42 @@ struct iovec;
 struct kiocb;
 struct pipe_inode_info;
 struct uio;
-struct xfs_inode;
 
+/*
+ * Test whether it is appropriate to check an inode for and free post EOF
+ * blocks. The 'force' parameter determines whether we should also consider
+ * regular files that are marked preallocated or append-only.
+ */
+static inline bool
+xfs_can_free_eofblocks(struct xfs_inode *ip, bool force)
+{
+	/* prealloc/delalloc exists only on regular files */
+	if (!S_ISREG(ip->i_d.di_mode))
+		return false;
+
+	/*
+	 * Zero sized files with no cached pages and delalloc blocks will not
+	 * have speculative prealloc/delalloc blocks to remove.
+	 */
+	if (VFS_I(ip)->i_size == 0 &&
+	    VN_CACHED(VFS_I(ip)) == 0 &&
+	    ip->i_delayed_blks == 0)
+		return false;
+
+	/* If we haven't read in the extent list, then don't do it now. */
+	if (!(ip->i_df.if_flags & XFS_IFEXTENTS))
+		return false;
+
+	/*
+	 * Do not free real preallocated or append-only files unless the file
+	 * has delalloc blocks and we are forced to remove them.
+	 */
+	if (ip->i_d.di_flags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND))
+		if (!force || ip->i_delayed_blks == 0)
+			return false;

fs/xfs/xfs_vnodeops.h struct iovec; · line 51

Quoted 3 lines

fs/xfs/xfs_vnodeops.h +3 -0

+
+	return true;
+}

And I'd say that function is too large for a static inline function in a header file, so moving it to xfs_inode.c is probably appropriate. I know, I asked you to write it like this, but not looking at it for a couple weeks brings new persepctive ;)

Cheers,

Dave.

Turn xfs_free_eofblocks() into a non-static function, return EAGAIN to indicate trylock failure and make sure this error is not propagated in xfs_release().

xfs_inodes_free_eofblocks() implements scanning functionality for EOFBLOCKS inodes. It uses the AG iterator to walk the tagged inodes and free post-EOF blocks via the xfs_inode_free_eofblocks() execute function. The scan can be invoked in best-effort mode or wait (force) mode.

A best-effort scan (default) handles all inodes that do not have a dirty cache and we successfully acquire the io lock via trylock. In wait mode, we continue to cycle through an AG until all inodes are handled.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_sync.c  |   40 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sync.h  |    1 +
 fs/xfs/xfs_trace.h |    1 +
 3 files changed, 42 insertions(+), 0 deletions(-)

fs/xfs/xfs_sync.c +10 -0

index 0da93c9..6854800 100644
@@ -1014,6 +1014,46 @@ xfs_reclaim_inodes_count(
 	return reclaimable;
 }
 
+STATIC int
+xfs_inode_free_eofblocks(
+	struct xfs_inode	*ip,
+	struct xfs_perag	*pag,
+	int			flags,
+	void			*args)
+{
+	int ret;
+	bool force = flags & SYNC_WAIT;
+

Quoted 6 lines · Brian Foster

xfs_inodes_free_eofblocks() implements scanning functionality for EOFBLOCKS inodes. It uses the AG iterator to walk the tagged inodes and free post-EOF blocks via the xfs_inode_free_eofblocks() execute function. The scan can be invoked in best-effort mode or wait (force) mode.

Quoted 6 lines

A best-effort scan (default) handles all inodes that do not have a dirty cache and we successfully acquire the io lock via trylock. In wait mode, we continue to cycle through an AG until all inodes are handled.

Signed-off-by Brian Foster <[email protected]>

xfs_icache.c rebase, and...

Quoted 24 lines

---
 fs/xfs/xfs_sync.c  |   40 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_sync.h  |    1 +
 fs/xfs/xfs_trace.h |    1 +
 3 files changed, 42 insertions(+), 0 deletions(-)

fs/xfs/xfs_sync.c +10 -0

index 0da93c9..6854800 100644
@@ -1014,6 +1014,46 @@ xfs_reclaim_inodes_count(
 	return reclaimable;
 }
 
+STATIC int
+xfs_inode_free_eofblocks(
+	struct xfs_inode	*ip,
+	struct xfs_perag	*pag,
+	int			flags,
+	void			*args)
+{
+	int ret;
+	bool force = flags & SYNC_WAIT;
+

fs/xfs/xfs_sync.c xfs_reclaim_inodes_count( · line 1036

Quoted 10 lines

fs/xfs/xfs_sync.c +10 -0

+	if (!xfs_can_free_eofblocks(ip, false)) {
+		/* inode could be preallocated or append-only */
+		trace_xfs_inode_free_eofblocks_invalid(ip);
+		xfs_inode_clear_eofblocks_tag(ip);
+		return 0;
+	}
+
+	if (!force && mapping_tagged(VFS_I(ip)->i_mapping,
+				     PAGECACHE_TAG_DIRTY))
+		return 0;

This reads rather strangely. I'd prefer that you don't use a "force" variable because we're not really "forcing" anything. SYNC_WAIT is telling us if we should block (wait) or not. i.e.

	/*
	 * if the mapping is dirty the operation can block and wait
	 * for some time. So unless we are waiting, skip it.
	 */
	if (!(flags & SYNC_WAIT) &&
	    (mapping_tagged(VFS_I(ip)->i_mapping, PAGECACHE_TAG_DIRTY)) 
		return 0;

makes more sense and is consistent with xfs_reclaim_inode() usage.

fs/xfs/xfs_sync.c xfs_reclaim_inodes_count( · line 1042

Quoted 5 lines

fs/xfs/xfs_sync.c +5 -0

+	ret = xfs_free_eofblocks(ip->i_mount, ip, true);
+
+	/* ignore EAGAIN on a best effort scan */
+	if (!force && (ret == EAGAIN))
+		ret = 0;

	/* don't revisit the inode if we not waiting */
	if (ret == EAGAIN && !(flags & SYNC_WAIT))
		return 0;
	return ret;

Quoted 9 lines

fs/xfs/xfs_sync.c +9 -0

+
+	return ret;
+}
+
+int
+xfs_inodes_free_eofblocks(
+	struct xfs_mount	*mp,
+	int			flags)
+{

fs/xfs/xfs_sync.c xfs_reclaim_inodes_count( · line 1055

Quoted 4 lines

fs/xfs/xfs_sync.c +4 -0

+	ASSERT((flags & ~(SYNC_TRYLOCK|SYNC_WAIT)) == 0);
+	return xfs_inode_ag_iterator_tag(mp, xfs_inode_free_eofblocks, flags,
+					 NULL, XFS_ICI_EOFBLOCKS_TAG);
+}

TWo functions very similarly named. Perhaps the latter would be better named xfs_icache_free_eofblocks() to indicate it is an inode cache operation, rather than an inode operation.

Then at some point in another patch set we can rename xfs_reclaim_inodes* to xfs_icache_reclaim_* and xfs_inode_ag_iterator* to xfs_icache_iterator* and so one so that there is a clear naming difference between operations on the inode cache and individual inodes...

Cheers,

Dave.

The XFS_IOC_FREE_EOFBLOCKS ioctl allows users to invoke an EOFBLOCKS scan. The xfs_eofblocks structure is defined to support the command parameters (scan mode).

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_fs.h    |   14 ++++++++++++++
 fs/xfs/xfs_ioctl.c |   12 ++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

fs/xfs/xfs_fs.h +0 -0

index c13fed8..32bb2e8 100644

fs/xfs/xfs_fs.h +4 -0

+
+
+/*
  * The user-level Handle Request interface structure.
  */
 typedef struct xfs_fsop_handlereq {

@@ -456,6 +469,7 @@ typedef struct xfs_handle {
 /*	XFS_IOC_GETBIOSIZE ---- deprecated 47	   */
 #define XFS_IOC_GETBMAPX	_IOWR('X', 56, struct getbmap)
 #define XFS_IOC_ZERO_RANGE	_IOW ('X', 57, struct xfs_flock64)
+#define XFS_IOC_FREE_EOFBLOCKS	_IOR ('X', 58, struct xfs_eofblocks)
 
 /*
  * ioctl commands that replace IRIX syssgi()'s

fs/xfs/xfs_ioctl.c +0 -0

index 0e0232c..216ca7a 100644

Support EOFBLOCKS scan filtering by quota ID or minimum file size. Add the appropriate fields/flags to the xfs_eofblocks structure and pass it down to xfs_inode_free_eofblocks() where filtering functionality is implemented.

A (user requested) quota ID based scan requires the associated quota mode be enabled.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_fs.h    |    9 +++++++++
 fs/xfs/xfs_ioctl.c |   10 +++++++++-
 fs/xfs/xfs_sync.c  |   30 ++++++++++++++++++++++++++----
 fs/xfs/xfs_sync.h  |    2 +-
 4 files changed, 45 insertions(+), 6 deletions(-)

fs/xfs/xfs_fs.h +0 -0

index 32bb2e8..54c0f39 100644

fs/xfs/xfs_fs.h +4 -0

+#define XFS_EOF_FLAGS_MINFILESIZE	0x10	/* minimum file size */
+
+#define XFS_EOF_VALID_QUOTA	(XFS_EOF_FLAGS_USRID|XFS_EOF_FLAGS_GRPID| \
+				 XFS_EOF_FLAGS_PROJID)
 
 
 /*

fs/xfs/xfs_ioctl.c +1 -0

index 216ca7a..a7bf847 100644
@@ -1609,8 +1609,16 @@ xfs_file_ioctl(
 		if (copy_from_user(&eofb, arg, sizeof(eofb)))
 			return -XFS_ERROR(EFAULT);
 
+		if (((eofb.eof_flags & XFS_EOF_FLAGS_USRID) &&

fs/xfs/xfs_ioctl.c +0 -0

 		return -error;
 	}
 

fs/xfs/xfs_sync.c +16 -0

index 6854800..c9e1c16 100644
@@ -1015,6 +1015,21 @@ xfs_reclaim_inodes_count(
 }
 
 STATIC int
+xfs_inode_match_quota_id(
+	struct xfs_inode	*ip,
+	struct xfs_eofblocks	*eofb)
+{
+	if (eofb->eof_flags & XFS_EOF_FLAGS_USRID)
+		return ip->i_d.di_uid == eofb->eof_id;
+	else if (eofb->eof_flags & XFS_EOF_FLAGS_GRPID)
+		return ip->i_d.di_gid == eofb->eof_id;
+	else if (eofb->eof_flags & XFS_EOF_FLAGS_PROJID)
+		return xfs_get_projid(ip) == eofb->eof_id;
+
+	return 0;
+}
+
+STATIC int
 xfs_inode_free_eofblocks(
 	struct xfs_inode	*ip,
 	struct xfs_perag	*pag,

@@ -1022,6 +1037,7 @@ xfs_inode_free_eofblocks(
 	void			*args)
 {
 	int ret;
+	struct xfs_eofblocks *eofb = args;
 	bool force = flags & SYNC_WAIT;
 
 	if (!xfs_can_free_eofblocks(ip, false)) {

@@ -1031,8 +1047,13 @@ xfs_inode_free_eofblocks(
 		return 0;
 	}
 

Quoted 26 lines · Brian Foster

Support EOFBLOCKS scan filtering by quota ID or minimum file size. Add the appropriate fields/flags to the xfs_eofblocks structure and pass it down to xfs_inode_free_eofblocks() where filtering functionality is implemented.

A (user requested) quota ID based scan requires the associated quota mode be enabled.

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_fs.h    |    9 +++++++++
 fs/xfs/xfs_ioctl.c |   10 +++++++++-
 fs/xfs/xfs_sync.c  |   30 ++++++++++++++++++++++++++----
 fs/xfs/xfs_sync.h  |    2 +-
 4 files changed, 45 insertions(+), 6 deletions(-)

fs/xfs/xfs_fs.h +2 -0

index 32bb2e8..54c0f39 100644
@@ -343,12 +343,21 @@ typedef struct xfs_error_injection {
  */
 struct xfs_eofblocks {
 	__u32		eof_flags;
+	__u32		eof_id;
+	__u64		eof_min_file_size;

fs/xfs/xfs_fs.h typedef struct xfs_error_injection { · line 350

Quoted 3 lines

fs/xfs/xfs_fs.h +0 -0

 	__s32		version;
 	unsigned char	pad[12];
 };

ACtually, looking at this the version needs to be the first field of the structure, so we can guarantee that it can be read by any kernel that supports the ioctl regardless of how the rest of the structure changes.

fs/xfs/xfs_fs.h typedef struct xfs_error_injection { · line 356

Quoted 6 lines

fs/xfs/xfs_fs.h +3 -0

 /* eof_flags values */
 #define XFS_EOF_FLAGS_FORCE		0x01	/* force/wait mode scan */
+#define XFS_EOF_FLAGS_USRID		0x02	/* filter by user id */
+#define XFS_EOF_FLAGS_GRPID		0x04	/* filter by group id */
+#define XFS_EOF_FLAGS_PROJID		0x08	/* filter by project id */

I'm wondering if it would be better to pass real quota fields (as per dqblk_xfs.h) than make up a new way to pass the same information). That way we might be able to use standard quota functions rather for checks and comparisons rather than duplicating them. That way if we ever add new quota types, we don't have to add flags and validation to this ioctl....

i.e. we have XFS_EOF_FLAGS_QUOTA to say "filter by quota fields", similar to the XFS_EOF_FLAGS_MINFILESIZE flag...

And it becomes much easier to convert to userns kqids that are not that far away:

https://lkml.org/lkml/2012/9/19/587

Quoted 15 lines

fs/xfs/xfs_fs.h +4 -0

+#define XFS_EOF_FLAGS_MINFILESIZE	0x10	/* minimum file size */
+
+#define XFS_EOF_VALID_QUOTA	(XFS_EOF_FLAGS_USRID|XFS_EOF_FLAGS_GRPID| \
+				 XFS_EOF_FLAGS_PROJID)
 
 
 /*

fs/xfs/xfs_ioctl.c +0 -0

index 216ca7a..a7bf847 100644
@@ -1609,8 +1609,16 @@ xfs_file_ioctl(
 		if (copy_from_user(&eofb, arg, sizeof(eofb)))
 			return -XFS_ERROR(EFAULT);
 

fs/xfs/xfs_ioctl.c xfs_file_ioctl( · line 1621

Quoted 11 lines

fs/xfs/xfs_ioctl.c +9 -1

+		if (((eofb.eof_flags & XFS_EOF_FLAGS_USRID) &&
+		     !XFS_IS_UQUOTA_ON(mp)) ||
+		    ((eofb.eof_flags & XFS_EOF_FLAGS_GRPID) &&
+		     !XFS_IS_GQUOTA_ON(mp)) ||
+		    ((eofb.eof_flags & XFS_EOF_FLAGS_PROJID) &&
+		     !XFS_IS_PQUOTA_ON(mp)))
+			return -XFS_ERROR(EINVAL);
+
 		flags = (eofb.eof_flags & XFS_EOF_FLAGS_FORCE) ? SYNC_WAIT : SYNC_TRYLOCK;
-		error = xfs_inodes_free_eofblocks(mp, flags);
+		error = xfs_inodes_free_eofblocks(mp, flags, &eofb);

You probably shoul djust pass the &eofb in the first patch, rather than having to change the implementation here again.

Quoted 41 lines

fs/xfs/xfs_ioctl.c +0 -0

 		return -error;
 	}
 

fs/xfs/xfs_sync.c +16 -0

index 6854800..c9e1c16 100644
@@ -1015,6 +1015,21 @@ xfs_reclaim_inodes_count(
 }
 
 STATIC int
+xfs_inode_match_quota_id(
+	struct xfs_inode	*ip,
+	struct xfs_eofblocks	*eofb)
+{
+	if (eofb->eof_flags & XFS_EOF_FLAGS_USRID)
+		return ip->i_d.di_uid == eofb->eof_id;
+	else if (eofb->eof_flags & XFS_EOF_FLAGS_GRPID)
+		return ip->i_d.di_gid == eofb->eof_id;
+	else if (eofb->eof_flags & XFS_EOF_FLAGS_PROJID)
+		return xfs_get_projid(ip) == eofb->eof_id;
+
+	return 0;
+}
+
+STATIC int
 xfs_inode_free_eofblocks(
 	struct xfs_inode	*ip,
 	struct xfs_perag	*pag,

@@ -1022,6 +1037,7 @@ xfs_inode_free_eofblocks(
 	void			*args)
 {
 	int ret;
+	struct xfs_eofblocks *eofb = args;
 	bool force = flags & SYNC_WAIT;
 
 	if (!xfs_can_free_eofblocks(ip, false)) {

@@ -1031,8 +1047,13 @@ xfs_inode_free_eofblocks(
 		return 0;
 	}
 

fs/xfs/xfs_sync.c xfs_inode_free_eofblocks( · line 1057

Quoted 10 lines

fs/xfs/xfs_sync.c +7 -2

-	if (!force && mapping_tagged(VFS_I(ip)->i_mapping,
-				     PAGECACHE_TAG_DIRTY))
+	if ((eofb &&
+	     (((eofb->eof_flags & XFS_EOF_VALID_QUOTA) &&
+	       !xfs_inode_match_quota_id(ip, eofb)) || 
+	      ((eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE) &&
+	       (XFS_ISIZE(ip) < eofb->eof_min_file_size)))) ||
+	    (!force && mapping_tagged(VFS_I(ip)->i_mapping,
+				      PAGECACHE_TAG_DIRTY)))
 		return 0;

Break that up into multiple "if() return 0" statements so it is possible to read the logic. ;)

Cheers,

Dave.

Create a delayed_work to enable background scanning and freeing of EOFBLOCKS inodes. The scanner kicks in once speculative preallocation occurs and stops requeueing itself when no EOFBLOCKS inodes exist.

Scans are queued on the existing syncd workqueue and the interval is based on the new eofb_timer tunable (default to 5m). The background scanner performs unfiltered, best effort scans (which skips inodes under lock contention or with a dirty cache mapping).

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_globals.c |    1 +
 fs/xfs/xfs_linux.h   |    1 +
 fs/xfs/xfs_mount.h   |    2 ++
 fs/xfs/xfs_sync.c    |   30 ++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysctl.c  |    9 +++++++++
 fs/xfs/xfs_sysctl.h  |    1 +
 6 files changed, 44 insertions(+), 0 deletions(-)

fs/xfs/xfs_globals.c +1 -0

index 76e81cf..fda9a66 100644
@@ -40,4 +40,5 @@ xfs_param_t xfs_params = {
 	.rotorstep	= {	1,		1,		255	},
 	.inherit_nodfrg	= {	0,		1,		1	},
 	.fstrm_timer	= {	1,		30*100,		3600*100},
+	.eofb_timer	= {	1*100,		300*100,	7200*100},
 };

fs/xfs/xfs_linux.h +0 -0

index 828662f..bbad99b 100644

fs/xfs/xfs_linux.h +0 -0

 #define current_cpu()		(raw_smp_processor_id())
 #define current_pid()		(current->pid)

fs/xfs/xfs_mount.h +2 -0

index deee09e..bf5ecfa 100644
@@ -199,6 +199,8 @@ typedef struct xfs_mount {
 	struct xfs_mru_cache	*m_filestream;  /* per-mount filestream data */
 	struct delayed_work	m_sync_work;	/* background sync work */
 	struct delayed_work	m_reclaim_work;	/* background inode reclaim */
+	struct delayed_work	m_eofblocks_work; /* background eof blocks
+						     trimming */
 	struct work_struct	m_flush_work;	/* background inode flush */
 	__int64_t		m_update_flags;	/* sb flags we need to update
 						   on the next remount,rw */

fs/xfs/xfs_sync.c +4 -0

index c9e1c16..31f678a 100644
@@ -532,6 +532,31 @@ xfs_flush_worker(
 	xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_WAIT);
 }
 
+/*
+ * Background scanning to trim post-EOF preallocated space. This is queued
+ * based on the 'eofb_centisecs' tunable (5m by default).
+ */

Quoted 35 lines · Brian Foster

Create a delayed_work to enable background scanning and freeing of EOFBLOCKS inodes. The scanner kicks in once speculative preallocation occurs and stops requeueing itself when no EOFBLOCKS inodes exist.

Scans are queued on the existing syncd workqueue and the interval is based on the new eofb_timer tunable (default to 5m). The background scanner performs unfiltered, best effort scans (which skips inodes under lock contention or with a dirty cache mapping).

Signed-off-by Brian Foster <[email protected]>

---
 fs/xfs/xfs_globals.c |    1 +
 fs/xfs/xfs_linux.h   |    1 +
 fs/xfs/xfs_mount.h   |    2 ++
 fs/xfs/xfs_sync.c    |   30 ++++++++++++++++++++++++++++++
 fs/xfs/xfs_sysctl.c  |    9 +++++++++
 fs/xfs/xfs_sysctl.h  |    1 +
 6 files changed, 44 insertions(+), 0 deletions(-)

fs/xfs/xfs_globals.c +1 -0

index 76e81cf..fda9a66 100644
@@ -40,4 +40,5 @@ xfs_param_t xfs_params = {
 	.rotorstep	= {	1,		1,		255	},
 	.inherit_nodfrg	= {	0,		1,		1	},
 	.fstrm_timer	= {	1,		30*100,		3600*100},
+	.eofb_timer	= {	1*100,		300*100,	7200*100},
 };

fs/xfs/xfs_linux.h +0 -0

index 828662f..bbad99b 100644

fs/xfs/xfs_linux.h line 121

Quoted 4 lines

fs/xfs/xfs_linux.h +1 -0

@@ -118,6 +118,7 @@
 #define xfs_rotorstep		xfs_params.rotorstep.val
 #define xfs_inherit_nodefrag	xfs_params.inherit_nodfrg.val
 #define xfs_fstrm_centisecs	xfs_params.fstrm_timer.val
+#define xfs_eofb_centisecs	xfs_params.eofb_timer.val

Let's not propagate that stupid "centiseconds" unit any further. Nobody uses it, and it was only introduced because jiffie was 10ms and there were 100 to a second so it was easy to convert in the code. I don't think there is any reason for needing sub-second granularity for this background function, so seconds shoul dbe just fine for it. If you think we nee dfiner granularity, milliseconds is the nex tunit to choose....

Quoted 28 lines

fs/xfs/xfs_linux.h +0 -0

 #define current_cpu()		(raw_smp_processor_id())
 #define current_pid()		(current->pid)

fs/xfs/xfs_mount.h +2 -0

index deee09e..bf5ecfa 100644
@@ -199,6 +199,8 @@ typedef struct xfs_mount {
 	struct xfs_mru_cache	*m_filestream;  /* per-mount filestream data */
 	struct delayed_work	m_sync_work;	/* background sync work */
 	struct delayed_work	m_reclaim_work;	/* background inode reclaim */
+	struct delayed_work	m_eofblocks_work; /* background eof blocks
+						     trimming */
 	struct work_struct	m_flush_work;	/* background inode flush */
 	__int64_t		m_update_flags;	/* sb flags we need to update
 						   on the next remount,rw */

fs/xfs/xfs_sync.c +4 -0

index c9e1c16..31f678a 100644
@@ -532,6 +532,31 @@ xfs_flush_worker(
 	xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_WAIT);
 }
 
+/*
+ * Background scanning to trim post-EOF preallocated space. This is queued
+ * based on the 'eofb_centisecs' tunable (5m by default).
+ */

fs/xfs/xfs_sync.c xfs_flush_worker( · line 548

Quoted 10 lines

fs/xfs/xfs_sync.c +10 -0

+STATIC void
+xfs_queue_eofblocks(
+	struct xfs_mount *mp)
+{
+	rcu_read_lock();
+	if (radix_tree_tagged(&mp->m_perag_tree, XFS_ICI_EOFBLOCKS_TAG))
+		queue_delayed_work(xfs_syncd_wq, &mp->m_eofblocks_work,
+			msecs_to_jiffies(xfs_eofb_centisecs * 10));
+	rcu_read_unlock();
+}

This will all need reworking for the new xfs_icache.c and per-mount workqueue structuring. Fundamentally there is nothing wrong with what you've done, it's just been reworked...

fs/xfs/xfs_sync.c xfs_flush_worker( · line 550

Quoted 2 lines

fs/xfs/xfs_sync.c +2 -0

+	{
+		.procname	= "eofb_centisecs",

Ugh. Call it something users might understand. Say "background_prealloc_discard_period", or something similarly informative...

Cheers,

Dave.