CC+: linux-nvme, Daniel, Chaitanya,
On Jun 17, 2024 / 19:05, Ofir Gal wrote:
[...]
>
> >> diff --git a/tests/md/001 b/tests/md/001
> >> new file mode 100755
> >> index 0000000..d5fb755
> >> --- /dev/null
> >> +++ b/tests/md/001
> >> @@ -0,0 +1,80 @@
> >> +#!/bin/bash
> >> +# SPDX-License-Identifier: GPL-3.0+
> >> +# Copyright (C) 2024 Ofir Gal
> >> +#
> >> +# 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
> >> +. tests/nvme/rc
> > I want to avoid cross references acoss test groups. So far, all test groups do
> > not have any cross reference to keep them independent. How about to add this
> > test case to the nvme test group?
> I don't mind to add it to the nvme test group, just to clarify the test
> checks a bug in md. The bug is "visible" only when the underlying device
> of the raid is a network block device that utilize MSG_SPLICE_PAGES.
Good to know this background. I suggest to add the last sentence above to the
test case script header comment.
>
> nvme-tcp is used as the network device, I'm not sure it's related to
> the nvme test group. What do you think?
I see... The bug is in md sub-system, then it's the better to have the new test
case in the new md test group. To avoid the cross reference, the nvmet related
helper functions should move from tests/nvme/rc to common/nvmet, so that this
test/md/001 can refer them. This will be another separated, preparation patch.
To nvme experts: If you have comments on this, please chime in.
>
> >> +. common/brd
> >> +
> >> +DESCRIPTION="Create a raid with bitmap on top of nvme device with
> >> +optimal-io-size over bitmap size"
> > This descrption is printed as blktests runs. All other blktests have single line
> > description then the two lines description looks strange. Can we make it shorter
> > to fit in one line?
> Yes, does "Raid with bitmap on nvme device with opt-io-size over bitmap
> size" sounds good?
The word "tcp" sounds important. And the word "nvmet" sounds better than "nvme".
So how about: "Raid with bitmap on tcp nvmet with opt-io-size over bitmap size"?
>
> >> +test() {
> >> + echo "Running ${TEST_NAME}"
> >> +
> >> + setup_underlying_device
> >> + setup_nvme_over_tcp
> >> +
> >> + # Hangs here without the fix
> >> + mdadm -q --create /dev/md/blktests_md --level=1 --bitmap=internal \
> > In the past, short options caused some trouble. I suggest to use the long option
> > "--quiet" in place of the short option "-q".
> Applied to v2.
>
