diff mbox series

[07/11] iomap: fix the iomap_fiemap prototype

Message ID 20200427181957.1606257-8-hch@lst.de
State Not Applicable
Headers show
Series [01/11] ext4: fix EXT4_MAX_LOGICAL_BLOCK macro | expand

Commit Message

Christoph Hellwig April 27, 2020, 6:19 p.m. UTC
iomap_fiemap should take u64 start and len arguments, just like the
->fiemap prototype.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/fiemap.c     | 2 +-
 include/linux/iomap.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Darrick Wong April 28, 2020, 3:08 p.m. UTC | #1
On Mon, Apr 27, 2020 at 08:19:53PM +0200, Christoph Hellwig wrote:
> iomap_fiemap should take u64 start and len arguments, just like the
> ->fiemap prototype.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/iomap/fiemap.c     | 2 +-
>  include/linux/iomap.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
> index fca3dfb9d964a..dd04e4added15 100644
> --- a/fs/iomap/fiemap.c
> +++ b/fs/iomap/fiemap.c
> @@ -66,7 +66,7 @@ iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  }
>  
>  int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
> -		loff_t start, loff_t len, const struct iomap_ops *ops)
> +		u64 start, u64 len, const struct iomap_ops *ops)
>  {
>  	struct fiemap_ctx ctx;
>  	loff_t ret;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 8b09463dae0db..63db02528b702 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -178,7 +178,7 @@ int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
>  vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf,
>  			const struct iomap_ops *ops);
>  int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		loff_t start, loff_t len, const struct iomap_ops *ops);
> +		u64 start, u64 len, const struct iomap_ops *ops);
>  loff_t iomap_seek_hole(struct inode *inode, loff_t offset,
>  		const struct iomap_ops *ops);
>  loff_t iomap_seek_data(struct inode *inode, loff_t offset,
> -- 
> 2.26.1
>
Ritesh Harjani May 1, 2020, 11:34 p.m. UTC | #2
On 4/27/20 11:49 PM, Christoph Hellwig wrote:
> iomap_fiemap should take u64 start and len arguments, just like the
> ->fiemap prototype.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

hmm.. I guess,
it's only ->fiemap ops in inode_operations which has
start and len arguments as u64.

While such other ops in struct file_operations have the
arguments of type loff_t. (e.g. ->fallocate, -->llseek etc).

But sure to match the ->fiemap prototype, this patch looks ok to me.

Feel free to add:
Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>

> ---
>   fs/iomap/fiemap.c     | 2 +-
>   include/linux/iomap.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
> index fca3dfb9d964a..dd04e4added15 100644
> --- a/fs/iomap/fiemap.c
> +++ b/fs/iomap/fiemap.c
> @@ -66,7 +66,7 @@ iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>   }
>   
>   int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
> -		loff_t start, loff_t len, const struct iomap_ops *ops)
> +		u64 start, u64 len, const struct iomap_ops *ops)
>   {
>   	struct fiemap_ctx ctx;
>   	loff_t ret;
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 8b09463dae0db..63db02528b702 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -178,7 +178,7 @@ int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
>   vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf,
>   			const struct iomap_ops *ops);
>   int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> -		loff_t start, loff_t len, const struct iomap_ops *ops);
> +		u64 start, u64 len, const struct iomap_ops *ops);
>   loff_t iomap_seek_hole(struct inode *inode, loff_t offset,
>   		const struct iomap_ops *ops);
>   loff_t iomap_seek_data(struct inode *inode, loff_t offset,
>
Christoph Hellwig May 5, 2020, 10:29 a.m. UTC | #3
On Sat, May 02, 2020 at 05:04:01AM +0530, Ritesh Harjani wrote:
>
>
> On 4/27/20 11:49 PM, Christoph Hellwig wrote:
>> iomap_fiemap should take u64 start and len arguments, just like the
>> ->fiemap prototype.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> hmm.. I guess,
> it's only ->fiemap ops in inode_operations which has
> start and len arguments as u64.
>
> While such other ops in struct file_operations have the
> arguments of type loff_t. (e.g. ->fallocate, -->llseek etc).
>
> But sure to match the ->fiemap prototype, this patch looks ok to me.

Yes, fiemap is rather weird here, but it matches the ioctl prototype,
so I'd rather pass it on to the method where fiemap_prep will catch
anything that overflows s_maxbytes due to the signeness of loff_t.
diff mbox series

Patch

diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index fca3dfb9d964a..dd04e4added15 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -66,7 +66,7 @@  iomap_fiemap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 }
 
 int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
-		loff_t start, loff_t len, const struct iomap_ops *ops)
+		u64 start, u64 len, const struct iomap_ops *ops)
 {
 	struct fiemap_ctx ctx;
 	loff_t ret;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 8b09463dae0db..63db02528b702 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -178,7 +178,7 @@  int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero,
 vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf,
 			const struct iomap_ops *ops);
 int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
-		loff_t start, loff_t len, const struct iomap_ops *ops);
+		u64 start, u64 len, const struct iomap_ops *ops);
 loff_t iomap_seek_hole(struct inode *inode, loff_t offset,
 		const struct iomap_ops *ops);
 loff_t iomap_seek_data(struct inode *inode, loff_t offset,