[PATCH v2 0/2] md/raid1: serialize overlapped ios for writemostly device

[PATCH v2 0/2] md/raid1: serialize overlapped ios for writemostly device

[Xiao Ni]

Xiao Ni (2):
  md/raid1: fix the comparing region of interval tree
  md/raid1: serialize overlap io for writemostly disk

 drivers/md/raid1.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/2] md/raid1: fix the comparing region of interval tree

[Xiao Ni]

Interval tree uses [start, end] as a region which stores in the tree.
In raid1, it uses the wrong end value. For example:
bio(A,B) is too big and needs to be split to bio1(A,C-1), bio2(C,B).
The region of bio1 is [A,C] and the region of bio2 is [C,B]. So bio1 and
bio2 overlap which is not right.

Fix this problem by using right end value of the region.

Fixes: d0d2d8ba0494 ("md/raid1: introduce wait_for_serialization")
Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/raid1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 181400e147c0..be2565dee420 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -62,7 +62,7 @@ static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
 	unsigned long flags;
 	int ret = 0;
 	sector_t lo = r1_bio->sector;
-	sector_t hi = lo + r1_bio->sectors;
+	sector_t hi = lo + r1_bio->sectors - 1;
 	struct serial_in_rdev *serial = &rdev->serial[idx];
 
 	spin_lock_irqsave(&serial->serial_lock, flags);
@@ -452,7 +452,7 @@ static void raid1_end_write_request(struct bio *bio)
 	int mirror = find_bio_disk(r1_bio, bio);
 	struct md_rdev *rdev = conf->mirrors[mirror].rdev;
 	sector_t lo = r1_bio->sector;
-	sector_t hi = r1_bio->sector + r1_bio->sectors;
+	sector_t hi = r1_bio->sector + r1_bio->sectors - 1;
 	bool ignore_error = !raid1_should_handle_error(bio) ||
 		(bio->bi_status && bio_op(bio) == REQ_OP_DISCARD);
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 1/2] md/raid1: fix the comparing region of interval tree

[Yu Kuai]

在 2026/3/5 9:18, Xiao Ni 写道:

> Interval tree uses [start, end] as a region which stores in the tree.
> In raid1, it uses the wrong end value. For example:
> bio(A,B) is too big and needs to be split to bio1(A,C-1), bio2(C,B).
> The region of bio1 is [A,C] and the region of bio2 is [C,B]. So bio1 and
> bio2 overlap which is not right.
>
> Fix this problem by using right end value of the region.
>
> Fixes: d0d2d8ba0494 ("md/raid1: introduce wait_for_serialization")
> Signed-off-by: Xiao Ni<xni@redhat.com>
> ---
>   drivers/md/raid1.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

patch 1 applied.

-- 
Thansk,
Kuai

[PATCH v2 2/2] md/raid1: serialize overlap io for writemostly disk

[Xiao Ni]

In behind mode, overlap bios for writemostly device are queued.
Those overlapped bios need to wait in waitqueue. They will be woken
up once the in-tree bio finishes.

Previously, using wait_event() would wake up all waiters simultaneously,
and they would compete for the tree lock. The bio which gets the lock
first will be handled, so the write sequence cannot be guaranteed.

For example:
bio1(100,200)
bio2(150,200)
bio3(150,300)

The write sequence of fast device is bio1,bio2,bio3. But the write
sequence of slow device could be bio1,bio3,bio2 due to lock competition.
This causes data corruption.

Use prepare_to_wait_exclusive() to ensure FIFO ordering. This adds
waiters to the tail of the waitqueue and wakes them from the head,
guaranteeing that bios are processed in the order they arrived.

Fixes: 3e148a320979 ("md/raid1: fix potential data inconsistency issue with write behind device")
Reviewed-by: Yu Kuai <yukuai@fnnas.com>
Signed-off-by: Xiao Ni <xni@redhat.com>
---
v2: use prepare_to_wait_exclusive
 drivers/md/raid1.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index be2565dee420..a95085fb3015 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -85,12 +85,20 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
 	struct serial_info *si;
 	int idx = sector_to_idx(r1_bio->sector);
 	struct serial_in_rdev *serial = &rdev->serial[idx];
+	DEFINE_WAIT(wait);
 
 	if (WARN_ON(!mddev->serial_info_pool))
 		return;
 	si = mempool_alloc(mddev->serial_info_pool, GFP_NOIO);
-	wait_event(serial->serial_io_wait,
-		   check_and_add_serial(rdev, r1_bio, si, idx) == 0);
+
+	for (;;) {
+		prepare_to_wait_exclusive(&serial->serial_io_wait, &wait,
+					  TASK_UNINTERRUPTIBLE);
+		if (check_and_add_serial(rdev, r1_bio, si, idx) == 0)
+			break;
+		schedule();
+	}
+	finish_wait(&serial->serial_io_wait, &wait);
 }
 
 static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
-- 
2.50.1 (Apple Git-155)

[PATCH 0/2] md/raid1: serialize overlapped ios for writemostly device

[Xiao Ni]

Xiao Ni (2):
  md/raid1: fix the comparing region of interval tree
  md/raid1: serialize overlap io for writemostly disk

 drivers/md/md.c    |  1 -
 drivers/md/md.h    |  4 +++-
 drivers/md/raid1.c | 43 +++++++++++++++++++++++++++++++------------
 3 files changed, 34 insertions(+), 14 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/2] md/raid1: fix the comparing region of interval tree

