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

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(-)

Ted,

there are still endianness problems with collapse range, I kind of
remember seeing some patches to fix that but I can not find those
anywhere. So let me know if you have those, otherwise I'll send
some.

Also since this patch set (or rather the patch #5) fixes the
collapse range so that it does not fail on fsx and fsstress we can
drop the fallocate mode block patches I think :)

Thanks!
-Lukas

On Wed, 16 Apr 2014, Lukas Czerner wrote:

> Date: Wed, 16 Apr 2014 20:32:58 +0200
> From: Lukas Czerner <lczerner@redhat.com>
> To: linux-ext4@vger.kernel.org
> Cc: linkinjeon@gmail.com, Lukas Czerner <lczerner@redhat.com>
> Subject: [PATCH 1/5] ext4: Use filemap_write_and_wait_range() correctly in
>     collapse range
> 
> 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;
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

So unfortunately this does not fix all the problems. Even though
this fixes all the problems with collapse rage when page size ==
block size it still fails some tests when page size > block size so
I guess we still have some work to do.

Thanks!
-Lukas

On Wed, 16 Apr 2014, Lukas Czerner wrote:

> Date: Wed, 16 Apr 2014 20:32:58 +0200
> From: Lukas Czerner <lczerner@redhat.com>
> To: linux-ext4@vger.kernel.org
> Cc: linkinjeon@gmail.com, Lukas Czerner <lczerner@redhat.com>
> Subject: [PATCH 1/5] ext4: Use filemap_write_and_wait_range() correctly in
>     collapse range
> 
> 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;
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Apr 16, 2014 at 08:32:58PM +0200, Lukas Czerner wrote:
> 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>

Thanks, applied.

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Apr 16, 2014 at 08:38:01PM +0200, Lukáš Czerner wrote:
> there are still endianness problems with collapse range, I kind of
> remember seeing some patches to fix that but I can not find those
> anywhere. So let me know if you have those, otherwise I'll send
> some.

Yes, those are in the tree already.  I had to adjust the patch #5
slightly so it would apply as a result.

> Also since this patch set (or rather the patch #5) fixes the
> collapse range so that it does not fail on fsx and fsstress we can
> drop the fallocate mode block patches I think :)

I'll move it to the unstable part of the tree as a debugging patch,
pending the xfstests changes to make it easy to block things like the
on-deck INSERT_RANGE patchset.  (BTW, it would be great if the
xfstests changes for INSERT_RANGE had an easy way to disable
INSERT_RANGE with a single environment variable.)

The point is that otherwise, I can't let the INSERT_RANGE patches into
the dev branch until it's completely regression free, or else it
interferes the testing of other patches during the development cycle.
Of course, if there are still problems that aren't solved by the end
of the development cycle, I'll still drop the patches and hold them
for the next merge window.

Thanks,

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html