All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: a.p.zijlstra@chello.nl, linux-kernel@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: [PATCH 3/3] xfs: demultiplex xfs_icsb_modify_counters()
Date: Tue, 30 Nov 2010 15:38:17 +1100	[thread overview]
Message-ID: <20101130043817.GE3556@dastard> (raw)
In-Reply-To: <20101129091920.GB13902@infradead.org>

On Mon, Nov 29, 2010 at 04:19:20AM -0500, Christoph Hellwig wrote:
> On Mon, Nov 29, 2010 at 11:36:41AM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > With the conversion to percpu counters, xfs_icsb_modify_counters() really does
> > not need to exist. Convert the inode counter modifications to use a common
> > helper function for the one place that calls them, and add another function for
> > the free block modification and convert all the callers to use that.
> 
> > +xfs_icsb_modify_inodes(
> > +	xfs_mount_t	*mp,
> 
> struct xfs_mount, please.
> 
> > +	int		cntr,
> > +	int64_t		delta,
> > +	int		rsvd)
> 
> the rsvd argument isn't used at all.
> 
> > +{
> > +	int		ret = 0;
> > +
> > +	ASSERT(cntr == XFS_ICSB_ICOUNT || cntr == XFS_ICSB_IFREE);
> > +
> > +	ret = xfs_icsb_add(mp, cntr, delta, 0);
> > +	if (ret < 0) {
> > +		ASSERT(0);
> > +		return XFS_ERROR(EINVAL);
> > +	}
> > +	return 0;
> 
> You could get rdif of the ret argument as we don't care about the
> value.  I also don't think we need the assert here - the caller already
> does one for us.
> 
> > +xfs_icsb_modify_free_blocks(
> > +	xfs_mount_t	*mp,
> 
> same here.

Ok, makes sense. I'll clean it up.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com, linux-kernel@vger.kernel.org, a.p.zijlstra@chello.nl
Subject: Re: [PATCH 3/3] xfs: demultiplex xfs_icsb_modify_counters()
Date: Tue, 30 Nov 2010 15:38:17 +1100	[thread overview]
Message-ID: <20101130043817.GE3556@dastard> (raw)
In-Reply-To: <20101129091920.GB13902@infradead.org>

On Mon, Nov 29, 2010 at 04:19:20AM -0500, Christoph Hellwig wrote:
> On Mon, Nov 29, 2010 at 11:36:41AM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > With the conversion to percpu counters, xfs_icsb_modify_counters() really does
> > not need to exist. Convert the inode counter modifications to use a common
> > helper function for the one place that calls them, and add another function for
> > the free block modification and convert all the callers to use that.
> 
> > +xfs_icsb_modify_inodes(
> > +	xfs_mount_t	*mp,
> 
> struct xfs_mount, please.
> 
> > +	int		cntr,
> > +	int64_t		delta,
> > +	int		rsvd)
> 
> the rsvd argument isn't used at all.
> 
> > +{
> > +	int		ret = 0;
> > +
> > +	ASSERT(cntr == XFS_ICSB_ICOUNT || cntr == XFS_ICSB_IFREE);
> > +
> > +	ret = xfs_icsb_add(mp, cntr, delta, 0);
> > +	if (ret < 0) {
> > +		ASSERT(0);
> > +		return XFS_ERROR(EINVAL);
> > +	}
> > +	return 0;
> 
> You could get rdif of the ret argument as we don't care about the
> value.  I also don't think we need the assert here - the caller already
> does one for us.
> 
> > +xfs_icsb_modify_free_blocks(
> > +	xfs_mount_t	*mp,
> 
> same here.

Ok, makes sense. I'll clean it up.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2010-11-30  4:36 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-29  0:36 [PATCH 0/3] Use generic percpu counters in XFS Dave Chinner
2010-11-29  0:36 ` Dave Chinner
2010-11-29  0:36 ` [PATCH 1/3] lib: percpu counter add unless less than functionality Dave Chinner
2010-11-29  0:36   ` Dave Chinner
2010-11-29 12:08   ` Peter Zijlstra
2010-11-29 12:08     ` Peter Zijlstra
2010-11-30  4:30     ` Dave Chinner
2010-11-30  4:30       ` Dave Chinner
2010-11-29  0:36 ` [PATCH 2/3] xfs: use generic per-cpu counter infrastructure Dave Chinner
2010-11-29  0:36   ` Dave Chinner
2010-11-29  9:16   ` Christoph Hellwig
2010-11-29  9:16     ` Christoph Hellwig
2010-11-30  4:37     ` Dave Chinner
2010-11-30  4:37       ` Dave Chinner
2010-11-29  0:36 ` [PATCH 3/3] xfs: demultiplex xfs_icsb_modify_counters() Dave Chinner
2010-11-29  0:36   ` Dave Chinner
2010-11-29  9:19   ` Christoph Hellwig
2010-11-29  9:19     ` Christoph Hellwig
2010-11-30  4:38     ` Dave Chinner [this message]
2010-11-30  4:38       ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2010-12-13  1:21 [PATCH 0/3] Use generic percpu counters in XFS V2 Dave Chinner
2010-12-13  1:21 ` [PATCH 3/3] xfs: demultiplex xfs_icsb_modify_counters() Dave Chinner
2010-12-13  1:21   ` Dave Chinner
2010-12-16 15:38   ` Christoph Hellwig
2010-12-16 15:38     ` Christoph Hellwig

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=20101130043817.GE3556@dastard \
    --to=david@fromorbit.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.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.