xfs
[Top] [All Lists]

[PATCH] xfs: xfs_release don't free eofblocks with extsize hint

To: xfs@xxxxxxxxxxx
Subject: [PATCH] xfs: xfs_release don't free eofblocks with extsize hint
From: Ben Myers <bpm@xxxxxxx>
Date: Mon, 11 Jun 2012 14:29:48 -0500
User-agent: Mutt/1.5.20 (2009-06-14)
Testing has shown that there is a regression when using extent size
hints with NFS.  There are many more extents created than expected.

This is the workload, from a single NFS client:
# for e in `seq 1 8`;
do
        mkdir -p $e; (cd $e; dd if=/dev/zero of=file bs=4096k count=100) &
done

# xfs_io -c extsize .
[16777216] .

Here is the extent count for each file without this patch:

385 387 387 387 400 398 379 334

Here is the extent count with commit aff3a9ed reverted (using delayed
allocation):

3 4 2 3 4 4 3 3

In a comparison of traces, one with the above workload run locally and
the other with the above workload over NFS, there were occasional calls
to xfs_free_eofblocks in NFS but not on local workloads.  These turned
out to be from xfs_release.

Not calling xfs_free_eofblocks when using the extent size hint resolves this
issue when using extsize hints and NFS.  I suspect that delayed allocation
mitigated the effect of xfs_free_eofblocks until commit aff3a9ed.

Here is the extent count with this patch:

6 3 9 12 3 3 8 10

Signed-off-by: Ben Myers <bpm@xxxxxxx>

Index: xfs/fs/xfs/xfs_vnodeops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_vnodeops.c
+++ xfs/fs/xfs/xfs_vnodeops.c
@@ -547,8 +547,9 @@ xfs_release(
        if ((S_ISREG(ip->i_d.di_mode) &&
             (VFS_I(ip)->i_size > 0 ||
              (VN_CACHED(VFS_I(ip)) > 0 || ip->i_delayed_blks > 0)) &&
-            (ip->i_df.if_flags & XFS_IFEXTENTS))  &&
-           (!(ip->i_d.di_flags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND)))) {
+            (ip->i_df.if_flags & XFS_IFEXTENTS)) &&
+           (!(ip->i_d.di_flags & (XFS_DIFLAG_PREALLOC|XFS_DIFLAG_APPEND))) &&
+           (!xfs_get_extsz_hint(ip))) {
 
                /*
                 * If we can't get the iolock just skip truncating the blocks
@@ -570,6 +571,11 @@ xfs_release(
                 * release. Hence on the first dirty close we will still remove
                 * the speculative allocation, but after that we will leave it
                 * in place.
+                *
+                * Additionally, do not free eofblocks for files with the
+                * extent size hint set.  This can cause excessive
+                * fragmentation which is inconsistent with the intent
+                * of the extsize feature.
                 */
                if (xfs_iflags_test(ip, XFS_IDIRTY_RELEASE))
                        return 0;

<Prev in Thread] Current Thread [Next in Thread>