[PATCH 0/3] md raid1: Fix writebehind/writemostly

[PATCH 0/3] md raid1: Fix writebehind/writemostly

[heinzm]

From: heinzm <heinzm@redhat.com>

Writemostly was only respected if set on the first leg.

Set on any other leg(s) but not on the first one did
not enable writebehind at all.

Fix changes the logic using the already defind bool writebehind.

Whilst on this, also make first_clone a bool as write_behind for
consistency and add an empty line.

Patches pass the MD test suite.

heinzm (3):
  md raid1: allow writebehind to work on any leg device set WriteMostly
  md raid1: make first_clone a bool
  md raid1: add empty line

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Tested-by: Xia Ni <xni@redhat.com>

 drivers/md/raid1.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.41.0

[PATCH 1/3] md raid1: allow writebehind to work on any leg device set WriteMostly

[heinzm]

From: heinzm <heinzm@redhat.com>

As the WriteMostly flag can be set on any component device of a RAID1 array,
remove the constraint that it only works if set on the first one.

Signed-off-by: heinzm <heinzm@redhat.com>
---
 drivers/md/raid1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index dd25832eb045..913cd46b786b 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1519,7 +1519,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 			 * allocate memory, or a reader on WriteMostly
 			 * is waiting for behind writes to flush */
 			if (bitmap &&
-			    test_bit(WriteMostly, &rdev->flags) &&
+			    write_behind &&
 			    (atomic_read(&bitmap->behind_writes)
 			     < mddev->bitmap_info.max_write_behind) &&
 			    !waitqueue_active(&bitmap->behind_wait)) {
-- 
2.41.0

Re: [PATCH 1/3] md raid1: allow writebehind to work on any leg device set WriteMostly

[Yu Kuai]

Hi,

在 2023/08/10 20:11, heinzm@redhat.com 写道:
> From: heinzm <heinzm@redhat.com>
> 
> As the WriteMostly flag can be set on any component device of a RAID1 array,
> remove the constraint that it only works if set on the first one.
> 
> Signed-off-by: heinzm <heinzm@redhat.com>
> ---
>   drivers/md/raid1.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index dd25832eb045..913cd46b786b 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1519,7 +1519,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
>   			 * allocate memory, or a reader on WriteMostly
>   			 * is waiting for behind writes to flush */
>   			if (bitmap &&
> -			    test_bit(WriteMostly, &rdev->flags) &&
> +			    write_behind &&

No need for a new line now.

Reviewed-by: Yu Kuai <yukuai3@huawei.com>

Thanks
>   			    (atomic_read(&bitmap->behind_writes)
>   			     < mddev->bitmap_info.max_write_behind) &&
>   			    !waitqueue_active(&bitmap->behind_wait)) {
> 

[PATCH 2/3] md raid1: make first_clone a bool

[heinzm]

From: heinzm <heinzm@redhat.com>

first_clone is used as a bool so make it one.

Signed-off-by: heinzm <heinzm@redhat.com>
---
 drivers/md/raid1.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 913cd46b786b..0aed74efd758 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1337,9 +1337,8 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 	struct bitmap *bitmap = mddev->bitmap;
 	unsigned long flags;
 	struct md_rdev *blocked_rdev;
-	int first_clone;
 	int max_sectors;
-	bool write_behind = false;
+	bool first_clone, write_behind = false;
 
 	if (mddev_is_clustered(mddev) &&
 	     md_cluster_ops->area_resyncing(mddev, WRITE,
@@ -1505,7 +1504,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 	atomic_set(&r1_bio->remaining, 1);
 	atomic_set(&r1_bio->behind_remaining, 0);
 
-	first_clone = 1;
+	first_clone = true;
 
 	for (i = 0; i < disks; i++) {
 		struct bio *mbio = NULL;
@@ -1528,7 +1527,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 
 			md_bitmap_startwrite(bitmap, r1_bio->sector, r1_bio->sectors,
 					     test_bit(R1BIO_BehindIO, &r1_bio->state));
-			first_clone = 0;
+			first_clone = false;
 		}
 
 		if (r1_bio->behind_master_bio) {
-- 
2.41.0

[PATCH 3/3] md raid1: add empty line

[heinzm]

From: heinzm <heinzm@redhat.com>

Signed-off-by: heinzm <heinzm@redhat.com>
---
 drivers/md/raid1.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 0aed74efd758..5c469ce843c8 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1509,6 +1509,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
 	for (i = 0; i < disks; i++) {
 		struct bio *mbio = NULL;
 		struct md_rdev *rdev = conf->mirrors[i].rdev;
+
 		if (!r1_bio->bios[i])
 			continue;
 
-- 
2.41.0

Re: [PATCH 0/3] md raid1: Fix writebehind/writemostly

[Song Liu]

On Thu, Aug 10, 2023 at 4:12 PM <heinzm@redhat.com> wrote:
>
> From: heinzm <heinzm@redhat.com>
>
> Writemostly was only respected if set on the first leg.
>
> Set on any other leg(s) but not on the first one did
> not enable writebehind at all.
>
> Fix changes the logic using the already defind bool writebehind.
>
> Whilst on this, also make first_clone a bool as write_behind for
> consistency and add an empty line.
>
> Patches pass the MD test suite.
>
> heinzm (3):
>   md raid1: allow writebehind to work on any leg device set WriteMostly

1/3 looks good. But please resend with your full name.

>   md raid1: make first_clone a bool
>   md raid1: add empty line

2/3 and 3/3 are not necessary. I would rather keep git-blame clean.

Thanks,
Song
>
> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
> Tested-by: Xia Ni <xni@redhat.com>
>
>  drivers/md/raid1.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> --
> 2.41.0
>