xfs
[Top] [All Lists]

Re: [PATCH 15/16] xfs: kill xfs_qm_idtodq

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 15/16] xfs: kill xfs_qm_idtodq
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 5 Dec 2011 16:31:31 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20111128082838.888554428@xxxxxxxxxxxxxxxxxxxxxx>
References: <20111128082722.604873274@xxxxxxxxxxxxxxxxxxxxxx> <20111128082838.888554428@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Nov 28, 2011 at 03:27:37AM -0500, Christoph Hellwig wrote:
> This function doesn't help the code flow, so merge the dquot allocation and
> transaction handling into xfs_qm_dqread.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

This version is much easier to follow.

Couple of small things:

> ---
>  fs/xfs/xfs_dquot.c |  134 
> ++++++++++++++++++-----------------------------------
>  1 file changed, 47 insertions(+), 87 deletions(-)
> 
> Index: xfs/fs/xfs/xfs_dquot.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_dquot.c       2011-11-25 11:56:40.861789014 +0100
> +++ xfs/fs/xfs/xfs_dquot.c    2011-11-25 11:56:42.488446869 +0100
> @@ -550,36 +550,59 @@ xfs_qm_dqtobp(
>   * Read in the ondisk dquot using dqtobp() then copy it to an incore version,
>   * and release the buffer immediately.
>   *
> + * If XFS_QMOPT_DQALLOC is set, allocate a dquot on disk if it needed.
>   */
> -/* ARGSUSED */
>  STATIC int
>  xfs_qm_dqread(
> -     xfs_trans_t     **tpp,
> -     xfs_dqid_t      id,
> -     xfs_dquot_t     *dqp,   /* dquot to get filled in */
> -     uint            flags)
> +     struct xfs_mount        *mp,
> +     xfs_dqid_t              id,
                  ^^^ whitespace damage
> +     uint                    type,
                ^^^^^ whitespace damage

> +     uint                    flags,
> +     struct xfs_dquot        **O_dqpp)
>  {
> -     xfs_disk_dquot_t *ddqp;
> -     xfs_buf_t        *bp;
> -     int              error;
> -     xfs_trans_t      *tp;
> +     struct xfs_dquot        *dqp;
> +     struct xfs_disk_dquot   *ddqp;
> +     struct xfs_buf           *bp;
> +     struct xfs_trans         *tp = NULL;
> +     int                      error;
> +     int                     cancelflags = 0;

Extra whitespace in the bp/tp/error declarations.

>  
> -     ASSERT(tpp);
> +     dqp = xfs_qm_dqinit(mp, id, type);
>  
>       trace_xfs_dqread(dqp);
>  
> +     if (flags & XFS_QMOPT_DQALLOC) {
> +             tp = xfs_trans_alloc(mp, XFS_TRANS_QM_DQALLOC);
> +             error = xfs_trans_reserve(tp, XFS_QM_DQALLOC_SPACE_RES(mp),
> +                             XFS_WRITE_LOG_RES(mp) +
> +                             BBTOB(mp->m_quotainfo->qi_dqchunklen) - 1 +
> +                             128,

That reservation value is asking for a macro or separate variable
and a comment explaining it....

therwise, looks OK.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH 15/16] xfs: kill xfs_qm_idtodq, Dave Chinner <=