Re: Low RAID10 performance during resync

[Tomasz Majchrzak <tomasz.majchrzak@intel.com>]

On Fri, Jun 10, 2016 at 05:08:12PM +1000, NeilBrown wrote:
> On Fri, Jun 10 2016, Shaohua Li wrote:
> 
> > On Thu, Jun 09, 2016 at 03:45:55PM +0200, Tomasz Majchrzak wrote:
> >> A low performance of mkfs has been observed on RAID10 array during resync. It
> >> is not so significant for NVMe drives but for my setup of RAID10 consisting
> >> of 4 SATA drives format time has increased by 200%.
> >> 
> >> I have looked into the problem and I have found out it is caused by this
> >> changeset:
> >> 
> >> commit 09314799e4f0589e52bafcd0ca3556c60468bc0e md: remove 'go_faster' option
> >> from ->sync_request()
> >> 
> >> It seemed the code had been redundant and could be safely removed due to
> >> barriers mechanism but it proved otherwise. The barriers don't provide enough
> >> throttle to resync IOs. They only assure non-resync IOs and resync IOs are
> >> not being executed at the same time. In result resync IOs take around 25% of
> >> CPU time, mostly because there are many of them but only one at a time so a
> >> lot of CPU time is simply wasted waiting for a single IO to complete.
> >> 
> >> The removed sleep call in resync IO had allowed a lot of non-resync activity
> >> to be scheduled (nobody waiting for a barrier). Once sleep call had ended,
> >> resync IO had to wait longer to raise a barrier as all non-resync activity
> >> had to be completed first. It had nicely throttled a number of resync IOs in
> >> favour of non-resync activity. Since we lack it now, the performance has
> >> dropped badly.
> >> 
> >> I would like to revert the changeset. We don't have to put a resync IO to
> >> sleep for a second though. I have done some testing and it seems even a delay
> >> of 100ms is sufficient. It slows down resync IOs to the same extent as sleep
> >> for a second - the sleep call ends sooner but the barrier cannot be raised
> >> until non-resync IOs complete.
> >
> > Add Neil.
> >
> > I'd like to make sure I understand the situation. With the change reverted, we
> > dispatch a lot of normal IO and then do a resync IO. Without it reverted, we
> > dispatch few normal IO and then do a resync IO. In other words, we don't batch
> > normal IO currently. Is this what you say?
> >
> > Agree the barrier doesn't throttle resync IOs, it only assures normal IO and
> > resync IO run in different time.

Yes, precisely, resync is faster. The problem is performance drop from user
perspective is too big.

> 
> I think the barrier mechanism will mostly let large batches of IO
> through as a match.  If there is a pending request, a new request will
> always be let straight through.  Resync needs to wait for all pending
> regular IO to complete before it gets a turn.
> 
> So I would only expect that patch to cause problems when IO is very
> synchronous: write, wait, write, wait, etc.
> 
> I really didn't like the "go_faster" mechanism, but it might be OK to
> have something like
>   if (conf->nr_waiting)
>       schedule_timeout_uninterruptible(1);
> 
> so it will wait one jiffie if there is normal IO.  This would batch this
> a lot more.
> 
> It is very hard to know the exact consequences of this sort of change on
> all different configurations, and the other commit you mentioned shows.
> 
> I keep thinking there must be a better way, but I haven't found it yet
> :-(
> 
> NeilBrown
> 
> 
> >
> > On the other hand, the change makes resync faster. Did you try to revert this one:
> > ac8fa4196d205ac8fff3f8932bddbad4f16e4110
> > If resync is fast, reverting this one will throttle resync.
> >
> > Thanks,
> > Shaohua

I reverted it and it brought performance to the initial level. It's not a
solution though, isn't it?

I have incorrectly reported current performance drop. At the moment mkfs on my
setup takes around 20 minutes. Before the change it used to take 1 min 20 secs.

I have checked Neil's proposal (schedule_timeout_uninterruptible for 1 jiffies)
- it would bring formatting time to 2 mins 16 secs - so it's a valid solution to
the problem.

I have also tried other approach. Neil has mentioned that pending requests will
be let straight through if there are requests already in progress. Well, the
code looks so, however current->bio_list is empty most of the time, even though
the requests are being processed. I added an extra time window which allows
requests to proceed, even though there is a barrier awaiting already. It brings
mkfs performance to the initial level (1 min 20 secs).

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index e3fd725..51caf87 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -916,6 +916,8 @@ static void raise_barrier(struct r10conf *conf, int force)
 			    !conf->nr_pending && conf->barrier < RESYNC_DEPTH,
 			    conf->resync_lock);
 
+	conf->last_resync_time = jiffies;
+
 	spin_unlock_irq(&conf->resync_lock);
 }
 
@@ -945,8 +947,9 @@ static void wait_barrier(struct r10conf *conf)
 		wait_event_lock_irq(conf->wait_barrier,
 				    !conf->barrier ||
 				    (conf->nr_pending &&
-				     current->bio_list &&
-				     !bio_list_empty(current->bio_list)),
+				     ((current->bio_list &&
+				       !bio_list_empty(current->bio_list)) ||
+				      (jiffies - conf->last_resync_time) < HZ / 20)),
 				    conf->resync_lock);
 		conf->nr_waiting--;
 	}

Please tell me if you prefer Neil's or my solution.

Tomek