Re: [PATCH v9 1/6] xfs: Move code around and remove typedefs

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

 



On Mon, 2013-06-24 at 15:31 +1000, Dave Chinner wrote:
> On Sun, Jun 23, 2013 at 09:48:22PM -0500, Chandra Seetharaman wrote:
> > Removed some typedefs, defined new functions, made some code clean up all in
> > preparation of the series.
> > 
> > No functional changes.
> 
> This does a lot of different stuff. Can you separate out the actual
> factoring changes (e.g. xfs_is_quota_inode() and xfs_dquot_tree())
> into separate patches, and the same with the struct xfs_dquot_acct
> as it changes both logic and structure...

Sure. I will follow Ben's suggestion in splitting these.
> 
> > diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> > index 5d16a6e..4b330f2 100644
> > --- a/fs/xfs/xfs_qm.h
> > +++ b/fs/xfs/xfs_qm.h
> > @@ -42,57 +42,89 @@ extern struct kmem_zone	*xfs_qm_dqtrxzone;
> >   * The mount structure keeps a pointer to this.
> >   */
> >  typedef struct xfs_quotainfo {
> > -	struct radix_tree_root qi_uquota_tree;
> > -	struct radix_tree_root qi_gquota_tree;
> > -	struct mutex qi_tree_lock;
> > -	xfs_inode_t	*qi_uquotaip;	 /* user quota inode */
> > -	xfs_inode_t	*qi_gquotaip;	 /* group quota inode */
> > -	struct list_head qi_lru_list;
> > -	struct mutex	 qi_lru_lock;
> > -	int		 qi_lru_count;
> > -	int		 qi_dquots;
> > -	time_t		 qi_btimelimit;	 /* limit for blks timer */
> > -	time_t		 qi_itimelimit;	 /* limit for inodes timer */
> > -	time_t		 qi_rtbtimelimit;/* limit for rt blks timer */
> > -	xfs_qwarncnt_t	 qi_bwarnlimit;	 /* limit for blks warnings */
> > -	xfs_qwarncnt_t	 qi_iwarnlimit;	 /* limit for inodes warnings */
> > -	xfs_qwarncnt_t	 qi_rtbwarnlimit;/* limit for rt blks warnings */
> > -	struct mutex	 qi_quotaofflock;/* to serialize quotaoff */
> > -	xfs_filblks_t	 qi_dqchunklen;	 /* # BBs in a chunk of dqs */
> > -	uint		 qi_dqperchunk;	 /* # ondisk dqs in above chunk */
> > -	xfs_qcnt_t	 qi_bhardlimit;	 /* default data blk hard limit */
> > -	xfs_qcnt_t	 qi_bsoftlimit;	 /* default data blk soft limit */
> > -	xfs_qcnt_t	 qi_ihardlimit;	 /* default inode count hard limit */
> > -	xfs_qcnt_t	 qi_isoftlimit;	 /* default inode count soft limit */
> > -	xfs_qcnt_t	 qi_rtbhardlimit;/* default realtime blk hard limit */
> > -	xfs_qcnt_t	 qi_rtbsoftlimit;/* default realtime blk soft limit */
> > -	struct shrinker  qi_shrinker;
> > +	struct radix_tree_root	qi_uquota_tree;
> > +	struct radix_tree_root	qi_gquota_tree;
> > +	struct mutex		qi_tree_lock;
> > +	struct xfs_inode	*qi_uquotaip;	 /* user quota inode */
> > +	struct xfs_inode	*qi_gquotaip;	 /* group quota inode */
> > +	struct list_head	qi_lru_list;
> > +	struct mutex		qi_lru_lock;
> > +	int		qi_lru_count;
> > +	int		qi_dquots;
> > +	time_t		qi_btimelimit;	 /* limit for blks timer */
> > +	time_t		qi_itimelimit;	 /* limit for inodes timer */
> > +	time_t		qi_rtbtimelimit;/* limit for rt blks timer */
> > +	xfs_qwarncnt_t	qi_bwarnlimit;	 /* limit for blks warnings */
> > +	xfs_qwarncnt_t	qi_iwarnlimit;	 /* limit for inodes warnings */
> > +	xfs_qwarncnt_t	qi_rtbwarnlimit;/* limit for rt blks warnings */
> > +	struct mutex	qi_quotaofflock;/* to serialize quotaoff */
> > +	xfs_filblks_t	qi_dqchunklen;	 /* # BBs in a chunk of dqs */
> > +	uint		qi_dqperchunk;	 /* # ondisk dqs in above chunk */
> > +	xfs_qcnt_t	qi_bhardlimit;	 /* default data blk hard limit */
> > +	xfs_qcnt_t	qi_bsoftlimit;	 /* default data blk soft limit */
> > +	xfs_qcnt_t	qi_ihardlimit;	 /* default inode count hard limit */
> > +	xfs_qcnt_t	qi_isoftlimit;	 /* default inode count soft limit */
> > +	xfs_qcnt_t	qi_rtbhardlimit;/* default realtime blk hard limit */
> > +	xfs_qcnt_t	qi_rtbsoftlimit;/* default realtime blk soft limit */
> > +	struct shrinker	qi_shrinker;
> >  } xfs_quotainfo_t;
> 
> I don't see much point in random whitespace cleanups like this. If
> you are going to change them, at least clean it up properly, so all
> variables are aligned, and all the comments are aligned as well. In
> most cases, the comments are redundant because the variable names
> are very descriptive and so probably could be removed....

I will align all these at the cost of these "valuable" comments :)
> 
> Cheers,
> 
> Dave.


_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs




[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux