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.
|