| Message ID | 20190712235742.22646-1-logang@deltatee.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix nvme block test issues | expand |
Oh, I forgot to mention there is a git branch of these patches available here: https://github.com/Eideticom/blktests nvme_fixes Logan
Thanks Logan for doing this. I am overall okay with overall with these changes. Once you post V2 I'll test it on my machine. On 07/12/2019 04:58 PM, Logan Gunthorpe wrote: > Hi, > > This patchset cleans up a number of issues and pain points > I've had with getting the nvme blktests to pass and run cleanly. > > The first three patches are meant to fix the Generation Counter > issue that's been discussed before but hasn't been fixed in months. > I primarily use a slightly fixed up patch posted by Michael Moese[1] > but add another patch to properly test the Generation Counter that > would no longer be tested otherwise. > > I've also taken it a step further to filter out more of the discovery > information so that we are less fragile against churn and less dependant > on the version of nvme-cli in use. This should also fix the issue discussed > in [2]. > > Patches 4 through 7 fix a number of smaller issues I've found with > individual tests. > > Patches 8 through 10 implement a system to ensure the nvme tests > clean themselves up properly especially when ctrl-c is used to > interrupt a test (working with the tests before this was a huge > pain seeing, when you ctrl-c, you have to either manually clean > up the nvmet configuration or reboot to get the system in a state > where it's sane to run the tests again). > > Patches 11 and 12 make some minor changes that allow running the > tests with the nvme modules built into the kernel. > > With these patches, plus a couple I've sent to the nvme list for the > kernel, I can consistently pass the entire nvme suite with the > exception of the lockdep false-positive with nvme/012 that still > seems to be in a bit of contention[3]. > > Thanks, > > Logan > > [1] https://github.com/osandov/blktests/pull/34 > [2] https://lore.kernel.org/linux-block/20190505150611.15776-4-minwoo.im.dev@gmail.com/ > [3] https://lore.kernel.org/lkml/20190214230058.196511-22-bvanassche@acm.org/ > > -- > > Logan Gunthorpe (11): > nvme: More agressively filter the discovery output > nvme: Add new test to verify the generation counter > nvme/003,004: Add missing calls to nvme disconnect > nvme/005: Don't rely on modprobing to set the multipath paramater > nvme/015: Ensure the namespace is flushed not the char device > nvme/018: Ignore error message generated by nvme read > check: Add the ability to call a cleanup function after a test ends > nvme: Cleanup modprobe lines into helper functions > nvme: Ensure all ports and subsystems are removed on cleanup > common: Use sysfs instead of modinfo for _have_module_param() > nvme: Ignore errors when removing modules > > Michael Moese (1): > Add filter function for nvme discover > > check | 9 + > common/rc | 18 +- > tests/nvme/002 | 10 +- > tests/nvme/002.out | 6003 +------------------------------------------- > tests/nvme/003 | 6 +- > tests/nvme/003.out | 1 + > tests/nvme/004 | 6 +- > tests/nvme/004.out | 1 + > tests/nvme/005 | 16 +- > tests/nvme/006 | 6 +- > tests/nvme/007 | 6 +- > tests/nvme/008 | 6 +- > tests/nvme/009 | 5 +- > tests/nvme/010 | 6 +- > tests/nvme/011 | 6 +- > tests/nvme/012 | 6 +- > tests/nvme/013 | 6 +- > tests/nvme/014 | 6 +- > tests/nvme/015 | 5 +- > tests/nvme/016 | 6 +- > tests/nvme/016.out | 9 +- > tests/nvme/017 | 8 +- > tests/nvme/017.out | 9 +- > tests/nvme/018 | 8 +- > tests/nvme/019 | 6 +- > tests/nvme/020 | 5 +- > tests/nvme/021 | 6 +- > tests/nvme/022 | 6 +- > tests/nvme/023 | 6 +- > tests/nvme/024 | 6 +- > tests/nvme/025 | 6 +- > tests/nvme/026 | 6 +- > tests/nvme/027 | 6 +- > tests/nvme/028 | 6 +- > tests/nvme/029 | 6 +- > tests/nvme/030 | 72 + > tests/nvme/030.out | 2 + > tests/nvme/rc | 64 + > 38 files changed, 208 insertions(+), 6163 deletions(-) > create mode 100755 tests/nvme/030 > create mode 100644 tests/nvme/030.out > > -- > 2.17.1 >
On Fri, Jul 12, 2019 at 05:57:32PM -0600, Logan Gunthorpe wrote: > Comparing the entire output of nvme-cli for discovery is fragile > and error prone as things change. There's already been the > long standing issue of the generation counter mismatching > and also some versions of nvme-cli print an extra "sq flow control > disable supported" text[1]. > > Instead, filter out all but a few key values from the discovery > text which should still be sufficient for this test and much > less likely to be subject to churn. > > [1] https://lore.kernel.org/linux-block/20190505150611.15776-4-minwoo.im.dev@gmail.com/ > > Signed-off-by: Logan Gunthorpe <logang@deltatee.com> > --- > tests/nvme/002.out | 6001 -------------------------------------------- > tests/nvme/016.out | 7 - > tests/nvme/017.out | 7 - > tests/nvme/rc | 4 +- > 4 files changed, 2 insertions(+), 6017 deletions(-) [snip] > diff --git a/tests/nvme/rc b/tests/nvme/rc > index 22833d8ef9bb..60dc05869726 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -120,6 +120,6 @@ _find_nvme_loop_dev() { > } > > _filter_discovery() { > - sed -r -e "s/portid: [0-9]+/portid: X/" \ > - -e "s/Generation counter [0-9]+/Generation counter X/" > + sed -r -e "s/Generation counter [0-9]+/Generation counter X/" | > + grep 'Discovery Log Number\|Log Entry\|trtype\|subnqn' > } This can be done in a single sed command instead of sed + grep: sed -rn -e 's/Generation counter [0-9]+/Generation counter X/' \ -e '/Discovery Log Number|Log Entry|trtype|subnqn/p'
On Fri, Jul 12, 2019 at 05:57:30PM -0600, Logan Gunthorpe wrote: > Hi, > > This patchset cleans up a number of issues and pain points > I've had with getting the nvme blktests to pass and run cleanly. > > The first three patches are meant to fix the Generation Counter > issue that's been discussed before but hasn't been fixed in months. > I primarily use a slightly fixed up patch posted by Michael Moese[1] > but add another patch to properly test the Generation Counter that > would no longer be tested otherwise. > > I've also taken it a step further to filter out more of the discovery > information so that we are less fragile against churn and less dependant > on the version of nvme-cli in use. This should also fix the issue discussed > in [2]. > > Patches 4 through 7 fix a number of smaller issues I've found with > individual tests. > > Patches 8 through 10 implement a system to ensure the nvme tests > clean themselves up properly especially when ctrl-c is used to > interrupt a test (working with the tests before this was a huge > pain seeing, when you ctrl-c, you have to either manually clean > up the nvmet configuration or reboot to get the system in a state > where it's sane to run the tests again). > > Patches 11 and 12 make some minor changes that allow running the > tests with the nvme modules built into the kernel. > > With these patches, plus a couple I've sent to the nvme list for the > kernel, I can consistently pass the entire nvme suite with the > exception of the lockdep false-positive with nvme/012 that still > seems to be in a bit of contention[3]. > > Thanks, > > Logan > > [1] https://github.com/osandov/blktests/pull/34 > [2] https://lore.kernel.org/linux-block/20190505150611.15776-4-minwoo.im.dev@gmail.com/ > [3] https://lore.kernel.org/lkml/20190214230058.196511-22-bvanassche@acm.org/ > > -- > > Logan Gunthorpe (11): > nvme: More agressively filter the discovery output > nvme: Add new test to verify the generation counter > nvme/003,004: Add missing calls to nvme disconnect > nvme/005: Don't rely on modprobing to set the multipath paramater > nvme/015: Ensure the namespace is flushed not the char device > nvme/018: Ignore error message generated by nvme read > check: Add the ability to call a cleanup function after a test ends > nvme: Cleanup modprobe lines into helper functions > nvme: Ensure all ports and subsystems are removed on cleanup > common: Use sysfs instead of modinfo for _have_module_param() > nvme: Ignore errors when removing modules > > Michael Moese (1): > Add filter function for nvme discover > > check | 9 + > common/rc | 18 +- > tests/nvme/002 | 10 +- > tests/nvme/002.out | 6003 +------------------------------------------- > tests/nvme/003 | 6 +- > tests/nvme/003.out | 1 + > tests/nvme/004 | 6 +- > tests/nvme/004.out | 1 + > tests/nvme/005 | 16 +- > tests/nvme/006 | 6 +- > tests/nvme/007 | 6 +- > tests/nvme/008 | 6 +- > tests/nvme/009 | 5 +- > tests/nvme/010 | 6 +- > tests/nvme/011 | 6 +- > tests/nvme/012 | 6 +- > tests/nvme/013 | 6 +- > tests/nvme/014 | 6 +- > tests/nvme/015 | 5 +- > tests/nvme/016 | 6 +- > tests/nvme/016.out | 9 +- > tests/nvme/017 | 8 +- > tests/nvme/017.out | 9 +- > tests/nvme/018 | 8 +- > tests/nvme/019 | 6 +- > tests/nvme/020 | 5 +- > tests/nvme/021 | 6 +- > tests/nvme/022 | 6 +- > tests/nvme/023 | 6 +- > tests/nvme/024 | 6 +- > tests/nvme/025 | 6 +- > tests/nvme/026 | 6 +- > tests/nvme/027 | 6 +- > tests/nvme/028 | 6 +- > tests/nvme/029 | 6 +- > tests/nvme/030 | 72 + > tests/nvme/030.out | 2 + > tests/nvme/rc | 64 + > 38 files changed, 208 insertions(+), 6163 deletions(-) > create mode 100755 tests/nvme/030 > create mode 100644 tests/nvme/030.out Thanks for cleaning this up! I replied with one nitpick, and besides that and comments from the other reviewers, I'm happy with it overall (assuming it passes shellcheck).
On 2019-07-15 5:14 p.m., Omar Sandoval wrote: > > Thanks for cleaning this up! I replied with one nitpick, and besides > that and comments from the other reviewers, I'm happy with it overall > (assuming it passes shellcheck). Thanks, yes my sed skills are obviously not as good as yours. I'll fix that nit up for v2 which I'll send in a couple days. I have run "make check" on the series and it does pass. Logan