[PATCH 0/2] hfs/hfsplus: release preallocated blocks on truncate

[PATCH 0/2] hfs/hfsplus: release preallocated blocks on truncate

[Viacheslav Dubeyko]

HFS and HFS+ extend a fork in whole clumps, and when an inode
is loaded the number of allocated blocks comes from the on-disk
physical size of the fork, while i_size and phys_size come from
the logical size. So the fork can hold more blocks than i_size
needs even when phys_size equals i_size.

hfs_file_truncate() and hfsplus_file_truncate() return early when
i_size equals phys_size, before comparing the required block
count with the allocated one. This logic has been added
to avoid dirtying an unchanged inode, but it also skips
releasing the extra blocks:

 - a file whose size is aligned on the block size keeps its clump
   preallocation after the last close;

 - the catalog delete path does not free the data fork and relies
   on truncating the inode to zero size, so a file with zero
   logical size but allocated blocks leaves those blocks marked
   as used in the bitmap after it has been deleted.

Make the early return happen only when neither the
size nor the allocation needs to be trimmed, which keeps the
original intent of not dirtying an unchanged inode.

Viacheslav Dubeyko (2):
  hfsplus: release preallocated blocks in hfsplus_file_truncate()
  hfs: release preallocated blocks in hfs_file_truncate()

 fs/hfs/extent.c      | 13 +++++++++++--
 fs/hfsplus/extents.c | 11 +++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH 1/2] hfsplus: release preallocated blocks in hfsplus_file_truncate()

[Viacheslav Dubeyko]

hfsplus_get_block() extends a fork through hfsplus_file_extend(),
which allocates whole clumps. Also, hfsplus_inode_read_fork()
initializes alloc_blocks from the on-disk total_blocks of the
fork, while i_size and phys_size are set to the logical size.
As a result, alloc_blocks can be ahead of the number of blocks
really needed to hold i_size, even when phys_size matches i_size.

hfsplus_file_truncate() returns early whenever i_size equals
phys_size, before it compares the required block count with
alloc_blocks. So, for example, a file whose size is aligned on
the block size keeps its clump preallocation after the last
close. Moreover, hfsplus_delete_cat() does not free the data
fork and relies on hfsplus_delete_inode() truncating the file
to zero size. If a file has zero logical size but allocated
blocks, the early return skips the truncation, and its blocks
remain marked as used in the allocation file after the file
has been deleted.

Compute the required block count first and return early only
when neither the size nor the allocation needs to be trimmed.
This keeps the intent of the original early return of not
dirtying an unchanged inode. The existing logic further down
already handles the case of blk_cnt being equal to alloc_blocks.

Fixes: f76d28d235cf ("[PATCH] hfs: don't dirty unchanged inode")
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
---
 fs/hfsplus/extents.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index eb7c11524d18..924941440066 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -570,12 +570,19 @@ void hfsplus_file_truncate(struct inode *inode)
 			return;
 		mark_inode_dirty(inode);
 		return;
-	} else if (inode->i_size == hip->phys_size)
-		return;
+	}
 
 	blk_cnt = (inode->i_size + HFSPLUS_SB(sb)->alloc_blksz - 1) >>
 			HFSPLUS_SB(sb)->alloc_blksz_shift;
 
+	/*
+	 * hfsplus_file_extend() allocates whole clumps, so alloc_blocks can
+	 * be ahead of phys_size even when phys_size matches i_size. Return
+	 * early only if neither the size nor the allocation needs trimming.
+	 */
+	if (inode->i_size == hip->phys_size && blk_cnt == hip->alloc_blocks)
+		return;
+
 	mutex_lock(&hip->extents_lock);
 
 	alloc_cnt = hip->alloc_blocks;
-- 
2.43.0

[PATCH 2/2] hfs: release preallocated blocks in hfs_file_truncate()

[Viacheslav Dubeyko]

hfs_get_block() extends a fork through hfs_extend_file(),
which allocates whole clumps. Also, hfs_inode_read_fork()
initializes alloc_blocks from the on-disk physical EOF,
while i_size and phys_size are set to the logical EOF.
As a result, alloc_blocks can be ahead of the number of
blocks really needed to hold i_size, even when phys_size
matches i_size.

hfs_file_truncate() returns early whenever i_size equals
phys_size, before it compares the required block count with
alloc_blocks. So, for example, a file whose size is aligned
on the block size keeps its clump preallocation after the
last close. Moreover, hfs_cat_delete() does not free the data
fork and relies on hfs_delete_inode() truncating the file to
zero size. If a file has zero logical size but allocated
blocks, the early return skips the truncation, and its blocks
remain marked as used in the volume bitmap after the file has
been deleted.

Compute the required block count first and return early only
when neither the size nor the allocation needs to be trimmed.
This keeps the intent of the original early return of not
dirtying an unchanged inode. The existing logic further down
already handles the case of blk_cnt being equal to alloc_blocks.

Fixes: f76d28d235cf ("[PATCH] hfs: don't dirty unchanged inode")
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
---
 fs/hfs/extent.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index 26357086517f..42231618f3f1 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -507,11 +507,20 @@ void hfs_file_truncate(struct inode *inode)
 		if (res)
 			inode->i_size = HFS_I(inode)->phys_size;
 		return;
-	} else if (inode->i_size == HFS_I(inode)->phys_size)
-		return;
+	}
+
 	size = inode->i_size + HFS_SB(sb)->alloc_blksz - 1;
 	blk_cnt = size / HFS_SB(sb)->alloc_blksz;
 	alloc_cnt = HFS_I(inode)->alloc_blocks;
+
+	/*
+	 * hfs_extend_file() allocates whole clumps, so alloc_blocks can
+	 * be ahead of phys_size even when phys_size matches i_size. Return
+	 * early only if neither the size nor the allocation needs trimming.
+	 */
+	if (inode->i_size == HFS_I(inode)->phys_size && blk_cnt == alloc_cnt)
+		return;
+
 	if (blk_cnt == alloc_cnt)
 		goto out;
 
-- 
2.43.0