xfs: don't truncate prealloc from frequently accessed inodes

A long standing problem for streaming writeѕ through the NFS server
has been that the NFS server opens and closes file descriptors on an
inode for every write. The result of this behaviour is that the
->release() function is called on every close and that results in
XFS truncating speculative preallocation beyond the EOF.  This has
an adverse effect on file layout when multiple files are being
written at the same time - they interleave their extents and can
result in severe fragmentation.

To avoid this problem, keep track of ->release calls made on a dirty
inode. For most cases, an inode is only going to be opened once for
writing and then closed again during it's lifetime in cache. Hence
if there are multiple ->release calls when the inode is dirty, there
is a good chance that the inode is being accessed by the NFS server.
Hence set a flag the first time ->release is called while there are
delalloc blocks still outstanding on the inode.

If this flag is set when ->release is next called, then do no
truncate away the speculative preallocation - leave it there so that
subsequent writes do not need to reallocate the delalloc space. This
will prevent interleaving of extents of different inodes written
concurrently to the same AG.

If we get this wrong, it is not a big deal as we truncate
speculative allocation beyond EOF anyway in xfs_inactive() when the
inode is thrown out of the cache.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 1c6514d73dc877572a421103f43997c10d2e1b09..5c95fa8ec11da9c703f756b4dbe42e81a569a6ac 100644 (file)
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -376,12 +376,13 @@ static inline void xfs_ifunlock(xfs_inode_t *ip)
 /*
  * In-core inode flags.
  */
-#define XFS_IRECLAIM    0x0001  /* we have started reclaiming this inode    */
-#define XFS_ISTALE     0x0002  /* inode has been staled */
-#define XFS_IRECLAIMABLE 0x0004 /* inode can be reclaimed */
-#define XFS_INEW       0x0008  /* inode has just been allocated */
-#define XFS_IFILESTREAM        0x0010  /* inode is in a filestream directory */
-#define XFS_ITRUNCATED 0x0020  /* truncated down so flush-on-close */
+#define XFS_IRECLAIM           0x0001  /* started reclaiming this inode */
+#define XFS_ISTALE             0x0002  /* inode has been staled */
+#define XFS_IRECLAIMABLE       0x0004  /* inode can be reclaimed */
+#define XFS_INEW               0x0008  /* inode has just been allocated */
+#define XFS_IFILESTREAM                0x0010  /* inode is in a filestream directory */
+#define XFS_ITRUNCATED         0x0020  /* truncated down so flush-on-close */
+#define XFS_IDIRTY_RELEASE     0x0040  /* dirty release already seen */
 
 /*
  * Flags for inode locking.

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 8e4a63c4151a48e05a11d9c791f7c5a9092bf197..d8e6f8cd6f0c89782dd0d5df7a2e7a0329688235 100644 (file)
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -964,29 +964,48 @@ xfs_release(
                        xfs_flush_pages(ip, 0, -1, XBF_ASYNC, FI_NONE);
        }
 
-       if (ip->i_d.di_nlink != 0) {
-               if ((((ip->i_d.di_mode & S_IFMT) == S_IFREG) &&
-                    ((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)))) {
+       if (ip->i_d.di_nlink == 0)
+               return 0;
 
-                       /*
-                        * If we can't get the iolock just skip truncating
-                        * the blocks past EOF because we could deadlock
-                        * with the mmap_sem otherwise.  We'll get another
-                        * chance to drop them once the last reference to
-                        * the inode is dropped, so we'll never leak blocks
-                        * permanently.
-                        */
-                       error = xfs_free_eofblocks(mp, ip,
-                                                  XFS_FREE_EOF_TRYLOCK);
-                       if (error)
-                               return error;
-               }
-       }
+       if ((((ip->i_d.di_mode & S_IFMT) == S_IFREG) &&
+            ((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)))) {
 
+               /*
+                * If we can't get the iolock just skip truncating the blocks
+                * past EOF because we could deadlock with the mmap_sem
+                * otherwise.  We'll get another chance to drop them once the
+                * last reference to the inode is dropped, so we'll never leak
+                * blocks permanently.
+                *
+                * Further, check if the inode is being opened, written and
+                * closed frequently and we have delayed allocation blocks
+                * oustanding (e.g. streaming writes from the NFS server),
+                * truncating the blocks past EOF will cause fragmentation to
+                * occur.
+                *
+                * In this case don't do the truncation, either, but we have to
+                * be careful how we detect this case. Blocks beyond EOF show
+                * up as i_delayed_blks even when the inode is clean, so we
+                * need to truncate them away first before checking for a dirty
+                * release. Hence on the first dirty close we will still remove
+                * the speculative allocation, but after that we will leave it
+                * in place.
+                */
+               if (xfs_iflags_test(ip, XFS_IDIRTY_RELEASE))
+                       return 0;
+
+               error = xfs_free_eofblocks(mp, ip,
+                                          XFS_FREE_EOF_TRYLOCK);
+               if (error)
+                       return error;
+
+               /* delalloc blocks after truncation means it really is dirty */
+               if (ip->i_delayed_blks)
+                       xfs_iflags_set(ip, XFS_IDIRTY_RELEASE);
+       }
        return 0;
 }