Re: [RFC 1/2]raid1: only write mismatch sectors in sync

[NeilBrown <neilb@suse.de>]

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

On Thu, 26 Jul 2012 16:01:50 +0800 Shaohua Li <shli@kernel.org> wrote:

> Write has some impacts to SSD:
> 1. wear out flash. Frequent write can speed out the progress.
> 2. increase the burden of garbage collection of SSD firmware. If no space
> left for write, SSD firmware garbage collection will try to free some space.
> 3. slow down subsequent write. After write SSD to some extents (for example,
> write the whole disk), subsequent write will slow down significantly (because
> almost every write invokes garbage collection in such case).
> 
> We want to avoid unnecessary write as more as possible. raid sync generally
> involves a lot of unnecessary write. For example, even two disks don't have
> any data, we write the second disk for the whole disk size.
> 
> To reduce write, we always compare raid disk data and only write mismatch part.
> This means sync will have extra IO read and memory compare. So this scheme is
> very bad for hard disk raid and sometimes SSD raid too if mismatch part is
> majority. But sometimes this can be very helpful to reduce write, in that case,
> since sync is rare operation, the extra IO/CPU usage is worthy paying. People
> who want to use the feature should understand the risk first. So this ability
> is off by default, a sysfs entry can be used to enable it.
> 
> Signed-off-by: Shaohua Li <shli@fusionio.com>
> ---
>  drivers/md/md.c    |   41 +++++++++++++++++++++++++++++++
>  drivers/md/md.h    |    3 ++
>  drivers/md/raid1.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++----
>  3 files changed, 110 insertions(+), 4 deletions(-)
> 
> Index: linux/drivers/md/md.h
> ===================================================================
> --- linux.orig/drivers/md/md.h	2012-07-25 13:51:00.353775521 +0800
> +++ linux/drivers/md/md.h	2012-07-26 10:36:38.500740552 +0800
> @@ -325,6 +325,9 @@ struct mddev {
>  #define	MD_RECOVERY_FROZEN	9
>  
>  	unsigned long			recovery;
> +#define MD_RECOVERY_MODE_REPAIR		0
> +#define MD_RECOVERY_MODE_DISCARD	1
> +	unsigned long			recovery_mode;

You have not documented the meaning of these two flags at all, and I don't
feel up to guessing.

The patch looks more complex that it should be.  The behaviour you are
suggesting is exactly the behaviour you get when MD_RECOVERY_REQUESTED is
set, so at most I expect to see a few places where that flag is tested
changed to test something else as well.

How would this be used?  It affects resync and resync happens as soon as the
array is assembled.  So when and how would you set the flag which says
"prefer reads to writes"?  It seems like it needs to be set in the metadata.

BTW RAID10 already does this - it reads and compares for a normal sync.  So
maybe just tell people to use raid10 if they want this behaviour?

NeilBrown

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