Hi,
On Thu, Sep 08, 2022 at 12:02:24AM +0000, Shinichiro Kawasaki wrote:
> Thanks for the patch. First of all, the commit title says nvme/045, but it adds
> nvme/046. Also I made a couple of comments on this patch in line.
Ah it's a stupid copy&past error. Will update it accordingly.
> I ran the added test case and observed test case failure [1] and KASAN
> slab-out-of-bounds [2]. To run this test case, I applied this patch on top of
> the PR #100 [3] and used kernel at nvme-6.1 branch tip 500e781dc0b0 at
> "git://git.infradead.org/nvme.git". Is this failure expected? Or do I miss
> any required set up to pass the test case?
No, the crash you observed is not expected. I'll look into it. I suspect
it has something to do with
'/sys/class/nvme-fabrics/ctl//state: No such file or directory'
>> > +requires() {
> > + _nvme_requires
> > + _have_loop
> > + _require_nvme_trtype_is_fabrics
> > + _require_min_cpus 2
>
> From my curiosity, what's the reason to require 2 cpus?
The number of CPUs define how many queues we will setup or request from
the target. As this tests starts with the default queue counts requested
by the host and then limits the queues count to 1 on the target side we
need to have more than 1 queue requested initially. I think it's
worthwhile to add this as comment to the test.
> > +_set_nvmet_attr_qid_max() {
> > + local nvmet_subsystem="$1"
> > + local qid_max="$2"
> > + local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> > +
> > + if [[ -f "${cfs_path}/attr_qid_max" ]]; then
> > + echo $qid_max > "${cfs_path}/attr_qid_max"
>
> I ran 'make check' and noticed the line above triggers a shellcheck warning:
>
> tests/nvme/rc:553:8: note: Double quote to prevent globbing and
> word splitting. [SC2086]
Will fix it.
> [1] test case failure messages
>
> $ sudo ./check nvme/046
> nvme/046 (Test queue count changes on reconnect) [failed]
> runtime 88.104s ... 87.687s
> --- tests/nvme/046.out 2022-09-08 08:35:02.063595059 +0900
> +++ /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/046.out.bad 2022-09-08 08:43:54.524174409 +0900
> @@ -1,3 +1,86 @@
> Running nvme/046
> -NQN:blktests-subsystem-1 disconnected 1 controller(s)
> +grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
> +grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
> +grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
> +grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
> +grep: /sys/class/nvme-fabrics/ctl//state: No such file or directory
> ...
> (Run 'diff -u tests/nvme/046.out /home/shin/kts/kernel-test-suite/src/blktests/results/nodev/nvme/046.out.bad' to see the entire diff)
>
> [2] KASAN: slab-out-of-bounds
>
> [ 151.315742] run blktests nvme/046 at 2022-09-08 08:42:26
> [ 151.834816] nvmet: adding nsid 1 to subsystem blktests-feature-detect
> [ 152.170966] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
> [ 152.514592] nvmet: creating nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:.
> [ 152.522907] nvme nvme6: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.
> [ 152.527164] nvme nvme6: creating 4 I/O queues.
> [ 152.533543] nvme nvme6: new ctrl: "blktests-subsystem-1"
> [ 154.339129] nvme nvme6: Removing ctrl: NQN "blktests-subsystem-1"
> [ 175.599995] ==================================================================
> [ 175.601755] BUG: KASAN: slab-out-of-bounds in nvmet_subsys_attr_qid_max_store+0x13d/0x160 [nvmet]
> [ 175.603816] Read of size 1 at addr ffff8881138dc450 by task check/946
>
> [ 175.605801] CPU: 1 PID: 946 Comm: check Not tainted 6.0.0-rc2+ #3
> [ 175.607232] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
> [ 175.609735] Call Trace:
> [ 175.610412] <TASK>
> [ 175.611039] dump_stack_lvl+0x5b/0x77
> [ 175.612016] print_report.cold+0x5e/0x602
> [ 175.612999] ? nvmet_subsys_attr_qid_max_store+0x13d/0x160 [nvmet]
Hmm, as qid_max is more or less a copy of existing attributes it might
not the only attribute store operation which has this problem.
Thanks for the review!
Daniel
