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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
