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

Hi,

在 2026/2/11 10:44, Xiao Ni 写道:
> 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.

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 <[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
>>
>
-- 
Thansk,
Kuai