xfs
[Top] [All Lists]

Re: [PATCH 2/3] [PATCH 2/3] xfs: do not use xfs_mod_incore_sb for per-cp

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/3] [PATCH 2/3] xfs: do not use xfs_mod_incore_sb for per-cpu counters
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 29 Sep 2010 07:45:20 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20100929113914.GP5665@dastard>
References: <20100929072221.583672974@xxxxxxxxxxxxxxxxxxxxxx> <20100929072238.162539532@xxxxxxxxxxxxxxxxxxxxxx> <20100929113914.GP5665@dastard>
User-agent: Mutt/1.5.20 (2009-08-17)
On Wed, Sep 29, 2010 at 09:39:14PM +1000, Dave Chinner wrote:
> Not sure I like the indenting of the second line. I'd prefer the
> parameters to have a little more indent or use three lines...

That whole area needs some larger refactoring / reformatting work.
I'll see if I can ad danother patch for that.

> > +   ASSERT(field < XFS_SBS_ICOUNT || field > XFS_SBS_FDBLOCKS);
> 
> That assert will cause issues with:
> 
> > @@ -97,6 +99,8 @@ extern void       xfs_icsb_sync_counters_locke
> >  #define xfs_icsb_reinit_counters(mp)               do { } while (0)
> >  #define xfs_icsb_sync_counters(mp, flags)  do { } while (0)
> >  #define xfs_icsb_sync_counters_locked(mp, flags) do { } while (0)
> > +#define xfs_icsb_modify_counters(mp, field, delta, rsvd) \
> > +   xfs_mod_incore_sb(mp, field, delta, rsvd)
> >  #endif
> 
> UP builds. Perhaps a CONFIG_SMP only assert? Given that the per-cpu
> counter rework I'm doing doesn't have a different code path for
> UP vs SMP, it'd only be a temporary concern....

Indeed, it should be conditional.

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