[blktests,00/12] Fix nvme block test issues
mbox series

Message ID 20190712235742.22646-1-logang@deltatee.com
Headers show
Series
  • Fix nvme block test issues
Related show

Message

Logan Gunthorpe July 12, 2019, 11:57 p.m. UTC
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

Comments

Logan Gunthorpe July 13, 2019, 12:01 a.m. UTC | #1
Oh, I forgot to mention there is a git branch of these patches available
here:

https://github.com/Eideticom/blktests nvme_fixes

Logan
Chaitanya Kulkarni July 15, 2019, 5:14 p.m. UTC | #2
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
>
Omar Sandoval July 15, 2019, 11:07 p.m. UTC | #3
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'
Omar Sandoval July 15, 2019, 11:14 p.m. UTC | #4
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).
Logan Gunthorpe July 15, 2019, 11:16 p.m. UTC | #5
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