[PATCH] right meaning of PARITY_ENABLE_RMW and PARITY_PREFER_RMW

[PATCH] right meaning of PARITY_ENABLE_RMW and PARITY_PREFER_RMW

[Song Liu]

In current handle_stripe_dirtying, the code prefers rmw with
PARITY_ENABLE_RMW; while prefers rcw with PARITY_PREFER_RMW.

This patch reverses this behavior.

Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/md/raid5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8959e6d..ad9e15a 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3600,7 +3600,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
 	pr_debug("for sector %llu, rmw=%d rcw=%d\n",
 		(unsigned long long)sh->sector, rmw, rcw);
 	set_bit(STRIPE_HANDLE, &sh->state);
-	if ((rmw < rcw || (rmw == rcw && conf->rmw_level == PARITY_ENABLE_RMW)) && rmw > 0) {
+	if ((rmw < rcw || (rmw == rcw && conf->rmw_level == PARITY_PREFER_RMW)) && rmw > 0) {
 		/* prefer read-modify-write, but need to get some data */
 		if (conf->mddev->queue)
 			blk_add_trace_msg(conf->mddev->queue,
@@ -3627,7 +3627,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
 			}
 		}
 	}
-	if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_ENABLE_RMW)) && rcw > 0) {
+	if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_PREFER_RMW)) && rcw > 0) {
 		/* want reconstruct write, but need to get some data */
 		int qread =0;
 		rcw = 0;
-- 
2.8.0.rc2

Re: [PATCH] right meaning of PARITY_ENABLE_RMW and PARITY_PREFER_RMW

[Shaohua Li]

On Mon, May 23, 2016 at 05:25:06PM -0700, Song Liu wrote:
> In current handle_stripe_dirtying, the code prefers rmw with
> PARITY_ENABLE_RMW; while prefers rcw with PARITY_PREFER_RMW.
> 
> This patch reverses this behavior.
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/md/raid5.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8959e6d..ad9e15a 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3600,7 +3600,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
>  	pr_debug("for sector %llu, rmw=%d rcw=%d\n",
>  		(unsigned long long)sh->sector, rmw, rcw);
>  	set_bit(STRIPE_HANDLE, &sh->state);
> -	if ((rmw < rcw || (rmw == rcw && conf->rmw_level == PARITY_ENABLE_RMW)) && rmw > 0) {
> +	if ((rmw < rcw || (rmw == rcw && conf->rmw_level == PARITY_PREFER_RMW)) && rmw > 0) {
>  		/* prefer read-modify-write, but need to get some data */
>  		if (conf->mddev->queue)
>  			blk_add_trace_msg(conf->mddev->queue,
> @@ -3627,7 +3627,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
>  			}
>  		}
>  	}
> -	if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_ENABLE_RMW)) && rcw > 0) {
> +	if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_PREFER_RMW)) && rcw > 0) {
>  		/* want reconstruct write, but need to get some data */
>  		int qread =0;
>  		rcw = 0;

The patch looks correct, I'll apply.

I'm wondering why original code is in current way. It dosn't work like what the
patch log (d06f191f8ecae) describes. Neil/Markus, is any patch missed?

Thanks,
Shaohua

AW: [PATCH] right meaning of PARITY_ENABLE_RMW and PARITY_PREFER_RMW

[Markus Stockhausen]

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

> Von: linux-raid-owner@vger.kernel.org [linux-raid-owner@vger.kernel.org]" im Auftrag von "Shaohua Li [shli@kernel.org]
> Gesendet: Mittwoch, 25. Mai 2016 18:02
> An: Song Liu
> Cc: linux-raid@vger.kernel.org; Markus Stockhausen; neilb@suse.de
> Betreff: Re: [PATCH] right meaning of PARITY_ENABLE_RMW and PARITY_PREFER_RMW
> 
> On Mon, May 23, 2016 at 05:25:06PM -0700, Song Liu wrote:
> > In current handle_stripe_dirtying, the code prefers rmw with
> > PARITY_ENABLE_RMW; while prefers rcw with PARITY_PREFER_RMW.
> >
> > This patch reverses this behavior.
> >
> > Signed-off-by: Song Liu <songliubraving@fb.com>
> > Signed-off-by: Shaohua Li <shli@fb.com>
> > ---
> >  drivers/md/raid5.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 8959e6d..ad9e15a 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -3600,7 +3600,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
> >       pr_debug("for sector %llu, rmw=%d rcw=%d\n",
> >               (unsigned long long)sh->sector, rmw, rcw);
> >       set_bit(STRIPE_HANDLE, &sh->state);
> > -     if ((rmw < rcw || (rmw == rcw && conf->rmw_level == PARITY_ENABLE_RMW)) && rmw > 0) {
> > +     if ((rmw < rcw || (rmw == rcw && conf->rmw_level == PARITY_PREFER_RMW)) && rmw > 0) {
> >               /* prefer read-modify-write, but need to get some data */
> >               if (conf->mddev->queue)
> >                       blk_add_trace_msg(conf->mddev->queue,
> > @@ -3627,7 +3627,7 @@ static void handle_stripe_dirtying(struct r5conf *conf,
> >                       }
> >               }
> >       }
> > -     if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_ENABLE_RMW)) && rcw > 0) {
> > +     if ((rcw < rmw || (rcw == rmw && conf->rmw_level != PARITY_PREFER_RMW)) && rcw > 0) {
> >               /* want reconstruct write, but need to get some data */
> >               int qread =0;
> >               rcw = 0;
> > 
> > The patch looks correct, I'll apply.
> 
> I'm wondering why original code is in current way. It dosn't work like what the
> patch log (d06f191f8ecae) describes. Neil/Markus, is any patch missed?

Hi,

simple reason for usage of PARITY_ENABLE_RMW was the patch order. Option
PARITY_PREFER_RMW was implemented after all the rest. I guess I missed 
changing that condition.

Thanks for the fix.

Markus=

[-- Attachment #2: InterScan_Disclaimer.txt --]
[-- Type: text/plain, Size: 1650 bytes --]

****************************************************************************
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte
Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail
irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und
vernichten Sie diese Mail. Das unerlaubte Kopieren sowie die unbefugte
Weitergabe dieser Mail ist nicht gestattet.

Über das Internet versandte E-Mails können unter fremden Namen erstellt oder
manipuliert werden. Deshalb ist diese als E-Mail verschickte Nachricht keine
rechtsverbindliche Willenserklärung.

Collogia
Unternehmensberatung AG
Ubierring 11
D-50678 Köln

Vorstand:
Kadir Akin
Dr. Michael Höhnerbach

Vorsitzender des Aufsichtsrates:
Hans Kristian Langva

Registergericht: Amtsgericht Köln
Registernummer: HRB 52 497

This e-mail may contain confidential and/or privileged information. If you
are not the intended recipient (or have received this e-mail in error)
please notify the sender immediately and destroy this e-mail. Any
unauthorized copying, disclosure or distribution of the material in this
e-mail is strictly forbidden.

e-mails sent over the internet may have been written under a wrong name or
been manipulated. That is why this message sent as an e-mail is not a
legally binding declaration of intention.

Collogia
Unternehmensberatung AG
Ubierring 11
D-50678 Köln

executive board:
Kadir Akin
Dr. Michael Höhnerbach

President of the supervisory board:
Hans Kristian Langva

Registry office: district court Cologne
Register number: HRB 52 497

****************************************************************************