[Xiao Ni]

Interval tree uses [start, end] as a region which stores in the tree.
In raid1, it uses the wrong end value. For example:
bio(A,B) is too big and needs to be split to bio1(A,C-1), bio2(C,B).
The region of bio1 is [A,C] and the region of bio2 is [C,B]. So bio1 and
bio2 overlap which is not right.

Fix this problem by using right end value of the region.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/raid1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 867db18bc3ba..a41b1ec3d695 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -62,7 +62,7 @@ static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
 	unsigned long flags;
 	int ret = 0;
 	sector_t lo = r1_bio->sector;
-	sector_t hi = lo + r1_bio->sectors;
+	sector_t hi = lo + r1_bio->sectors - 1;
 	struct serial_in_rdev *serial = &rdev->serial[idx];
 
 	spin_lock_irqsave(&serial->serial_lock, flags);
@@ -453,7 +453,7 @@ static void raid1_end_write_request(struct bio *bio)
 	int mirror = find_bio_disk(r1_bio, bio);
 	struct md_rdev *rdev = conf->mirrors[mirror].rdev;
 	sector_t lo = r1_bio->sector;
-	sector_t hi = r1_bio->sector + r1_bio->sectors;
+	sector_t hi = r1_bio->sector + r1_bio->sectors - 1;
 	bool ignore_error = !raid1_should_handle_error(bio) ||
 		(bio->bi_status && bio_op(bio) == REQ_OP_DISCARD);
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 1/2] md/raid1: fix the comparing region of interval tree

[Yu Kuai]

Hi,

在 2026/2/6 13:38, Xiao Ni 写道:
> Interval tree uses [start, end] as a region which stores in the tree.
> In raid1, it uses the wrong end value. For example:
> bio(A,B) is too big and needs to be split to bio1(A,C-1), bio2(C,B).
> The region of bio1 is [A,C] and the region of bio2 is [C,B]. So bio1 and
> bio2 overlap which is not right.
>
> Fix this problem by using right end value of the region.
>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>   drivers/md/raid1.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Perhaps also add a fix tag, with this:

Reviewed-by: Yu Kuai <yukuai@fnnas.com>

>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 867db18bc3ba..a41b1ec3d695 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -62,7 +62,7 @@ static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
>   	unsigned long flags;
>   	int ret = 0;
>   	sector_t lo = r1_bio->sector;
> -	sector_t hi = lo + r1_bio->sectors;
> +	sector_t hi = lo + r1_bio->sectors - 1;
>   	struct serial_in_rdev *serial = &rdev->serial[idx];
>   
>   	spin_lock_irqsave(&serial->serial_lock, flags);
> @@ -453,7 +453,7 @@ static void raid1_end_write_request(struct bio *bio)
>   	int mirror = find_bio_disk(r1_bio, bio);
>   	struct md_rdev *rdev = conf->mirrors[mirror].rdev;
>   	sector_t lo = r1_bio->sector;
> -	sector_t hi = r1_bio->sector + r1_bio->sectors;
> +	sector_t hi = r1_bio->sector + r1_bio->sectors - 1;
>   	bool ignore_error = !raid1_should_handle_error(bio) ||
>   		(bio->bi_status && bio_op(bio) == REQ_OP_DISCARD);
>   

-- 
Thansk,
Kuai

Re: [PATCH 1/2] md/raid1: fix the comparing region of interval tree

[Xiao Ni]

On Wed, Feb 11, 2026 at 10:19 AM Yu Kuai <yukuai@fnnas.com> wrote:
>
> Hi,
>
> 在 2026/2/6 13:38, Xiao Ni 写道:
> > Interval tree uses [start, end] as a region which stores in the tree.
> > In raid1, it uses the wrong end value. For example:
> > bio(A,B) is too big and needs to be split to bio1(A,C-1), bio2(C,B).
> > The region of bio1 is [A,C] and the region of bio2 is [C,B]. So bio1 and
> > bio2 overlap which is not right.
> >
> > Fix this problem by using right end value of the region.
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >   drivers/md/raid1.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
>
> Perhaps also add a fix tag, with this:

Thanks for the suggestion. I'll add it and send V2
>
> Reviewed-by: Yu Kuai <yukuai@fnnas.com>
>
> >
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index 867db18bc3ba..a41b1ec3d695 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -62,7 +62,7 @@ static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
> >       unsigned long flags;
> >       int ret = 0;
> >       sector_t lo = r1_bio->sector;
> > -     sector_t hi = lo + r1_bio->sectors;
> > +     sector_t hi = lo + r1_bio->sectors - 1;
> >       struct serial_in_rdev *serial = &rdev->serial[idx];
> >
> >       spin_lock_irqsave(&serial->serial_lock, flags);
> > @@ -453,7 +453,7 @@ static void raid1_end_write_request(struct bio *bio)
> >       int mirror = find_bio_disk(r1_bio, bio);
> >       struct md_rdev *rdev = conf->mirrors[mirror].rdev;
> >       sector_t lo = r1_bio->sector;
> > -     sector_t hi = r1_bio->sector + r1_bio->sectors;
> > +     sector_t hi = r1_bio->sector + r1_bio->sectors - 1;
> >       bool ignore_error = !raid1_should_handle_error(bio) ||
> >               (bio->bi_status && bio_op(bio) == REQ_OP_DISCARD);
> >
>
> --
> Thansk,
> Kuai
>