[Btrfs-devel] [PATCH] delay commit if work is done while we're asleep

[Josef Bacik]

On Thu, Aug 09, 2007 at 11:44:17AM -0400, Chris Mason wrote:
> On Thu, 9 Aug 2007 11:34:14 -0400
> Josef Bacik <jwhiter at redhat.com> wrote:
> 
> > On Thu, Aug 09, 2007 at 11:20:51AM -0400, Chris Mason wrote:
> > > On Thu, 9 Aug 2007 11:07:39 -0400
> > > Josef Bacik <jwhiter at redhat.com> wrote:
> > > 
> > > > Hello,
> > > > 
> > > > Ok this one works I promise :).  Cleaned a few things up and such,
> > > > but basically same idea as before, I keep the waitqueue, I use
> > > > schedule_timeout which keeps us from deadlocking.  This speeds us
> > > > up a decent amount, without the patch I usually get 70-75
> > > > files/sec. With this patch I get between 85-115 files/sec. The
> > > > reason there is such a wide spread depends mostly on when the
> > > > transaction cleaner runs while I'm running the test, as this is
> > > > only single threaded. Feedback is appreciated.  Thank you,
> > > 
> > > It looks great to me, thanks for working on this.  One change I'll
> > > probably make here is to do something like this:
> > > 
> > > prepare_to_wait
> > > if (trans->transaction->num_writers > 1)
> > >     timeout = MAX_SCHEDULE_TIMEOUT
> > > else
> > >     timeout = 1
> > > drop locks
> > > schedule_timeout(timeout);
> > > 
> > > This way we don't loop in schedule when there are writers around who
> > > will wake us.  I'll give it a try this afternoon.
> >  
> > 
> > Sounds good, but one quick comment, I just hit a panic doing a
> > multithreaded test.  Fortunately its wasn't introduced by my code,
> > just made it easier to hit. Here is the patch that fixes this
> > problem, tested it out with and without my patch with no issues.
> 
> Ack, looks correct to me.
> 
> >  Do
> > you want me to make the above changes?  Or are you going to do it?
> 
> I welcome all patches ;)  Didn't want to ask you for another rewrite
> for a small change, but I'd actually love to see numbers for how much
> it helps/hurts your runs.  I'm starting a long test here of unrelated
> changes, so I won't be testing this out until later today.
> 
> BTW, which revisions are you testing against?
> 

Ok I'll rework the patch and re-run my tests and include my numbers next time
around.  What do you mean which revisions?  I pulled down what was in mercurial
this morning and ran tests against that, then did my patch and re-ran my tests,
if thats what you are asking.  Thanks much,

Josef