Re: [PATCH] md/raid5: Add compensation in sysfs for raid456.

[NeilBrown <neilb@suse.de>]

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

On Tue, 14 Jan 2014 13:40:20 +0800 majianpeng <majianpeng@gmail.com> wrote:

> For non-fullstripe-write, raid456 need read some old data no matter how
> rcw or rmw. We use compensation to recored the value about the
> additional read-data.The unit of compenstaion is PAGE_SIZE.
> Using the formula compensation*PAGE_SIZE/write_count_for_md, we can know
> the efficiency.Using this we can tune the write mode to get the best
> efficiency.
> 
> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
> ---
>  drivers/md/raid5.c |   16 ++++++++++++++++
>  drivers/md/raid5.h |    5 +++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index cc055da..e456134 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3116,6 +3116,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
>  						 "%d for r-m-w\n", i);
>  					set_bit(R5_LOCKED, &dev->flags);
>  					set_bit(R5_Wantread, &dev->flags);
> +					atomic_inc(&conf->compensation);
>  					s->locked++;
>  				} else {
>  					set_bit(STRIPE_DELAYED, &sh->state);
> @@ -3144,6 +3145,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
>  						"%d for Reconstruct\n", i);
>  					set_bit(R5_LOCKED, &dev->flags);
>  					set_bit(R5_Wantread, &dev->flags);
> +					atomic_inc(&conf->compensation);
>  					s->locked++;
>  					qread++;
>  				} else {
> @@ -5426,12 +5428,25 @@ static struct md_sysfs_entry
>  raid5_group_thread_cnt = __ATTR(group_thread_cnt, S_IRUGO | S_IWUSR,
>  				raid5_show_group_thread_cnt,
>  				raid5_store_group_thread_cnt);
> +static ssize_t
> +compensation_show(struct mddev *mddev, char *page)
> +{
> +	struct r5conf *conf = mddev->private;
> +	if (conf)
> +		return sprintf(page, "%d\n", atomic_read(&conf->compensation));
> +	else
> +		return 0;
> +}
> +
> +static struct md_sysfs_entry
> +raid5_compensation = __ATTR_RO(compensation);
>  
>  static struct attribute *raid5_attrs[] =  {
>  	&raid5_stripecache_size.attr,
>  	&raid5_stripecache_active.attr,
>  	&raid5_preread_bypass_threshold.attr,
>  	&raid5_group_thread_cnt.attr,
> +	&raid5_compensation.attr,
>  	NULL,
>  };
>  static struct attribute_group raid5_attrs_group = {
> @@ -5686,6 +5701,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
>  	atomic_set(&conf->active_stripes, 0);
>  	atomic_set(&conf->preread_active_stripes, 0);
>  	atomic_set(&conf->active_aligned_reads, 0);
> +	atomic_set(&conf->compensation, 0);
>  	conf->bypass_threshold = BYPASS_THRESHOLD;
>  	conf->recovery_disabled = mddev->recovery_disabled - 1;
>  
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index 01ad8ae..8fa538d 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
> @@ -494,6 +494,11 @@ struct r5conf {
>  	struct r5worker_group	*worker_groups;
>  	int			group_cnt;
>  	int			worker_cnt_per_group;
> +
> +	/*For non-fullstripe-write, it need read some data
> +	 * using this filed to acount the additional data.
> +	 * The unit of compensation is PAGE_SIZE*/
> +	atomic_t		compensation;
>  };
>  
>  /*

I have no idea what this patch is supposed to provide.  It seems to count
something.  What use is that?

If you want to be taken seriously, you need to explain clearly what benefit
the brings.  The comments you did provide say something about tuning.  Maybe
some step-by-step instructions on how you would use this new functionality to
tune something, and some results indication how something performed better
after the tuning.

NeilBrown

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