All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 15/16] xfs: kill xfs_qm_idtodq
Date: Mon, 5 Dec 2011 16:31:31 +1100	[thread overview]
Message-ID: <20111205053131.GV7046@dastard> (raw)
In-Reply-To: <20111128082838.888554428@bombadil.infradead.org>

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

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

  reply	other threads:[~2011-12-05  6:16 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-28  8:27 [PATCH 00/16] quota cleanups for Linux 3.3 Christoph Hellwig
2011-11-28  8:27 ` [PATCH 01/16] xfs: untange SYNC_WAIT and SYNC_TRYLOCK meanings for xfs_qm_dqflush Christoph Hellwig
2011-12-05  3:57   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 02/16] xfs: make sure to really flush all dquots in xfs_qm_quotacheck Christoph Hellwig
2011-12-05  3:59   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 03/16] xfs: remove xfs_qm_sync Christoph Hellwig
2011-12-05  4:09   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 04/16] xfs: remove the sync_mode argument to xfs_qm_dqflush_all Christoph Hellwig
2011-12-05  4:09   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 05/16] xfs: cleanup dquot locking helpers Christoph Hellwig
2011-12-05  4:12   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 06/16] xfs: cleanup xfs_qm_dqlookup Christoph Hellwig
2011-12-05  4:17   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 07/16] xfs: remove XFS_DQ_INACTIVE Christoph Hellwig
2011-12-05  4:23   ` Dave Chinner
2011-12-05  8:37     ` Christoph Hellwig
2011-12-06 14:43       ` Christoph Hellwig
2011-12-06 20:34         ` Dave Chinner
2011-11-28  8:27 ` [PATCH 08/16] xfs: implement lazy removal for the dquot freelist Christoph Hellwig
2011-12-05  4:32   ` Dave Chinner
2011-12-05  8:38     ` Christoph Hellwig
2011-11-28  8:27 ` [PATCH 09/16] xfs: flatten the dquot lock ordering Christoph Hellwig
2011-12-05  5:04   ` Dave Chinner
2011-12-05  9:11     ` Christoph Hellwig
2011-12-05  9:34       ` Dave Chinner
2011-12-05 11:50         ` Christoph Hellwig
2011-12-06  0:25           ` Dave Chinner
2011-11-28  8:27 ` [PATCH 10/16] xfs: nest qm_dqfrlist_lock insise the dquot qlock Christoph Hellwig
2011-12-05  5:10   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 11/16] xfs: simplify xfs_qm_dqattach_grouphint Christoph Hellwig
2011-12-05  5:14   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 12/16] xfs: simplify xfs_qm_detach_gdquots Christoph Hellwig
2011-12-05  5:18   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 13/16] xfs: add a xfs_dqhold helper Christoph Hellwig
2011-12-05  5:22   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 14/16] xfs: merge xfs_qm_dqinit_core into the only caller Christoph Hellwig
2011-12-05  5:23   ` Dave Chinner
2011-11-28  8:27 ` [PATCH 15/16] xfs: kill xfs_qm_idtodq Christoph Hellwig
2011-12-05  5:31   ` Dave Chinner [this message]
2011-11-28  8:27 ` [PATCH 16/16] xfs: remove XFS_QMOPT_DQSUSER Christoph Hellwig
2011-12-05  5:34   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111205053131.GV7046@dastard \
    --to=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.