[PATCH blktests v3 2/2] md: add regression test for "md/md-bitmap: fix writing non bitmap pages"

[Ofir Gal]

On 7/19/24 06:28, Yu Kuai wrote:
> Hi,
>
> 在 2024/07/16 19:50, Ofir Gal 写道:
>> A bug in md-bitmap has been discovered by setting up a md raid on top of
>> nvme-tcp devices that has optimal io size larger than the allocated
>> bitmap.
>>
>> The following test reproduce the bug by setting up a md-raid on top of
>> nvme-tcp device over ram device that sets the optimal io size by using
>> dm-stripe.
>>
>> Signed-off-by: Ofir Gal <ofir.gal at volumez.com>
>> ---
>>   common/brd       | 28 ++++++++++++++++
>>   tests/md/001     | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/md/001.out |  3 ++
>>   tests/md/rc      | 12 +++++++
>>   4 files changed, 128 insertions(+)
>>   create mode 100644 common/brd
>>   create mode 100755 tests/md/001
>>   create mode 100644 tests/md/001.out
>>   create mode 100644 tests/md/rc
>>
>> diff --git a/common/brd b/common/brd
>> new file mode 100644
>> index 0000000..31e964f
>> --- /dev/null
>> +++ b/common/brd
>> @@ -0,0 +1,28 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Ofir Gal
>> +#
>> +# brd helper functions
>> +
>> +. common/shellcheck
>> +
>> +_have_brd() {
>> +    _have_module brd
>> +}
>> +
>> +_init_brd() {
>> +    # _have_brd loads brd, we need to wait a bit for brd to be not in use in
>> +    # order to reload it
>> +    sleep 0.2
>> +
>> +    if ! modprobe -r brd || ! modprobe brd "$@" ; then
>> +        echo "failed to reload brd with args: $*"
>> +        return 1
>> +    fi
>> +
>> +    return 0
>> +}
>> +
>> +_cleanup_brd() {
>> +    modprobe -r brd
>> +}
>> diff --git a/tests/md/001 b/tests/md/001
>> new file mode 100755
>> index 0000000..e9578e8
>> --- /dev/null
>> +++ b/tests/md/001
>> @@ -0,0 +1,85 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Ofir Gal
>> +#
>> +# The bug is "visible" only when the underlying device of the raid is a network
>> +# block device that utilize MSG_SPLICE_PAGES. nvme-tcp is used as the network device.
>> +#
>> +# Regression test for patch "md/md-bitmap: fix writing non bitmap pages" and
>> +# for patch "nvme-tcp: use sendpages_ok() instead of sendpage_ok()"
>> +
>> +. tests/md/rc
>> +. common/brd
>> +. common/nvme
>> +
>> +DESCRIPTION="Raid with bitmap on tcp nvmet with opt-io-size over bitmap size"
>> +QUICK=1
>> +
>> +#restrict test to nvme-tcp only
>> +nvme_trtype=tcp
>> +nvmet_blkdev_type="device"
>> +
>> +requires() {
>> +    # Require dm-stripe
>> +    _have_program dmsetup
>> +    _have_driver dm-mod
>> +
>> +    _require_nvme_trtype tcp
>> +    _have_brd
>> +}
>> +
>> +# Sets up a brd device of 1G with optimal-io-size of 256K
>> +setup_underlying_device() {
>> +    if ! _init_brd rd_size=1048576 rd_nr=1; then
>> +        return 1
>> +    fi
>> +
>> +    dmsetup create ram0_big_optio --table \
>> +        "0 $(blockdev --getsz /dev/ram0) striped 1 512 /dev/ram0 0"
>> +}
>> +
>> +cleanup_underlying_device() {
>> +    dmsetup remove ram0_big_optio
>> +    _cleanup_brd
>> +}
>
> This is okay for now, however, it'll will be greate to add a common
> helper in rc, so that other test can reuse this in fulture.
>> +
>> +# Sets up a local host nvme over tcp
>> +setup_nvme_over_tcp() {
>> +    _setup_nvmet
>> +
>> +    local port
>> +    port="$(_create_nvmet_port "${nvme_trtype}")"
>> +
>> +    _create_nvmet_subsystem "${def_subsysnqn}" "/dev/mapper/ram0_big_optio" "${def_subsys_uuid}"
>> +    _add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
>> +
>> +    _create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}"
>> +
>> +    _nvme_connect_subsys
>> +}
>> +
>> +cleanup_nvme_over_tcp() {
>> +    _nvme_disconnect_subsys
>> +    _nvmet_target_cleanup --subsysnqn "${def_subsysnqn}"
>> +}
>> +
>> +test() {
>> +    echo "Running ${TEST_NAME}"
>> +
>> +    setup_underlying_device
>> +    setup_nvme_over_tcp
>> +
>> +    local ns
>> +    ns=$(_find_nvme_ns "${def_subsys_uuid}")
>> +
>> +    # Hangs here without the fix
>> +    mdadm --quiet --create /dev/md/blktests_md --level=1 --bitmap=internal \
>> +        --bitmap-chunk=1024K --assume-clean --run --raid-devices=2 \
>> +        /dev/"${ns}" missing
>
> Perhaps add raid1 to requires()?
Applied to v4

>> +
>> +    mdadm --quiet --stop /dev/md/blktests_md
>> +    cleanup_nvme_over_tcp
>> +    cleanup_underlying_device
>> +
>> +    echo "Test complete"
>> +}
>> diff --git a/tests/md/001.out b/tests/md/001.out
>> new file mode 100644
>> index 0000000..23071ec
>> --- /dev/null
>> +++ b/tests/md/001.out
>> @@ -0,0 +1,3 @@
>> +Running md/001
>> +disconnected 1 controller(s)
>> +Test complete
>> diff --git a/tests/md/rc b/tests/md/rc
>> new file mode 100644
>> index 0000000..d492579
>> --- /dev/null
>> +++ b/tests/md/rc
>> @@ -0,0 +1,12 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Ofir Gal
>> +#
>> +# Tests for md raid
>> +
>> +. common/rc
>> +
>> +group_requires() {
>> +    _have_root
>> +    _have_program mdadm
>> +}
>
> And md-mod here.
Applied to v4.

>
> It's nice to start adding md tests here.
>
> Thanks,
> Kuai
>>
>