On Thu, Feb 20, 2014 at 01:06:02PM -0600, Eric Sandeen wrote:
> On 2/19/14, 11:55 PM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > When discontiguous directory buffer support was fixed in xfs_repair,
> > (dd9093d xfs_repair: fix discontiguous directory block support)
> > it changed to using libxfs_getbuf_map() to support mapping
> > discontiguous blocks, and the prefetch code special cased such
> > discontiguous buffers.
> >
> > The issue is that libxfs_getbuf_map() marks all buffers, even
> > contiguous ones - as LIBXFS_B_DISCONTIG, and so the prefetch code
> > was treating every buffer as discontiguous. This causes the prefetch
> > code to completely bypass the large IO optimisations for dense areas
> > of metadata. Because there was no obvious change in performance or
> > IO patterns, this wasn't noticed during performance testing.
> >
> > However, this change mysteriously fixed a regression in xfs/033 in
> > the v3.2.0-alpha release, and this change in behaviour was
> > discovered as part of triaging why it "fixed" the regression.
> > Anyway, restoring the large IO prefetch optimisation results
> > a reapiron a 10 million inode filesystem dropping from 197s to 173s,
> > and the peak IOPS rate in phase 3 dropping from 25,000 to roughly
> > 2,000 by trading off a bandwidth increase of roughly 100% (i.e.
> > 200MB/s to 400MB/s). Phase 4 saw similar changes in IO profile and
> > speed increases.
> >
> > This, however, re-introduces the regression in xfs/033, which will
> > now be fixed in a separate patch.
>
> Thanks for finding this. I was getting close. ;)
>
> It seems fine, although a little unexpected; why do we ever
> create a map of 1? It feels a little odd to call getbuf_map
> with only 1 item, and then short-circuit it. Should this
> be something more obvious in the callers?
Because we have application code that is building buffer maps from
extent maps, and so having a API for both
contigous and discontiguous buffers makes sense. That's the way we
do it in the kernel - everything now uses the _map paths - but the
userspace code is a much larger surface area to change over and so
it's only being done in the places that matter first...
> Wel, I guess it's pretty much consistent w/ the same behavior
> in libxfs_readbuf_map()... *shrug*
Right. And like I said, it's also how the kernel does stuff, which
is why the libxfs_readbuf_map code functions like it does. ;)
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|