Re: [PATCH/RFC/RFT] md: allow resync to go faster when there is competing IO.

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 4317 bytes --]

On Wed, Jan 27 2016, Shaohua Li wrote:

> On Wed, Jan 27, 2016 at 10:08:45AM +1100, Neil Brown wrote:
>> On Wed, Jan 27 2016, Shaohua Li wrote:
>> 
>> > On Wed, Jan 27, 2016 at 09:12:23AM +1100, Neil Brown wrote:
>> >> On Tue, Jan 26 2016, Chien Lee wrote:
>> >> 
>> >> > Hello,
>> >> >
>> >> > Recently we find a bug about this patch (commit No. is
>> >> > ac8fa4196d205ac8fff3f8932bddbad4f16e4110 ).
>> >> >
>> >> > We know that this patch committed after Linux kernel 4.1.x is intended
>> >> > to allowing resync to go faster when there is competing IO. However,
>> >> > we find the performance of random read on syncing Raid6 will come up
>> >> > with a huge drop in this case. The following is our testing detail.
>> >> >
>> >> > The OS what we choose in our test is CentOS Linux release 7.1.1503
>> >> > (Core) and the kernel image will be replaced for testing. In our
>> >> > testing result, the 4K random read performance on syncing raid6 in
>> >> > Kernel 4.2.8 is much lower than in Kernel 3.19.8. In order to find out
>> >> > the root cause, we try to rollback this patch in Kernel 4.2.8, and we
>> >> > find the 4K random read performance on syncing Raid6 will be improved
>> >> > and go back to as what it should be in Kernel 3.19.8.
>> >> >
>> >> > Nevertheless, it seems that it will not affect some other read/write
>> >> > patterns. In our testing result, the 1M sequential read/write, 4K
>> >> > random write performance in Kernel 4.2.8 is performed almost the same
>> >> > as in Kernel 3.19.8.
>> >> >
>> >> > It seems that although this patch increases the resync speed, the
>> >> > logic of !is_mddev_idle() cause the sync request wait too short and
>> >> > reduce the chance for raid5d to handle the random read I/O.
>> >> 
>> >> This has been raised before.
>> >> Can you please try the patch at the end of 
>> >> 
>> >>   http://permalink.gmane.org/gmane.linux.raid/51002
>> >> 
>> >> and let me know if it makes any difference.  If it isn't sufficient I
>> >> will explore further.
>> >
>> > I'm curious why we don't calculate the wait time. Say the target resync speed
>> > is speed_min. The wait time should be:
>> >
>> > (currspeed * SYNC_MARK_STEP - speed_min * SYNC_MARK_STEP) / speed_min
>> > = (currspeed / speed_min - 1) * SYNC_MARK_STEP
>> >
>> > if SYNC_MARK_STEP is too big and sync speed has drift, we can make it smaller.
>> 
>> What do you hope this would achieve?
>
> The whole point is to throttle sync speed to specific speed. If we know the
> target speed, for any given time interval, we can calculate the sync
> IO size.

Actually, no.  The main point is to not interfere with filesystem IO too
much.  Limiting to a low target is a fairly poor way to do that (but is
all we have) and as there is such a wide range of device speeds it is no
longer possible to choose a sensible default.

>  
>> If I understand correctly, this might allow the thread to sleep for
>> longer instead of looping around every 500ms or so.  But we don't really
>> want to do that.  As soon as filesystem IO pauses, we want resync IO to
>> go back to full speed.
>> 
>> The "speed_min" isn't really a "target".  It is only a "target" for
>> those times when there is no filesystem IO.
>
> Yep, target is a little bit hard to determine. I think we can do:
> if (curspeed > min) {
> 	if (!is_mddev_idle())
> 		targetspeed = minspeed;
> 	if (curspeed > max)
> 		targetspeed = maxspeed;
> 	sleep(max((currspeed / targetspeed - 1), 0) * SYNC_MARK_STEP)
> }
>
> This way we don't throttle if there is no filesystem IO. would this
> work?

But if there is filesystem IO, then we throttle for at least 3 seconds
(SYNC_MARK_STEP is 3*HZ).  The filesystem could go idle in 1 second and
then we would spend 2 seconds pointlessly doing nothing.  We could make
SYNC_MARK_STEP smaller, but if recent speed had been high we could still
throttle for a lot longer than needed.

Before the patch that caused the regression we would throttle for 500ms,
so if the filesystem went idle we would waste at most 500ms.
After the patch, we throttle until pending requests have completed.
This causes the delay to scale with the speed of the device, but doesn't
seem to be enough of a delay in some cases.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]