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

On Wed, Feb 11, 2026 at 10:32 AM Yu Kuai <[email protected]> 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 <[email protected]>
> > ---
> >   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
>