Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug()

[Chris Mason <clm@fb.com>]

On Fri, Sep 11, 2015 at 02:04:13PM -0700, Linus Torvalds wrote:
> On Fri, Sep 11, 2015 at 1:40 PM, Josef Bacik <jbacik@fb.com> wrote:
> >
> > So we talked about this when we were trying to figure out a solution. The
> > problem with this approach is now we have a plug that covers multiple super
> > blocks (__writeback_inodes_wb loops through the sb's starts writeback),
> > which is likely to give us crappier performance than no plug at all.
> 
> Why would that be? Either they are on separate disks, and the IO is
> all independent anyway, and at most it got started by some (small)
> CPU-amount later. Actual throughput should be the same. No?
> 
> Or the filesystems are on the same disk, in which case it should
> presumably be a win to submit the IO together.
> 
> Of course, actual numbers would be the deciding factor if this really
> is noticeable. But "cleaner code and saner locking" is definitely an
> issue at least for me.

Originally I was worried about the latency impact of holding the
plugs over more than one super with high end flash.  I just didn't want
to hold onto the IO for longer than we had to.

But, since this isn't really latency sensitive anyway, if we find we're
not keeping the flash pipelines full the right answer is to short
circuit the plugging in general.  I'd agree actual throughput should be
the same.

But benchmarking is the best choice, I'll be able to reproduce
Dave's original results without too much trouble.  Our thinking for this
duct tape patch was the lock wasn't very hot and this was the best
immediate compromise between the bug and the perf improvement.

Happy to sign up for pushing the lock higher if that's what people would
rather see.

-chris