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

On Wed, Feb 11, 2026 at 10:44 AM Xiao Ni <[email protected]> wrote:
>
> 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.

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