[PATCH 15/16] xfs: kill xfs_qm_idtodq

[Dave Chinner]

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 at lst.de>

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 at redhat.com>
-- 
Dave Chinner
david at fromorbit.com