xfs: fix freeing speculative preallocations for preallocated files

[ Upstream commit 610b29161b0aa9feb59b78dc867553274f17fb01 ]

xfs_can_free_eofblocks returns false for files that have persistent
preallocations unless the force flag is passed and there are delayed
blocks.  This means it won't free delalloc reservations for files
with persistent preallocations unless the force flag is set, and it
will also free the persistent preallocations if the force flag is
set and the file happens to have delayed allocations.

Both of these are bad, so do away with the force flag and always free
only post-EOF delayed allocations for files with the XFS_DIFLAG_PREALLOC
or APPEND flags set.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 399451aab26ab37ad750b74470646ca86909d8a6..62b92e92a685d2304d77c6463ba76c99db4dbb97 100644 (file)
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -636,13 +636,11 @@ out_unlock:
 
 /*
  * 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.
+ * blocks.
  */
 bool
 xfs_can_free_eofblocks(
-       struct xfs_inode        *ip,
-       bool                    force)
+       struct xfs_inode        *ip)
 {
        struct xfs_bmbt_irec    imap;
        struct xfs_mount        *mp = ip->i_mount;
@@ -676,11 +674,11 @@ xfs_can_free_eofblocks(
                return false;
 
        /*
-        * Do not free real preallocated or append-only files unless the file
-        * has delalloc blocks and we are forced to remove them.
+        * Only free real extents for inodes with persistent preallocations or
+        * the append-only flag.
         */
        if (ip->i_diflags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND))
-               if (!force || ip->i_delayed_blks == 0)
+               if (ip->i_delayed_blks == 0)
                        return false;
 
        /*
@@ -734,6 +732,22 @@ xfs_free_eofblocks(
        /* Wait on dio to ensure i_size has settled. */
        inode_dio_wait(VFS_I(ip));
 
+       /*
+        * For preallocated files only free delayed allocations.
+        *
+        * Note that this means we also leave speculative preallocations in
+        * place for preallocated files.
+        */
+       if (ip->i_diflags & (XFS_DIFLAG_PREALLOC | XFS_DIFLAG_APPEND)) {
+               if (ip->i_delayed_blks) {
+                       xfs_bmap_punch_delalloc_range(ip,
+                               round_up(XFS_ISIZE(ip), mp->m_sb.sb_blocksize),
+                               LLONG_MAX);
+               }
+               xfs_inode_clear_eofblocks_tag(ip);
+               return 0;
+       }
+
        error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
        if (error) {
                ASSERT(xfs_is_shutdown(mp));
@@ -1048,7 +1062,7 @@ xfs_prepare_shift(
         * Trim eofblocks to avoid shifting uninitialized post-eof preallocation
         * into the accessible region of the file.
         */
-       if (xfs_can_free_eofblocks(ip, true)) {
+       if (xfs_can_free_eofblocks(ip)) {
                error = xfs_free_eofblocks(ip);
                if (error)
                        return error;

diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index 6888078f5c31e009b8a0f1b91938d3a1381423ce..1383019ccdb755f6f055ea264fed416b27aa4cd5 100644 (file)
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -63,7 +63,7 @@ int   xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
                                xfs_off_t len);
 
 /* EOF block manipulation functions */
-bool   xfs_can_free_eofblocks(struct xfs_inode *ip, bool force);
+bool   xfs_can_free_eofblocks(struct xfs_inode *ip);
 int    xfs_free_eofblocks(struct xfs_inode *ip);
 
 int    xfs_swap_extents(struct xfs_inode *ip, struct xfs_inode *tip,

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 6df826fc787c6410a5364330200cb61bc5083e38..586d26c05160131ccd8449c59450469ef636443d 100644 (file)
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1186,7 +1186,7 @@ xfs_inode_free_eofblocks(
        }
        *lockflags |= XFS_IOLOCK_EXCL;
 
-       if (xfs_can_free_eofblocks(ip, false))
+       if (xfs_can_free_eofblocks(ip))
                return xfs_free_eofblocks(ip);
 
        /* inode could be preallocated or append-only */

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 26961b0dae03a1139076cb7d4fb43f5b5bbf88d7..b26d26d29273dc281d22bdf0a79975f75f94cb74 100644 (file)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1459,7 +1459,7 @@ xfs_release(
        if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
                return 0;
 
-       if (xfs_can_free_eofblocks(ip, false)) {
+       if (xfs_can_free_eofblocks(ip)) {
                /*
                 * Check if the inode is being opened, written and closed
                 * frequently and we have delayed allocation blocks outstanding
@@ -1675,15 +1675,13 @@ xfs_inode_needs_inactive(
 
        /*
         * This file isn't being freed, so check if there are post-eof blocks
-        * to free.  @force is true because we are evicting an inode from the
-        * cache.  Post-eof blocks must be freed, lest we end up with broken
-        * free space accounting.
+        * to free.
         *
         * Note: don't bother with iolock here since lockdep complains about
         * acquiring it in reclaim context. We have the only reference to the
         * inode at this point anyways.
         */
-       return xfs_can_free_eofblocks(ip, true);
+       return xfs_can_free_eofblocks(ip);
 }
 
 /*
@@ -1734,15 +1732,11 @@ xfs_inactive(
 
        if (VFS_I(ip)->i_nlink != 0) {
                /*
-                * force is true because we are evicting an inode from the
-                * cache. Post-eof blocks must be freed, lest we end up with
-                * broken free space accounting.
-                *
                 * Note: don't bother with iolock here since lockdep complains
                 * about acquiring it in reclaim context. We have the only
                 * reference to the inode at this point anyways.
                 */
-               if (xfs_can_free_eofblocks(ip, true))
+               if (xfs_can_free_eofblocks(ip))
                        error = xfs_free_eofblocks(ip);
 
                goto out;