Re: [patch 7/8] raid5: raid5d handle stripe in batch way

[NeilBrown <neilb@xxxxxxx>]

On Mon, 04 Jun 2012 16:01:59 +0800 Shaohua Li <shli@xxxxxxxxxx> wrote:

> Let raid5d handle stripe in batch way to reduce conf->device_lock locking.
> 
> Signed-off-by: Shaohua Li <shli@xxxxxxxxxxxx>

I like this.
I don't think it justifies a separate function.

#define MAX_STRIPE_BATCH 8
struct stripe_head *batch[MAX_STRIPE_BATCH]
int batch_size = 0;

...

while (batch_size < MAX_STRPE_BATCH &&
       (sh = __get_priority_stripe(conf)) != NULL)
     batch[batch_size++] = sh;

spin_unlock(&conf->device_lock);
if (batch_size == 0)
     break;

handled += batch_size;

for (i = 0; i < batch_size; i++)
     handle_stripe(batch[i]);
cond_resched();
if (....) md_check_recovery(mddev);

spin_lock_irq(&conf->lock);
for (i = 0; i < batch_size; i++)
     __release_stripe(batch[i]);


something like that?

Thanks,
NeilBrown



> ---
>  drivers/md/raid5.c |   35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> Index: linux/drivers/md/raid5.c
> ===================================================================
> --- linux.orig/drivers/md/raid5.c	2012-06-01 14:34:03.987606911 +0800
> +++ linux/drivers/md/raid5.c	2012-06-01 14:49:26.388010973 +0800
> @@ -4585,6 +4585,22 @@ static int  retry_aligned_read(struct r5
>  	return handled;
>  }
>  
> +static int __get_stripe_batch(struct r5conf *conf,
> +		struct stripe_head_batch *batch)
> +{
> +	struct stripe_head *sh;
> +
> +	batch->count = 0;
> +	do {
> +		sh = __get_priority_stripe(conf);
> +		if (sh) {
> +			batch->stripes[batch->count] = sh;
> +			batch->count++;
> +		}
> +	} while (sh && batch->count < MAX_STRIPE_BATCH);
> +
> +	return batch->count;
> +}
>  
>  /*
>   * This is our raid5 kernel thread.
> @@ -4595,10 +4611,10 @@ static int  retry_aligned_read(struct r5
>   */
>  static void raid5d(struct mddev *mddev)
>  {
> -	struct stripe_head *sh;
>  	struct r5conf *conf = mddev->private;
> -	int handled;
> +	int handled, i;
>  	struct blk_plug plug;
> +	struct stripe_head_batch batch;
>  
>  	pr_debug("+++ raid5d active\n");
>  
> @@ -4633,15 +4649,16 @@ static void raid5d(struct mddev *mddev)
>  			handled++;
>  		}
>  
> -		sh = __get_priority_stripe(conf);
> -
> -		if (!sh)
> +		if (!__get_stripe_batch(conf, &batch))
>  			break;
>  		spin_unlock_irq(&conf->device_lock);
> -		
> -		handled++;
> -		handle_stripe(sh);
> -		release_stripe(sh);
> +
> +		for (i = 0; i < batch.count; i++) {
> +			handled++;
> +			handle_stripe(batch.stripes[i]);
> +		}
> +
> +		release_stripe_flush_batch(&batch);
>  		cond_resched();
>  
>  		if (mddev->flags & ~(1<<MD_CHANGE_PENDING))

Attachment: signature.asc
Description: PGP signature