RE: [PATCH 2/5] ext4: fix removing status extents in ext4_collapse_range()

RE: [PATCH 2/5] ext4: fix removing status extents in ext4_collapse_range()

[Namjae Jeon]

> Currently in ext4_collapse_range() when calling ext4_es_remove_extent() to
> remove status extents we're passing (EXT_MAX_BLOCKS - punch_start - 1)
> in order to remove all extents from start of the collapse range to the
> end of the file. However this is wrong because we might miss the
> possible extent covering the last block of the file.
> 
> Fix it by removing the -1.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Looks good to me! 

Reviewed-by: Namjae Jeon <namjae.jeon@samsung.com>

Thanks Lukas.

Re: [PATCH 2/5] ext4: fix removing status extents in ext4_collapse_range()

[Theodore Ts'o]

On Thu, Apr 17, 2014 at 02:54:18PM +0900, Namjae Jeon wrote:
> > Currently in ext4_collapse_range() when calling ext4_es_remove_extent() to
> > remove status extents we're passing (EXT_MAX_BLOCKS - punch_start - 1)
> > in order to remove all extents from start of the collapse range to the
> > end of the file. However this is wrong because we might miss the
> > possible extent covering the last block of the file.
> > 
> > Fix it by removing the -1.
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Looks good to me! 
> 
> Reviewed-by: Namjae Jeon <namjae.jeon@samsung.com>

Thanks, applied.

					- Ted

[PATCH 1/5] ext4: Use filemap_write_and_wait_range() correctly in collapse range

[Lukas Czerner]

Currently we're passing -1 as lend argumnet for
filemap_write_and_wait_range() which is wrong since lend is signed type
so it would cause some confusion and we might not write_and_wait for the
entire range we're expecting to write.

Fix it by using LLONG_MAX instead.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/extents.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ff823b7..821c1d4 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5378,7 +5378,7 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	punch_stop = (offset + len) >> EXT4_BLOCK_SIZE_BITS(sb);
 
 	/* Write out all dirty pages */
-	ret = filemap_write_and_wait_range(inode->i_mapping, offset, -1);
+	ret = filemap_write_and_wait_range(inode->i_mapping, offset, LLONG_MAX);
 	if (ret)
 		return ret;
 
-- 
1.8.3.1

[PATCH 2/5] ext4: fix removing status extents in ext4_collapse_range()

[Lukas Czerner]

Currently in ext4_collapse_range() when calling ext4_es_remove_extent() to
remove status extents we're passing (EXT_MAX_BLOCKS - punch_start - 1)
in order to remove all extents from start of the collapse range to the
end of the file. However this is wrong because we might miss the
possible extent covering the last block of the file.

Fix it by removing the -1.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/extents.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 821c1d4..25ed60f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5422,7 +5422,7 @@ int ext4_collapse_range(struct inode *inode, loff_t offset, loff_t len)
 	ext4_discard_preallocations(inode);
 
 	ret = ext4_es_remove_extent(inode, punch_start,
-				    EXT_MAX_BLOCKS - punch_start - 1);
+				    EXT_MAX_BLOCKS - punch_start);
 	if (ret) {
 		up_write(&EXT4_I(inode)->i_data_sem);
 		goto out_stop;
-- 
1.8.3.1