Re: [PATCH 5/9] fs: Move start and length fiemap fields into fiemap_extent_info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux