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

[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
>

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

[Xiao Ni]

In behind mode, overlap bios for writemostly device are queued. They
In behind mode, bios which overlap the bio from the interval tree
need to wait in waitqueue. Those bios will be handled once the in-tree
bio finishes. They are waken up and try to get the tree lock. The bio
which gets the lock will be handled. So the sequence can't 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 is bio1,bio3,bio2. This causes data corruption.
Replace waitqueue with a fifo list to guarantee the write sequence.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/md.c    |  1 -
 drivers/md/md.h    |  4 +++-
 drivers/md/raid1.c | 39 +++++++++++++++++++++++++++++----------
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 59cd303548de..ab91d17c2d68 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -188,7 +188,6 @@ static int rdev_init_serial(struct md_rdev *rdev)
 
 		spin_lock_init(&serial_tmp->serial_lock);
 		serial_tmp->serial_rb = RB_ROOT_CACHED;
-		init_waitqueue_head(&serial_tmp->serial_io_wait);
 	}
 
 	rdev->serial = serial;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ac84289664cd..64ffbb2efb87 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -126,7 +126,6 @@ enum sync_action {
 struct serial_in_rdev {
 	struct rb_root_cached serial_rb;
 	spinlock_t serial_lock;
-	wait_queue_head_t serial_io_wait;
 };
 
 /*
@@ -382,6 +381,9 @@ struct serial_info {
 	sector_t start;		/* start sector of rb node */
 	sector_t last;		/* end sector of rb node */
 	sector_t _subtree_last; /* highest sector in subtree of rb node */
+	struct list_head	list_node;
+	struct list_head	waiters;
+	struct completion	ready;
 };
 
 /*
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a41b1ec3d695..38c73538d038 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -57,19 +57,25 @@ INTERVAL_TREE_DEFINE(struct serial_info, node, sector_t, _subtree_last,
 		     START, LAST, static inline, raid1_rb);
 
 static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
-				struct serial_info *si, int idx)
+				struct serial_info *si)
 {
 	unsigned long flags;
 	int ret = 0;
 	sector_t lo = r1_bio->sector;
 	sector_t hi = lo + r1_bio->sectors - 1;
+	int idx = sector_to_idx(r1_bio->sector);
 	struct serial_in_rdev *serial = &rdev->serial[idx];
+	struct serial_info *head_si;
 
 	spin_lock_irqsave(&serial->serial_lock, flags);
 	/* collision happened */
-	if (raid1_rb_iter_first(&serial->serial_rb, lo, hi))
+	head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
+	if (head_si && head_si != si) {
+		si->start = lo;
+		si->last = hi;
+		list_add_tail(&si->list_node, &head_si->waiters);
 		ret = -EBUSY;
-	else {
+	} else if (!head_si) {
 		si->start = lo;
 		si->last = hi;
 		raid1_rb_insert(si, &serial->serial_rb);
@@ -83,19 +89,23 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
 {
 	struct mddev *mddev = rdev->mddev;
 	struct serial_info *si;
-	int idx = sector_to_idx(r1_bio->sector);
-	struct serial_in_rdev *serial = &rdev->serial[idx];
 
 	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);
+	INIT_LIST_HEAD(&si->waiters);
+	INIT_LIST_HEAD(&si->list_node);
+	init_completion(&si->ready);
+	while (check_and_add_serial(rdev, r1_bio, si)) {
+		wait_for_completion(&si->ready);
+		reinit_completion(&si->ready);
+	}
 }
 
 static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
 {
 	struct serial_info *si;
+	struct serial_info *first_si;
 	unsigned long flags;
 	int found = 0;
 	struct mddev *mddev = rdev->mddev;
@@ -106,16 +116,25 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
 	for (si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
 	     si; si = raid1_rb_iter_next(si, lo, hi)) {
 		if (si->start == lo && si->last == hi) {
-			raid1_rb_remove(si, &serial->serial_rb);
-			mempool_free(si, mddev->serial_info_pool);
 			found = 1;
 			break;
 		}
 	}
 	if (!found)
 		WARN(1, "The write IO is not recorded for serialization\n");
+	else {
+		raid1_rb_remove(si, &serial->serial_rb);
+		if (!list_empty(&si->waiters)) {
+			first_si = list_first_entry(&si->waiters,
+						struct serial_info, list_node);
+			list_del_init(&first_si->list_node);
+			list_splice_init(&si->waiters, &first_si->waiters);
+			raid1_rb_insert(first_si, &serial->serial_rb);
+			complete(&first_si->ready);
+		}
+		mempool_free(si, mddev->serial_info_pool);
+	}
 	spin_unlock_irqrestore(&serial->serial_lock, flags);
-	wake_up(&serial->serial_io_wait);
 }
 
 /*
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 2/2] md/raid1: serialize overlapped io for writemostly disk

[Yu Kuai]

Hi,

在 2026/2/6 13:38, Xiao Ni 写道:
> In behind mode, overlap bios for writemostly device are queued. They
> In behind mode, bios which overlap the bio from the interval tree
> need to wait in waitqueue. Those bios will be handled once the in-tree
> bio finishes. They are waken up and try to get the tree lock. The bio
> which gets the lock will be handled. So the sequence can't 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 is bio1,bio3,bio2. This causes data corruption.
> Replace waitqueue with a fifo list to guarantee the write sequence.

To be honest, I think this is fine with the respect user won't send overlap
bios. In this case, bio1 is done in fast disk and pending in slow disk, then
bio2 will stuck while issuing and waiting for bio1 to return from slow disk.
So send bio3 in this case is not expected, and if you do you can't guarantee
the complete order from fast disk as bio2, bio3 as well.

>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>   drivers/md/md.c    |  1 -
>   drivers/md/md.h    |  4 +++-
>   drivers/md/raid1.c | 39 +++++++++++++++++++++++++++++----------
>   3 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 59cd303548de..ab91d17c2d68 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -188,7 +188,6 @@ static int rdev_init_serial(struct md_rdev *rdev)
>   
>   		spin_lock_init(&serial_tmp->serial_lock);
>   		serial_tmp->serial_rb = RB_ROOT_CACHED;
> -		init_waitqueue_head(&serial_tmp->serial_io_wait);
>   	}
>   
>   	rdev->serial = serial;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index ac84289664cd..64ffbb2efb87 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -126,7 +126,6 @@ enum sync_action {
>   struct serial_in_rdev {
>   	struct rb_root_cached serial_rb;
>   	spinlock_t serial_lock;
> -	wait_queue_head_t serial_io_wait;
>   };
>   
>   /*
> @@ -382,6 +381,9 @@ struct serial_info {
>   	sector_t start;		/* start sector of rb node */
>   	sector_t last;		/* end sector of rb node */
>   	sector_t _subtree_last; /* highest sector in subtree of rb node */
> +	struct list_head	list_node;
> +	struct list_head	waiters;
> +	struct completion	ready;
>   };
>   
>   /*
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index a41b1ec3d695..38c73538d038 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -57,19 +57,25 @@ INTERVAL_TREE_DEFINE(struct serial_info, node, sector_t, _subtree_last,
>   		     START, LAST, static inline, raid1_rb);
>   
>   static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
> -				struct serial_info *si, int idx)
> +				struct serial_info *si)
>   {
>   	unsigned long flags;
>   	int ret = 0;
>   	sector_t lo = r1_bio->sector;
>   	sector_t hi = lo + r1_bio->sectors - 1;
> +	int idx = sector_to_idx(r1_bio->sector);
>   	struct serial_in_rdev *serial = &rdev->serial[idx];
> +	struct serial_info *head_si;
>   
>   	spin_lock_irqsave(&serial->serial_lock, flags);
>   	/* collision happened */
> -	if (raid1_rb_iter_first(&serial->serial_rb, lo, hi))
> +	head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> +	if (head_si && head_si != si) {
> +		si->start = lo;
> +		si->last = hi;
> +		list_add_tail(&si->list_node, &head_si->waiters);
>   		ret = -EBUSY;
> -	else {
> +	} else if (!head_si) {
>   		si->start = lo;
>   		si->last = hi;
>   		raid1_rb_insert(si, &serial->serial_rb);
> @@ -83,19 +89,23 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
>   {
>   	struct mddev *mddev = rdev->mddev;
>   	struct serial_info *si;
> -	int idx = sector_to_idx(r1_bio->sector);
> -	struct serial_in_rdev *serial = &rdev->serial[idx];
>   
>   	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);
> +	INIT_LIST_HEAD(&si->waiters);
> +	INIT_LIST_HEAD(&si->list_node);
> +	init_completion(&si->ready);
> +	while (check_and_add_serial(rdev, r1_bio, si)) {
> +		wait_for_completion(&si->ready);
> +		reinit_completion(&si->ready);
> +	}
>   }
>   
>   static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
>   {
>   	struct serial_info *si;
> +	struct serial_info *first_si;
>   	unsigned long flags;
>   	int found = 0;
>   	struct mddev *mddev = rdev->mddev;
> @@ -106,16 +116,25 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
>   	for (si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
>   	     si; si = raid1_rb_iter_next(si, lo, hi)) {
>   		if (si->start == lo && si->last == hi) {
> -			raid1_rb_remove(si, &serial->serial_rb);
> -			mempool_free(si, mddev->serial_info_pool);
>   			found = 1;
>   			break;
>   		}
>   	}
>   	if (!found)
>   		WARN(1, "The write IO is not recorded for serialization\n");
> +	else {
> +		raid1_rb_remove(si, &serial->serial_rb);
> +		if (!list_empty(&si->waiters)) {
> +			first_si = list_first_entry(&si->waiters,
> +						struct serial_info, list_node);
> +			list_del_init(&first_si->list_node);
> +			list_splice_init(&si->waiters, &first_si->waiters);
> +			raid1_rb_insert(first_si, &serial->serial_rb);
> +			complete(&first_si->ready);
> +		}
> +		mempool_free(si, mddev->serial_info_pool);
> +	}
>   	spin_unlock_irqrestore(&serial->serial_lock, flags);
> -	wake_up(&serial->serial_io_wait);
>   }
>   
>   /*

-- 
Thansk,
Kuai

Re: [PATCH 2/2] md/raid1: serialize overlapped io for writemostly disk

[Xiao Ni]

On Wed, Feb 11, 2026 at 10:32 AM Yu Kuai <yukuai@fnnas.com> wrote:
>
> Hi,
>
> 在 2026/2/6 13:38, Xiao Ni 写道:
> > In behind mode, overlap bios for writemostly device are queued. They
> > In behind mode, bios which overlap the bio from the interval tree
> > need to wait in waitqueue. Those bios will be handled once the in-tree
> > bio finishes. They are waken up and try to get the tree lock. The bio
> > which gets the lock will be handled. So the sequence can't 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 is bio1,bio3,bio2. This causes data corruption.
> > Replace waitqueue with a fifo list to guarantee the write sequence.
>
> To be honest, I think this is fine with the respect user won't send overlap
> bios. In this case, bio1 is done in fast disk and pending in slow disk, then
> bio2 will stuck while issuing and waiting for bio1 to return from slow disk.
> So send bio3 in this case is not expected, and if you do you can't guarantee
> the complete order from fast disk as bio2, bio3 as well.

Hi Kuai

In write behind mode, raid1 returns bio1 to the upper layer once the
write on the fast disk finishes. The upper layer (app/filesystem)
thinks that bio1 has finished, while the write of bio1 to the slow
disk is still in progress. So bio2 will not be stuck while issuing, am
I right? If so, bio2 can be submitted to raid1 once the write of bio1
to the fast disk finishes. And bio3 can be submitted to raid1 once the
write of bio2 to fast disk finishes.

Best Regards
Xiao
>
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >   drivers/md/md.c    |  1 -
> >   drivers/md/md.h    |  4 +++-
> >   drivers/md/raid1.c | 39 +++++++++++++++++++++++++++++----------
> >   3 files changed, 32 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 59cd303548de..ab91d17c2d68 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -188,7 +188,6 @@ static int rdev_init_serial(struct md_rdev *rdev)
> >
> >               spin_lock_init(&serial_tmp->serial_lock);
> >               serial_tmp->serial_rb = RB_ROOT_CACHED;
> > -             init_waitqueue_head(&serial_tmp->serial_io_wait);
> >       }
> >
> >       rdev->serial = serial;
> > diff --git a/drivers/md/md.h b/drivers/md/md.h
> > index ac84289664cd..64ffbb2efb87 100644
> > --- a/drivers/md/md.h
> > +++ b/drivers/md/md.h
> > @@ -126,7 +126,6 @@ enum sync_action {
> >   struct serial_in_rdev {
> >       struct rb_root_cached serial_rb;
> >       spinlock_t serial_lock;
> > -     wait_queue_head_t serial_io_wait;
> >   };
> >
> >   /*
> > @@ -382,6 +381,9 @@ struct serial_info {
> >       sector_t start;         /* start sector of rb node */
> >       sector_t last;          /* end sector of rb node */
> >       sector_t _subtree_last; /* highest sector in subtree of rb node */
> > +     struct list_head        list_node;
> > +     struct list_head        waiters;
> > +     struct completion       ready;
> >   };
> >
> >   /*
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index a41b1ec3d695..38c73538d038 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -57,19 +57,25 @@ INTERVAL_TREE_DEFINE(struct serial_info, node, sector_t, _subtree_last,
> >                    START, LAST, static inline, raid1_rb);
> >
> >   static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
> > -                             struct serial_info *si, int idx)
> > +                             struct serial_info *si)
> >   {
> >       unsigned long flags;
> >       int ret = 0;
> >       sector_t lo = r1_bio->sector;
> >       sector_t hi = lo + r1_bio->sectors - 1;
> > +     int idx = sector_to_idx(r1_bio->sector);
> >       struct serial_in_rdev *serial = &rdev->serial[idx];
> > +     struct serial_info *head_si;
> >
> >       spin_lock_irqsave(&serial->serial_lock, flags);
> >       /* collision happened */
> > -     if (raid1_rb_iter_first(&serial->serial_rb, lo, hi))
> > +     head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> > +     if (head_si && head_si != si) {
> > +             si->start = lo;
> > +             si->last = hi;
> > +             list_add_tail(&si->list_node, &head_si->waiters);
> >               ret = -EBUSY;
> > -     else {
> > +     } else if (!head_si) {
> >               si->start = lo;
> >               si->last = hi;
> >               raid1_rb_insert(si, &serial->serial_rb);
> > @@ -83,19 +89,23 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
> >   {
> >       struct mddev *mddev = rdev->mddev;
> >       struct serial_info *si;
> > -     int idx = sector_to_idx(r1_bio->sector);
> > -     struct serial_in_rdev *serial = &rdev->serial[idx];
> >
> >       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);
> > +     INIT_LIST_HEAD(&si->waiters);
> > +     INIT_LIST_HEAD(&si->list_node);
> > +     init_completion(&si->ready);
> > +     while (check_and_add_serial(rdev, r1_bio, si)) {
> > +             wait_for_completion(&si->ready);
> > +             reinit_completion(&si->ready);
> > +     }
> >   }
> >
> >   static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
> >   {
> >       struct serial_info *si;
> > +     struct serial_info *first_si;
> >       unsigned long flags;
> >       int found = 0;
> >       struct mddev *mddev = rdev->mddev;
> > @@ -106,16 +116,25 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
> >       for (si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> >            si; si = raid1_rb_iter_next(si, lo, hi)) {
> >               if (si->start == lo && si->last == hi) {
> > -                     raid1_rb_remove(si, &serial->serial_rb);
> > -                     mempool_free(si, mddev->serial_info_pool);
> >                       found = 1;
> >                       break;
> >               }
> >       }
> >       if (!found)
> >               WARN(1, "The write IO is not recorded for serialization\n");
> > +     else {
> > +             raid1_rb_remove(si, &serial->serial_rb);
> > +             if (!list_empty(&si->waiters)) {
> > +                     first_si = list_first_entry(&si->waiters,
> > +                                             struct serial_info, list_node);
> > +                     list_del_init(&first_si->list_node);
> > +                     list_splice_init(&si->waiters, &first_si->waiters);
> > +                     raid1_rb_insert(first_si, &serial->serial_rb);
> > +                     complete(&first_si->ready);
> > +             }
> > +             mempool_free(si, mddev->serial_info_pool);
> > +     }
> >       spin_unlock_irqrestore(&serial->serial_lock, flags);
> > -     wake_up(&serial->serial_io_wait);
> >   }
> >
> >   /*
>
> --
> Thansk,
> Kuai
>

Re: [PATCH 2/2] md/raid1: serialize overlapped io for writemostly disk

[Xiao Ni]

On Wed, Feb 11, 2026 at 10:44 AM Xiao Ni <xni@redhat.com> wrote:
>
> On Wed, Feb 11, 2026 at 10:32 AM Yu Kuai <yukuai@fnnas.com> wrote:
> >
> > Hi,
> >
> > 在 2026/2/6 13:38, Xiao Ni 写道:
> > > In behind mode, overlap bios for writemostly device are queued. They
> > > In behind mode, bios which overlap the bio from the interval tree
> > > need to wait in waitqueue. Those bios will be handled once the in-tree
> > > bio finishes. They are waken up and try to get the tree lock. The bio
> > > which gets the lock will be handled. So the sequence can't 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 is bio1,bio3,bio2. This causes data corruption.
> > > Replace waitqueue with a fifo list to guarantee the write sequence.
> >
> > To be honest, I think this is fine with the respect user won't send overlap
> > bios. In this case, bio1 is done in fast disk and pending in slow disk, then
> > bio2 will stuck while issuing and waiting for bio1 to return from slow disk.
> > So send bio3 in this case is not expected, and if you do you can't guarantee
> > the complete order from fast disk as bio2, bio3 as well.
>
> Hi Kuai
>
> In write behind mode, raid1 returns bio1 to the upper layer once the
> write on the fast disk finishes. The upper layer (app/filesystem)
> thinks that bio1 has finished, while the write of bio1 to the slow
> disk is still in progress. So bio2 will not be stuck while issuing, am
> I right? If so, bio2 can be submitted to raid1 once the write of bio1
> to the fast disk finishes. And bio3 can be submitted to raid1 once the
> write of bio2 to fast disk finishes.

By the way, it's reported by our customer, there is a data corruption
by the following test script:
#!/bin/sh

disk1=/dev/loop0
disk2=/dev/loop1
mddev=/dev/md0
mount_md=/data-md
mount_disk1=/data-1
mount_disk2=/data-2
bitmap=/root/bitmap
test_prg=/root/test-serializewrite
test_num=1000
test_str=abcdefghijklmnopqrstuvwxyz

loop=0
touch MD-TEST
while [ -f MD-TEST ]; do
    set -x
    loop=$((loop + 1))
    echo "START LOOP=$loop at $(date)"

    echo "# lsblk"
    lsblk

    echo "# dmsetup"
    dmsetup create disk-500ms --table "0 $(blockdev --getsz $disk2)
delay $disk2 0 500"
    sleep 2

    echo "# mdadm"
    #mdadm --create $mddev --run --assume-clean --force --level=1
--write-behind=1024 --bitmap=$bitmap --raid-devices=2 $disk1
--write-mostly /dev/mapper/disk-500ms
    mdadm --create $mddev --run --assume-clean --force --level=1
--write-behind=1024 --bitmap=internal --raid-devices=2 $disk1
--write-mostly /dev/mapper/disk-500ms

    echo "# cat /proc/mdstat"
    cat /proc/mdstat

    #echo "# mdadm -X $bitmap"
    #mdadm -X $bitmap

    echo "# mkfs.xfs -f $mddev"
    mkfs.xfs -f $mddev

    echo "# mount $mddev $mount_md"
    mount $mddev $mount_md

    set +x
    echo "# $test_prg"
    for n in $(seq 1 $test_num); do
$test_prg $mount_md/test.$n $test_str &
    done

    while true; do
sleep 2
pgrep -f $(basename $test_prg) > /dev/null
if [ $? -ne 0 ]; then
    break
fi
    done

    set -x
    echo "# cat /proc/mdstat"
    cat /proc/mdstat

    #echo "# mdadm -X $bitmap"
    #mdadm -X $bitmap

    echo "# umount $mount_md"
    sleep 10
    while true; do
sleep 2
umount $mount_md
if [ $? -eq 0 ]; then
    break
fi
    done

    echo "# mdadm --stop $mddev"
    mdadm --stop $mddev

    echo "# dmsetup remove disk-500ms"
    while true; do
        dmsetup remove disk-500ms
        if [ $? -ne 0 ]; then
            sleep 5
        else
            break
        fi
    done

    offset=$(mdadm --examine $disk1 | awk -F': ' '/Data Offset/ {print
$2}' | awk '{print $1}')
    offset=$((offset * 512))
    echo "# mount -o ro,offset=$offset,nouuid $disk1 $mount_disk1"
    mount -o ro,offset=$offset,nouuid $disk1 $mount_disk1

    offset=$(mdadm --examine $disk2 | awk -F': ' '/Data Offset/ {print
$2}' | awk '{print $1}')
    #offset=$(mdadm --examine /dev/mapper/disk-500ms | awk -F': '
'/Data Offset/ {print $2}' | awk '{print $1}')

    offset=$((offset * 512))
    echo "# mount -o ro,offset=$offset,nouuid $disk2 $mount_disk2"
    #echo "# mount -o ro,offset=$offset,nouuid /dev/mapper/disk-500ms
$mount_disk2"

    mount -o ro,offset=$offset,nouuid $disk2 $mount_disk2
    #mount -o ro,offset=$offset,nouuid /dev/mapper/disk-500ms $mount_disk2

    echo "# diff -rq $mount_disk1 $mount_disk2"
    diff -rq $mount_disk1 $mount_disk2
    if [ $? -eq 0 ]; then
echo "@@@ PASS (loop=$loop)"
    else
echo "@@@ FAIL (loop=$loop)"
    fi

    umount $mount_disk1
    umount $mount_disk2

    mdadm --zero-superblock  $disk1
    mdadm --zero-superblock  $disk2

    sleep 10
    set +x
done
>
> Best Regards
> Xiao
> >
> > >
> > > Signed-off-by: Xiao Ni <xni@redhat.com>
> > > ---
> > >   drivers/md/md.c    |  1 -
> > >   drivers/md/md.h    |  4 +++-
> > >   drivers/md/raid1.c | 39 +++++++++++++++++++++++++++++----------
> > >   3 files changed, 32 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > index 59cd303548de..ab91d17c2d68 100644
> > > --- a/drivers/md/md.c
> > > +++ b/drivers/md/md.c
> > > @@ -188,7 +188,6 @@ static int rdev_init_serial(struct md_rdev *rdev)
> > >
> > >               spin_lock_init(&serial_tmp->serial_lock);
> > >               serial_tmp->serial_rb = RB_ROOT_CACHED;
> > > -             init_waitqueue_head(&serial_tmp->serial_io_wait);
> > >       }
> > >
> > >       rdev->serial = serial;
> > > diff --git a/drivers/md/md.h b/drivers/md/md.h
> > > index ac84289664cd..64ffbb2efb87 100644
> > > --- a/drivers/md/md.h
> > > +++ b/drivers/md/md.h
> > > @@ -126,7 +126,6 @@ enum sync_action {
> > >   struct serial_in_rdev {
> > >       struct rb_root_cached serial_rb;
> > >       spinlock_t serial_lock;
> > > -     wait_queue_head_t serial_io_wait;
> > >   };
> > >
> > >   /*
> > > @@ -382,6 +381,9 @@ struct serial_info {
> > >       sector_t start;         /* start sector of rb node */
> > >       sector_t last;          /* end sector of rb node */
> > >       sector_t _subtree_last; /* highest sector in subtree of rb node */
> > > +     struct list_head        list_node;
> > > +     struct list_head        waiters;
> > > +     struct completion       ready;
> > >   };
> > >
> > >   /*
> > > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > > index a41b1ec3d695..38c73538d038 100644
> > > --- a/drivers/md/raid1.c
> > > +++ b/drivers/md/raid1.c
> > > @@ -57,19 +57,25 @@ INTERVAL_TREE_DEFINE(struct serial_info, node, sector_t, _subtree_last,
> > >                    START, LAST, static inline, raid1_rb);
> > >
> > >   static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
> > > -                             struct serial_info *si, int idx)
> > > +                             struct serial_info *si)
> > >   {
> > >       unsigned long flags;
> > >       int ret = 0;
> > >       sector_t lo = r1_bio->sector;
> > >       sector_t hi = lo + r1_bio->sectors - 1;
> > > +     int idx = sector_to_idx(r1_bio->sector);
> > >       struct serial_in_rdev *serial = &rdev->serial[idx];
> > > +     struct serial_info *head_si;
> > >
> > >       spin_lock_irqsave(&serial->serial_lock, flags);
> > >       /* collision happened */
> > > -     if (raid1_rb_iter_first(&serial->serial_rb, lo, hi))
> > > +     head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> > > +     if (head_si && head_si != si) {
> > > +             si->start = lo;
> > > +             si->last = hi;
> > > +             list_add_tail(&si->list_node, &head_si->waiters);
> > >               ret = -EBUSY;
> > > -     else {
> > > +     } else if (!head_si) {
> > >               si->start = lo;
> > >               si->last = hi;
> > >               raid1_rb_insert(si, &serial->serial_rb);
> > > @@ -83,19 +89,23 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
> > >   {
> > >       struct mddev *mddev = rdev->mddev;
> > >       struct serial_info *si;
> > > -     int idx = sector_to_idx(r1_bio->sector);
> > > -     struct serial_in_rdev *serial = &rdev->serial[idx];
> > >
> > >       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);
> > > +     INIT_LIST_HEAD(&si->waiters);
> > > +     INIT_LIST_HEAD(&si->list_node);
> > > +     init_completion(&si->ready);
> > > +     while (check_and_add_serial(rdev, r1_bio, si)) {
> > > +             wait_for_completion(&si->ready);
> > > +             reinit_completion(&si->ready);
> > > +     }
> > >   }
> > >
> > >   static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
> > >   {
> > >       struct serial_info *si;
> > > +     struct serial_info *first_si;
> > >       unsigned long flags;
> > >       int found = 0;
> > >       struct mddev *mddev = rdev->mddev;
> > > @@ -106,16 +116,25 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
> > >       for (si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> > >            si; si = raid1_rb_iter_next(si, lo, hi)) {
> > >               if (si->start == lo && si->last == hi) {
> > > -                     raid1_rb_remove(si, &serial->serial_rb);
> > > -                     mempool_free(si, mddev->serial_info_pool);
> > >                       found = 1;
> > >                       break;
> > >               }
> > >       }
> > >       if (!found)
> > >               WARN(1, "The write IO is not recorded for serialization\n");
> > > +     else {
> > > +             raid1_rb_remove(si, &serial->serial_rb);
> > > +             if (!list_empty(&si->waiters)) {
> > > +                     first_si = list_first_entry(&si->waiters,
> > > +                                             struct serial_info, list_node);
> > > +                     list_del_init(&first_si->list_node);
> > > +                     list_splice_init(&si->waiters, &first_si->waiters);
> > > +                     raid1_rb_insert(first_si, &serial->serial_rb);
> > > +                     complete(&first_si->ready);
> > > +             }
> > > +             mempool_free(si, mddev->serial_info_pool);
> > > +     }
> > >       spin_unlock_irqrestore(&serial->serial_lock, flags);
> > > -     wake_up(&serial->serial_io_wait);
> > >   }
> > >
> > >   /*
> >
> > --
> > Thansk,
> > Kuai
> >

Re: [PATCH 2/2] md/raid1: serialize overlapped io for writemostly disk

[Yu Kuai]

Hi,

在 2026/2/11 10:44, Xiao Ni 写道:
> On Wed, Feb 11, 2026 at 10:32 AM Yu Kuai <yukuai@fnnas.com> wrote:
>> Hi,
>>
>> 在 2026/2/6 13:38, Xiao Ni 写道:
>>> In behind mode, overlap bios for writemostly device are queued. They
>>> In behind mode, bios which overlap the bio from the interval tree
>>> need to wait in waitqueue. Those bios will be handled once the in-tree
>>> bio finishes. They are waken up and try to get the tree lock. The bio
>>> which gets the lock will be handled. So the sequence can't 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 is bio1,bio3,bio2. This causes data corruption.
>>> Replace waitqueue with a fifo list to guarantee the write sequence.
>> To be honest, I think this is fine with the respect user won't send overlap
>> bios. In this case, bio1 is done in fast disk and pending in slow disk, then
>> bio2 will stuck while issuing and waiting for bio1 to return from slow disk.
>> So send bio3 in this case is not expected, and if you do you can't guarantee
>> the complete order from fast disk as bio2, bio3 as well.
> Hi Kuai
>
> In write behind mode, raid1 returns bio1 to the upper layer once the
> write on the fast disk finishes. The upper layer (app/filesystem)
> thinks that bio1 has finished, while the write of bio1 to the slow
> disk is still in progress. So bio2 will not be stuck while issuing, am
> I right? If so, bio2 can be submitted to raid1 once the write of bio1
> to the fast disk finishes. And bio3 can be submitted to raid1 once the
> write of bio2 to fast disk finishes.

I just take a look at raid1 code, turns out raid1_write_request() just allocate
bio and issue it directly in each rdev iteration, I misunderstood.

This solution looks fine to me, however, I'll suggest to keep the wait_queue and
use prepare_to_wait_exclusive() to guarantee the fifo order.

>
> Best Regards
> Xiao
>>> Signed-off-by: Xiao Ni <xni@redhat.com>
>>> ---
>>>    drivers/md/md.c    |  1 -
>>>    drivers/md/md.h    |  4 +++-
>>>    drivers/md/raid1.c | 39 +++++++++++++++++++++++++++++----------
>>>    3 files changed, 32 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>> index 59cd303548de..ab91d17c2d68 100644
>>> --- a/drivers/md/md.c
>>> +++ b/drivers/md/md.c
>>> @@ -188,7 +188,6 @@ static int rdev_init_serial(struct md_rdev *rdev)
>>>
>>>                spin_lock_init(&serial_tmp->serial_lock);
>>>                serial_tmp->serial_rb = RB_ROOT_CACHED;
>>> -             init_waitqueue_head(&serial_tmp->serial_io_wait);
>>>        }
>>>
>>>        rdev->serial = serial;
>>> diff --git a/drivers/md/md.h b/drivers/md/md.h
>>> index ac84289664cd..64ffbb2efb87 100644
>>> --- a/drivers/md/md.h
>>> +++ b/drivers/md/md.h
>>> @@ -126,7 +126,6 @@ enum sync_action {
>>>    struct serial_in_rdev {
>>>        struct rb_root_cached serial_rb;
>>>        spinlock_t serial_lock;
>>> -     wait_queue_head_t serial_io_wait;
>>>    };
>>>
>>>    /*
>>> @@ -382,6 +381,9 @@ struct serial_info {
>>>        sector_t start;         /* start sector of rb node */
>>>        sector_t last;          /* end sector of rb node */
>>>        sector_t _subtree_last; /* highest sector in subtree of rb node */
>>> +     struct list_head        list_node;
>>> +     struct list_head        waiters;
>>> +     struct completion       ready;
>>>    };
>>>
>>>    /*
>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>> index a41b1ec3d695..38c73538d038 100644
>>> --- a/drivers/md/raid1.c
>>> +++ b/drivers/md/raid1.c
>>> @@ -57,19 +57,25 @@ INTERVAL_TREE_DEFINE(struct serial_info, node, sector_t, _subtree_last,
>>>                     START, LAST, static inline, raid1_rb);
>>>
>>>    static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
>>> -                             struct serial_info *si, int idx)
>>> +                             struct serial_info *si)
>>>    {
>>>        unsigned long flags;
>>>        int ret = 0;
>>>        sector_t lo = r1_bio->sector;
>>>        sector_t hi = lo + r1_bio->sectors - 1;
>>> +     int idx = sector_to_idx(r1_bio->sector);
>>>        struct serial_in_rdev *serial = &rdev->serial[idx];
>>> +     struct serial_info *head_si;
>>>
>>>        spin_lock_irqsave(&serial->serial_lock, flags);
>>>        /* collision happened */
>>> -     if (raid1_rb_iter_first(&serial->serial_rb, lo, hi))
>>> +     head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
>>> +     if (head_si && head_si != si) {
>>> +             si->start = lo;
>>> +             si->last = hi;
>>> +             list_add_tail(&si->list_node, &head_si->waiters);
>>>                ret = -EBUSY;
>>> -     else {
>>> +     } else if (!head_si) {
>>>                si->start = lo;
>>>                si->last = hi;
>>>                raid1_rb_insert(si, &serial->serial_rb);
>>> @@ -83,19 +89,23 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
>>>    {
>>>        struct mddev *mddev = rdev->mddev;
>>>        struct serial_info *si;
>>> -     int idx = sector_to_idx(r1_bio->sector);
>>> -     struct serial_in_rdev *serial = &rdev->serial[idx];
>>>
>>>        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);
>>> +     INIT_LIST_HEAD(&si->waiters);
>>> +     INIT_LIST_HEAD(&si->list_node);
>>> +     init_completion(&si->ready);
>>> +     while (check_and_add_serial(rdev, r1_bio, si)) {
>>> +             wait_for_completion(&si->ready);
>>> +             reinit_completion(&si->ready);
>>> +     }
>>>    }
>>>
>>>    static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
>>>    {
>>>        struct serial_info *si;
>>> +     struct serial_info *first_si;
>>>        unsigned long flags;
>>>        int found = 0;
>>>        struct mddev *mddev = rdev->mddev;
>>> @@ -106,16 +116,25 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
>>>        for (si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
>>>             si; si = raid1_rb_iter_next(si, lo, hi)) {
>>>                if (si->start == lo && si->last == hi) {
>>> -                     raid1_rb_remove(si, &serial->serial_rb);
>>> -                     mempool_free(si, mddev->serial_info_pool);
>>>                        found = 1;
>>>                        break;
>>>                }
>>>        }
>>>        if (!found)
>>>                WARN(1, "The write IO is not recorded for serialization\n");
>>> +     else {
>>> +             raid1_rb_remove(si, &serial->serial_rb);
>>> +             if (!list_empty(&si->waiters)) {
>>> +                     first_si = list_first_entry(&si->waiters,
>>> +                                             struct serial_info, list_node);
>>> +                     list_del_init(&first_si->list_node);
>>> +                     list_splice_init(&si->waiters, &first_si->waiters);
>>> +                     raid1_rb_insert(first_si, &serial->serial_rb);
>>> +                     complete(&first_si->ready);
>>> +             }
>>> +             mempool_free(si, mddev->serial_info_pool);
>>> +     }
>>>        spin_unlock_irqrestore(&serial->serial_lock, flags);
>>> -     wake_up(&serial->serial_io_wait);
>>>    }
>>>
>>>    /*
>> --
>> Thansk,
>> Kuai
>>
>
-- 
Thansk,
Kuai

Re: [PATCH 2/2] md/raid1: serialize overlapped io for writemostly disk

[Xiao Ni]

On Wed, Feb 11, 2026 at 11:15 AM Yu Kuai <yukuai@fnnas.com> wrote:
>
> Hi,
>
> 在 2026/2/11 10:44, Xiao Ni 写道:
> > On Wed, Feb 11, 2026 at 10:32 AM Yu Kuai <yukuai@fnnas.com> wrote:
> >> Hi,
> >>
> >> 在 2026/2/6 13:38, Xiao Ni 写道:
> >>> In behind mode, overlap bios for writemostly device are queued. They
> >>> In behind mode, bios which overlap the bio from the interval tree
> >>> need to wait in waitqueue. Those bios will be handled once the in-tree
> >>> bio finishes. They are waken up and try to get the tree lock. The bio
> >>> which gets the lock will be handled. So the sequence can't 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 is bio1,bio3,bio2. This causes data corruption.
> >>> Replace waitqueue with a fifo list to guarantee the write sequence.
> >> To be honest, I think this is fine with the respect user won't send overlap
> >> bios. In this case, bio1 is done in fast disk and pending in slow disk, then
> >> bio2 will stuck while issuing and waiting for bio1 to return from slow disk.
> >> So send bio3 in this case is not expected, and if you do you can't guarantee
> >> the complete order from fast disk as bio2, bio3 as well.
> > Hi Kuai
> >
> > In write behind mode, raid1 returns bio1 to the upper layer once the
> > write on the fast disk finishes. The upper layer (app/filesystem)
> > thinks that bio1 has finished, while the write of bio1 to the slow
> > disk is still in progress. So bio2 will not be stuck while issuing, am
> > I right? If so, bio2 can be submitted to raid1 once the write of bio1
> > to the fast disk finishes. And bio3 can be submitted to raid1 once the
> > write of bio2 to fast disk finishes.
>
> I just take a look at raid1 code, turns out raid1_write_request() just allocate
> bio and issue it directly in each rdev iteration, I misunderstood.
>
> This solution looks fine to me, however, I'll suggest to keep the wait_queue and
> use prepare_to_wait_exclusive() to guarantee the fifo order.

Thanks for the suggestion. I'll use prepare_to_wait_exclusive and send v2.

Best Regards
Xiao
>
> >
> > Best Regards
> > Xiao
> >>> Signed-off-by: Xiao Ni <xni@redhat.com>
> >>> ---
> >>>    drivers/md/md.c    |  1 -
> >>>    drivers/md/md.h    |  4 +++-
> >>>    drivers/md/raid1.c | 39 +++++++++++++++++++++++++++++----------
> >>>    3 files changed, 32 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >>> index 59cd303548de..ab91d17c2d68 100644
> >>> --- a/drivers/md/md.c
> >>> +++ b/drivers/md/md.c
> >>> @@ -188,7 +188,6 @@ static int rdev_init_serial(struct md_rdev *rdev)
> >>>
> >>>                spin_lock_init(&serial_tmp->serial_lock);
> >>>                serial_tmp->serial_rb = RB_ROOT_CACHED;
> >>> -             init_waitqueue_head(&serial_tmp->serial_io_wait);
> >>>        }
> >>>
> >>>        rdev->serial = serial;
> >>> diff --git a/drivers/md/md.h b/drivers/md/md.h
> >>> index ac84289664cd..64ffbb2efb87 100644
> >>> --- a/drivers/md/md.h
> >>> +++ b/drivers/md/md.h
> >>> @@ -126,7 +126,6 @@ enum sync_action {
> >>>    struct serial_in_rdev {
> >>>        struct rb_root_cached serial_rb;
> >>>        spinlock_t serial_lock;
> >>> -     wait_queue_head_t serial_io_wait;
> >>>    };
> >>>
> >>>    /*
> >>> @@ -382,6 +381,9 @@ struct serial_info {
> >>>        sector_t start;         /* start sector of rb node */
> >>>        sector_t last;          /* end sector of rb node */
> >>>        sector_t _subtree_last; /* highest sector in subtree of rb node */
> >>> +     struct list_head        list_node;
> >>> +     struct list_head        waiters;
> >>> +     struct completion       ready;
> >>>    };
> >>>
> >>>    /*
> >>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> >>> index a41b1ec3d695..38c73538d038 100644
> >>> --- a/drivers/md/raid1.c
> >>> +++ b/drivers/md/raid1.c
> >>> @@ -57,19 +57,25 @@ INTERVAL_TREE_DEFINE(struct serial_info, node, sector_t, _subtree_last,
> >>>                     START, LAST, static inline, raid1_rb);
> >>>
> >>>    static int check_and_add_serial(struct md_rdev *rdev, struct r1bio *r1_bio,
> >>> -                             struct serial_info *si, int idx)
> >>> +                             struct serial_info *si)
> >>>    {
> >>>        unsigned long flags;
> >>>        int ret = 0;
> >>>        sector_t lo = r1_bio->sector;
> >>>        sector_t hi = lo + r1_bio->sectors - 1;
> >>> +     int idx = sector_to_idx(r1_bio->sector);
> >>>        struct serial_in_rdev *serial = &rdev->serial[idx];
> >>> +     struct serial_info *head_si;
> >>>
> >>>        spin_lock_irqsave(&serial->serial_lock, flags);
> >>>        /* collision happened */
> >>> -     if (raid1_rb_iter_first(&serial->serial_rb, lo, hi))
> >>> +     head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> >>> +     if (head_si && head_si != si) {
> >>> +             si->start = lo;
> >>> +             si->last = hi;
> >>> +             list_add_tail(&si->list_node, &head_si->waiters);
> >>>                ret = -EBUSY;
> >>> -     else {
> >>> +     } else if (!head_si) {
> >>>                si->start = lo;
> >>>                si->last = hi;
> >>>                raid1_rb_insert(si, &serial->serial_rb);
> >>> @@ -83,19 +89,23 @@ static void wait_for_serialization(struct md_rdev *rdev, struct r1bio *r1_bio)
> >>>    {
> >>>        struct mddev *mddev = rdev->mddev;
> >>>        struct serial_info *si;
> >>> -     int idx = sector_to_idx(r1_bio->sector);
> >>> -     struct serial_in_rdev *serial = &rdev->serial[idx];
> >>>
> >>>        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);
> >>> +     INIT_LIST_HEAD(&si->waiters);
> >>> +     INIT_LIST_HEAD(&si->list_node);
> >>> +     init_completion(&si->ready);
> >>> +     while (check_and_add_serial(rdev, r1_bio, si)) {
> >>> +             wait_for_completion(&si->ready);
> >>> +             reinit_completion(&si->ready);
> >>> +     }
> >>>    }
> >>>
> >>>    static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
> >>>    {
> >>>        struct serial_info *si;
> >>> +     struct serial_info *first_si;
> >>>        unsigned long flags;
> >>>        int found = 0;
> >>>        struct mddev *mddev = rdev->mddev;
> >>> @@ -106,16 +116,25 @@ static void remove_serial(struct md_rdev *rdev, sector_t lo, sector_t hi)
> >>>        for (si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
> >>>             si; si = raid1_rb_iter_next(si, lo, hi)) {
> >>>                if (si->start == lo && si->last == hi) {
> >>> -                     raid1_rb_remove(si, &serial->serial_rb);
> >>> -                     mempool_free(si, mddev->serial_info_pool);
> >>>                        found = 1;
> >>>                        break;
> >>>                }
> >>>        }
> >>>        if (!found)
> >>>                WARN(1, "The write IO is not recorded for serialization\n");
> >>> +     else {
> >>> +             raid1_rb_remove(si, &serial->serial_rb);
> >>> +             if (!list_empty(&si->waiters)) {
> >>> +                     first_si = list_first_entry(&si->waiters,
> >>> +                                             struct serial_info, list_node);
> >>> +                     list_del_init(&first_si->list_node);
> >>> +                     list_splice_init(&si->waiters, &first_si->waiters);
> >>> +                     raid1_rb_insert(first_si, &serial->serial_rb);
> >>> +                     complete(&first_si->ready);
> >>> +             }
> >>> +             mempool_free(si, mddev->serial_info_pool);
> >>> +     }
> >>>        spin_unlock_irqrestore(&serial->serial_lock, flags);
> >>> -     wake_up(&serial->serial_io_wait);
> >>>    }
> >>>
> >>>    /*
> >> --
> >> Thansk,
> >> Kuai
> >>
> >
> --
> Thansk,
> Kuai
>