On Fri, Aug 02, 2019 at 08:15:19AM -0700, Darrick J. Wong wrote:
> On Fri, Aug 02, 2019 at 11:51:16AM +0200, Carlos Maiolino wrote:
> > > >
> > > > STATIC int
> > > > xfs_vn_fiemap(
> > > > - struct inode *inode,
> > > > - struct fiemap_extent_info *fieinfo,
> > > > - u64 start,
> > > > - u64 length)
> > > > + struct inode *inode,
> > > > + struct fiemap_extent_info *fieinfo)
> > > > {
> > > > - int error;
> > > > + u64 start = fieinfo->fi_start;
> > > > + u64 length = fieinfo->fi_len;
> > > > + int error;
> > >
> > > Would be nice if the variable name indentation was consistent here, but
> > > otherwise the xfs part looks ok.
> >
> > These fields are removed on the next patch, updating it is really required?
>
> Yes, please.
NP then
> > > > + u64 fi_start;
> > > > + u64 fi_len;
> > >
> > > Comments for these two new fields?
> >
> > Sure, how about this:
> >
> > u64 fi_start; /* Logical offset at which
> > start mapping */
> > u64 fi_len; /* Logical length of mapping
> > the caller cares about */
> >
> >
> > btw, Above indentation won't match final result
>
> Looks good to me.
Will update the fields, thanks for the review
>
> >
> > Christoph, may I keep your reviewed tag by updating the comments as above?
> > Otherwise I'll just remove your tag
> >
> > >
> > > --D
> > >
> > > > + unsigned int fi_extents_mapped; /* Number of mapped extents */
> > > > + unsigned int fi_extents_max; /* Size of fiemap_extent array */
> > > > + struct fiemap_extent __user *fi_extents_start; /* Start of
> > > > + fiemap_extent
> > > > + array */
> > > > };
> > > > int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> > > > u64 phys, u64 len, u32 flags);
> > > > @@ -1841,8 +1844,7 @@ struct inode_operations {
> > > > int (*setattr) (struct dentry *, struct iattr *);
> > > > int (*getattr) (const struct path *, struct kstat *, u32, unsigned int);
> > > > ssize_t (*listxattr) (struct dentry *, char *, size_t);
> > > > - int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
> > > > - u64 len);
> > > > + int (*fiemap)(struct inode *, struct fiemap_extent_info *);
> > > > int (*update_time)(struct inode *, struct timespec64 *, int);
> > > > int (*atomic_open)(struct inode *, struct dentry *,
> > > > struct file *, unsigned open_flag,
> > > > @@ -3199,11 +3201,10 @@ extern int vfs_readlink(struct dentry *, char __user *, int);
> > > >
> > > > extern int __generic_block_fiemap(struct inode *inode,
> > > > struct fiemap_extent_info *fieinfo,
> > > > - loff_t start, loff_t len,
> > > > get_block_t *get_block);
> > > > extern int generic_block_fiemap(struct inode *inode,
> > > > - struct fiemap_extent_info *fieinfo, u64 start,
> > > > - u64 len, get_block_t *get_block);
> > > > + struct fiemap_extent_info *fieinfo,
> > > > + get_block_t *get_block);
> > > >
> > > > extern struct file_system_type *get_filesystem(struct file_system_type *fs);
> > > > extern void put_filesystem(struct file_system_type *fs);
> > > > --
> > > > 2.20.1
> > > >
> >
> > --
> > Carlos
--
Carlos
