Skip to content

nvme/068: add new test for nvme subsystem-reset test#229

Open
yizhanglinux wants to merge 1 commit intolinux-blktests:masterfrom
yizhanglinux:add-new-test-nvme-subsystem-reset
Open

nvme/068: add new test for nvme subsystem-reset test#229
yizhanglinux wants to merge 1 commit intolinux-blktests:masterfrom
yizhanglinux:add-new-test-nvme-subsystem-reset

Conversation

@yizhanglinux
Copy link
Copy Markdown
Contributor

No description provided.

@yizhanglinux
Copy link
Copy Markdown
Contributor Author

Here is the ouput of the test:

# ./check nvme/068
nvme/068 => nvme0n1 (Test NVMe subsystem-reset command)      [passed]
    runtime  35.304s  ...  35.134s
nvme/068 => nvme1n1 (Test NVMe subsystem-reset command)      [not run]
    /dev/nvme1n1 doesn't support subsystem-reset operation
nvme/068 => nvme2n1 (Test NVMe subsystem-reset command)      [passed]
    runtime  35.427s  ...  35.379s
nvme/068 => nvme3n1 (Test NVMe subsystem-reset command)      [not run]
    /dev/nvme3n1 doesn't support subsystem-reset operation
nvme/068 => nvme4n1 (Test NVMe subsystem-reset command)      [passed]
    runtime  35.169s  ...  35.240s
nvme/068 => nvme5n1 (Test NVMe subsystem-reset command)      [passed]
    runtime  46.028s  ...  45.451s

@yizhanglinux
Copy link
Copy Markdown
Contributor Author

@shroffni

Comment thread tests/nvme/rc
}

_require_test_dev_support_subsystem_reset() {
if ! nvme show-regs "$TEST_DEV" -H | grep -q "NSSRS.*Yes"; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment on the already closed PR. :)

nvme show-regs is likely not to work: https://github.com/linux-nvme/nvme-cli/wiki/FAQ#nvme-show-regs-devnvme0-returns-nvme0-failed-to-map

I think this is something the kernel needs to expose via the sysfs interface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just tried one kernel with CONFIG_IO_STRICT_DEVMEM enabled, and the cmd failed.

# nvme show-regs /dev/nvme0
NVMe status: Invalid Command Opcode: A reserved coded value or an unsupported value in the command opcode field(0x4001)
# cat /boot/config-7.0.0-0.rc2.21.fc45.x86_64 | grep IO_STRI
CONFIG_IO_STRICT_DEVMEM=y

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we could do is add a sysfs entry which exposes all the registers and nvme show-regs uses these when evailable otherwise it falls back to the raw memory access.

Comment thread tests/nvme/068
fi

# Wait NVMe disk reinitialized
sleep 10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least on PPC platform, adding delay of sleep 10 would not work. As we know, running subsystem-reset command on PPC platform would cause the communication loss to the NVMe adapter. So on PPC system any I/Os those were running while susbsytem-reset is executed or any new I/O submitted after the subsystem-reset is executed would eventually times out after 30 seconds. The nvme timeout handler code would then attempt to read PCIe/MMIO config space register which triggers the EEH and then EEH would recover the communication link to the NVMe adapter. So in theory, in worst case, it would take more than 30 seconds for the link to be restored and device to be back online post subsystem-reset.

I'd suggest following steps:

  1. Start I/O (maybe using fio)
  2. execute nvme subsystem-reset
  3. Add sleep 35 (30 seconds for I/O request timeout plus additional 5 seconds as cushion)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even better you may first retrieve timeout value from, /sys/block/<blk-dev>/queue/io_timeout (which is in ms) and then accordingly adjust sleep.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to set the default timeout to something short like 5s to avoid tests just waiting for timeouts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants