Re: [patch v3 1/5] raid5: make release_stripe lockless

[Shaohua Li <shli@kernel.org>]

On Wed, Aug 28, 2013 at 10:04:22AM -0400, Tejun Heo wrote:
> Hello, Shaohua.
> 
> On Tue, Aug 27, 2013 at 05:50:39PM +0800, Shaohua Li wrote:
> > release_stripe still has big lock contention. We just add the stripe to a llist
> > without taking device_lock. We let the raid5d thread to do the real stripe
> > release, which must hold device_lock anyway. In this way, release_stripe
> > doesn't hold any locks.
> > 
> > The side effect is the released stripes order is changed. But sounds not a big
> > deal, stripes are never handled in order. And I thought block layer can already
> > do nice request merge, which means order isn't that important.
> 
> I wrote this before but the order of requests is an important
> information to the elevator and the existing elevators will behave
> less effectively if you make the relative order and timing of
> processed IOs deviate from the original issuer's and the effect could
> be very noticeable depending on the workload and stacking drivers
> should always strive to preserve as much IO characteristics.
> 
> It doesn't make the changes unacceptable or anything but the patch
> description is quite misleading.  It'd be nice if you at least can
> note that the implemented behavior is far from optimal in the comment
> and description.

This order issue is fixed in the second patch.

It's true making raid5 multi-threading might change order, I mentioned this in
the third patch (the direct impact is request size)

Thanks,
Shaohua