xfs
[Top] [All Lists]

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

To: Chandra Seetharaman <sekharan@xxxxxxxxxx>
Subject: Re: [PATCH v9 1/6] xfs: Move code around and remove typedefs
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 24 Jun 2013 15:31:54 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1372042107-27332-2-git-send-email-sekharan@xxxxxxxxxx>
References: <1372042107-27332-1-git-send-email-sekharan@xxxxxxxxxx> <1372042107-27332-2-git-send-email-sekharan@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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...

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

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>