On Fri, Jul 13, 2012 at 03:19:07PM +0200, Lukas Czerner wrote:
> Currently we're passing -1 to truncate_inode_pages_range() which is
> actually really confusing since the argument is signed so we do not get
> "huge" number as one would expect, but rather just -1. To make things
> clearer and easier for truncate_inode_pages_range() just pass LLONG_MAX
> since it is actually what was intended anyway.
>
> It also makes thing easier for allowing truncate_inode_pages_range() to
> handle non page aligned regions. Moreover letting the lend argument to
> be negative might actually hide some bugs.
>
> Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
> Cc: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_fs_subr.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/xfs/xfs_fs_subr.c b/fs/xfs/xfs_fs_subr.c
> index 652b875..6e9b052 100644
> --- a/fs/xfs/xfs_fs_subr.c
> +++ b/fs/xfs/xfs_fs_subr.c
> @@ -34,7 +34,8 @@ xfs_tosspages(
> {
> /* can't toss partial tail pages, so mask them out */
> last &= ~(PAGE_SIZE - 1);
> - truncate_inode_pages_range(VFS_I(ip)->i_mapping, first, last - 1);
> + truncate_inode_pages_range(VFS_I(ip)->i_mapping, first,
> + last == -1 ? LLONG_MAX : last);
The last paramter changed from (last -1) to last. so if we pass in
last = 16384, we now truncate to 16384 (first byte of page index 5)
instead of 16383 (last byte of page index 4). That's a change of
behaviour and a potential off-by one error, right?
> @@ -53,7 +54,8 @@ xfs_flushinval_pages(
> ret = filemap_write_and_wait_range(mapping, first,
> last == -1 ? LLONG_MAX : last);
> if (!ret)
> - truncate_inode_pages_range(mapping, first, last);
> + truncate_inode_pages_range(mapping, first,
> + last == -1 ? LLONG_MAX : last);
Given this is also done immediately above in the function, perhaps
this should be done before anything:
if (last == -1)
last = LLONG_MAX;
and the parameter simply passed to the two functions without the
conditional logic?
Cheers,
Dave.
--
Dave Chinner
dchinner@xxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
