[PATCH 1/2] xfs: report zeroed or not correctly in xfs_zero_range()

[PATCH 1/2] xfs: report zeroed or not correctly in xfs_zero_range()

[Eryu Guan]

The 'did_zero' param of xfs_zero_range() was not passed to
iomap_zero_range() correctly. This was introduced by commit
7bb41db3ea16 ("xfs: handle 64-bit length in xfs_iozero"), and found
by code inspection.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 fs/xfs/xfs_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index c4893e226fd8..812cd17b331f 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -58,7 +58,7 @@ xfs_zero_range(
 	xfs_off_t		count,
 	bool			*did_zero)
 {
-	return iomap_zero_range(VFS_I(ip), pos, count, NULL, &xfs_iomap_ops);
+	return iomap_zero_range(VFS_I(ip), pos, count, did_zero, &xfs_iomap_ops);
 }
 
 int
-- 
2.13.5

[PATCH 2/2] xfs: kill meaningless variable 'zero'

[Eryu Guan]

In xfs_file_aio_write_checks(), variable 'zero' is there only to
satisfy xfs_zero_eof(), the result of it is ignored. Now, with
iomap_zero_range() based xfs_zero_eof(), we can safely pass NULL as
the last param of it and kill 'zero'.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 fs/xfs/xfs_file.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 812cd17b331f..e7dbeed0f0cb 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -373,8 +373,6 @@ xfs_file_aio_write_checks(
 	 */
 	spin_lock(&ip->i_flags_lock);
 	if (iocb->ki_pos > i_size_read(inode)) {
-		bool	zero = false;
-
 		spin_unlock(&ip->i_flags_lock);
 		if (!drained_dio) {
 			if (*iolock == XFS_IOLOCK_SHARED) {
@@ -395,7 +393,7 @@ xfs_file_aio_write_checks(
 			drained_dio = true;
 			goto restart;
 		}
-		error = xfs_zero_eof(ip, iocb->ki_pos, i_size_read(inode), &zero);
+		error = xfs_zero_eof(ip, iocb->ki_pos, i_size_read(inode), NULL);
 		if (error)
 			return error;
 	} else
-- 
2.13.5

Re: [PATCH 1/2] xfs: report zeroed or not correctly in xfs_zero_range()

[Carlos Maiolino]

On Thu, Sep 07, 2017 at 11:42:53AM +0800, Eryu Guan wrote:
> The 'did_zero' param of xfs_zero_range() was not passed to
> iomap_zero_range() correctly. This was introduced by commit
> 7bb41db3ea16 ("xfs: handle 64-bit length in xfs_iozero"), and found
> by code inspection.
> 

Looks good,

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  fs/xfs/xfs_file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index c4893e226fd8..812cd17b331f 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -58,7 +58,7 @@ xfs_zero_range(
>  	xfs_off_t		count,
>  	bool			*did_zero)
>  {
> -	return iomap_zero_range(VFS_I(ip), pos, count, NULL, &xfs_iomap_ops);
> +	return iomap_zero_range(VFS_I(ip), pos, count, did_zero, &xfs_iomap_ops);
>  }
>  
>  int
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

Re: [PATCH 2/2] xfs: kill meaningless variable 'zero'

[Carlos Maiolino]

On Thu, Sep 07, 2017 at 11:42:54AM +0800, Eryu Guan wrote:
> In xfs_file_aio_write_checks(), variable 'zero' is there only to
> satisfy xfs_zero_eof(), the result of it is ignored. Now, with
> iomap_zero_range() based xfs_zero_eof(), we can safely pass NULL as
> the last param of it and kill 'zero'.
> 

Looks good to me, you can add:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
>  fs/xfs/xfs_file.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 812cd17b331f..e7dbeed0f0cb 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -373,8 +373,6 @@ xfs_file_aio_write_checks(
>  	 */
>  	spin_lock(&ip->i_flags_lock);
>  	if (iocb->ki_pos > i_size_read(inode)) {
> -		bool	zero = false;
> -
>  		spin_unlock(&ip->i_flags_lock);
>  		if (!drained_dio) {
>  			if (*iolock == XFS_IOLOCK_SHARED) {
> @@ -395,7 +393,7 @@ xfs_file_aio_write_checks(
>  			drained_dio = true;
>  			goto restart;
>  		}
> -		error = xfs_zero_eof(ip, iocb->ki_pos, i_size_read(inode), &zero);
> +		error = xfs_zero_eof(ip, iocb->ki_pos, i_size_read(inode), NULL);
>  		if (error)
>  			return error;
>  	} else
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

Re: [PATCH 1/2] xfs: report zeroed or not correctly in xfs_zero_range()

[Christoph Hellwig]

On Thu, Sep 07, 2017 at 11:42:53AM +0800, Eryu Guan wrote:
> The 'did_zero' param of xfs_zero_range() was not passed to
> iomap_zero_range() correctly. This was introduced by commit
> 7bb41db3ea16 ("xfs: handle 64-bit length in xfs_iozero"), and found
> by code inspection.

Looks good.  Although this makes me wonder why we pass the
argument at all, given that it doesn't seem to matter :)

Joking aside: xfs_setattr_size seems to care to write out
the buffer cache zeroed byes, so this looks good to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] xfs: kill meaningless variable 'zero'

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] xfs: report zeroed or not correctly in xfs_zero_range()

[Darrick J. Wong]

On Thu, Sep 07, 2017 at 11:42:53AM +0800, Eryu Guan wrote:
> The 'did_zero' param of xfs_zero_range() was not passed to
> iomap_zero_range() correctly. This was introduced by commit
> 7bb41db3ea16 ("xfs: handle 64-bit length in xfs_iozero"), and found
> by code inspection.
> 
> Signed-off-by: Eryu Guan <eguan@redhat.com>

Both look ok,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index c4893e226fd8..812cd17b331f 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -58,7 +58,7 @@ xfs_zero_range(
>  	xfs_off_t		count,
>  	bool			*did_zero)
>  {
> -	return iomap_zero_range(VFS_I(ip), pos, count, NULL, &xfs_iomap_ops);
> +	return iomap_zero_range(VFS_I(ip), pos, count, did_zero, &xfs_iomap_ops);
>  }
>  
>  int
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html