[PATCH] md - 2 of 10 - Don't allow raid5 rebuild to swamp raid5 stripe cache

[PATCH] md - 2 of 10 - Don't allow raid5 rebuild to swamp raid5 stripe cache

[NeilBrown]

raid5 rebuild takes stripes so agressively that other
access cannot get a look-in.
With this patch, the rebuild pauses slightly if there is a 
shortage of stripes to let other processes have a chance.

 ----------- Diffstat output ------------
 ./drivers/md/raid5.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletion(-)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2004-01-16 11:58:51.000000000 +1100
+++ ./drivers/md/raid5.c	2004-01-16 11:59:17.000000000 +1100
@@ -1395,7 +1395,14 @@ static int sync_request (mddev_t *mddev,
 
 	first_sector = raid5_compute_sector(stripe*data_disks*sectors_per_chunk
 		+ chunk_offset, raid_disks, data_disks, &dd_idx, &pd_idx, conf);
-	sh = get_active_stripe(conf, sector_nr, pd_idx, 0);
+	sh = get_active_stripe(conf, sector_nr, pd_idx, 1);
+	if (sh == NULL) {
+		sh = get_active_stripe(conf, sector_nr, pd_idx, 0);
+		/* make sure we don't swamp the stripe cache if someone else
+		 * is trying to get access 
+		 */
+		yield();
+	}
 	spin_lock(&sh->lock);	
 	set_bit(STRIPE_SYNCING, &sh->state);
 	clear_bit(STRIPE_INSYNC, &sh->state);

Re: [PATCH] md - 2 of 10 - Don't allow raid5 rebuild to swamp raid5 stripe cache

[Andrew Morton]

NeilBrown <neilb@cse.unsw.edu.au> wrote:
>
> +	if (sh == NULL) {
> +		sh = get_active_stripe(conf, sector_nr, pd_idx, 0);
> +		/* make sure we don't swamp the stripe cache if someone else
> +		 * is trying to get access 
> +		 */
> +		yield();
> +	}

In 2.6, yield() has utterly awful behaviour when there are a lot of other
runnable tasks.

Have you tested this when the kernel is busy doing other stuff, like a
`make -j2' or a few busywait loops?  You might find that rebuild simply
stops.

Re: [PATCH] md - 2 of 10 - Don't allow raid5 rebuild to swamp raid5 stripe cache

[Neil Brown]

On Thursday January 15, akpm@osdl.org wrote:
> NeilBrown <neilb@cse.unsw.edu.au> wrote:
> >
> > +	if (sh == NULL) {
> > +		sh = get_active_stripe(conf, sector_nr, pd_idx, 0);
> > +		/* make sure we don't swamp the stripe cache if someone else
> > +		 * is trying to get access 
> > +		 */
> > +		yield();
> > +	}
> 
> In 2.6, yield() has utterly awful behaviour when there are a lot of other
> runnable tasks.
> 
> Have you tested this when the kernel is busy doing other stuff, like a
> `make -j2' or a few busywait loops?  You might find that rebuild simply
> stops.
> 

No I haven't, and I'm not near my test box today, so it would be
awkward. 

No other patches are dependant on that one so you can just leave it
out if you like.

I just want to wait until every other runnable process has had a
turn.  Maybe cond_resched() would be better??  I try some things out.

NeilBrown

Re: [PATCH] md - 2 of 10 - Don't allow raid5 rebuild to swamp raid5 stripe cache

[Andrew Morton]

Neil Brown <neilb@cse.unsw.edu.au> wrote:
>
> On Thursday January 15, akpm@osdl.org wrote:
> > NeilBrown <neilb@cse.unsw.edu.au> wrote:
> > >
> > > +	if (sh == NULL) {
> > > +		sh = get_active_stripe(conf, sector_nr, pd_idx, 0);
> > > +		/* make sure we don't swamp the stripe cache if someone else
> > > +		 * is trying to get access 
> > > +		 */
> > > +		yield();
> > > +	}
> > 
> > In 2.6, yield() has utterly awful behaviour when there are a lot of other
> > runnable tasks.
> > 
> > Have you tested this when the kernel is busy doing other stuff, like a
> > `make -j2' or a few busywait loops?  You might find that rebuild simply
> > stops.
> > 
> 
> No I haven't, and I'm not near my test box today, so it would be
> awkward. 

OK.  We had a yeild like that in ext3 for a while.  The CPU scheduler
changes killed it stone dead.  I ended up just making it a schedule(),
which is too weak and probably broke the transaction batching speedup :(

> No other patches are dependant on that one so you can just leave it
> out if you like.

OK.  It'll take a few days after Linus gets back from .au to get all this
stuff flushed out anyway.   I'll probably have >700 patches by then.

> I just want to wait until every other runnable process has had a
> turn.  Maybe cond_resched() would be better??  I try some things out.

There isn't an equivalent, really.  We could add one to the CPU scheduler.

With HZ=1000 you'll probably find that a brief schedule_timeout() is OK. 
blk_congestion_wait() probably does the wrong thing too.