Re: [PATCH blktests 0/2] add nvme test for creating sleep while atomic kernel BUG

Shinichiro Kawasaki
2024-12-03T08:26:23Z

CC: Jirong,

On Dec 03, 2024 / 11:08, Nilay Shroff wrote:
>
>
> On 11/30/24 14:40, Shinichiro Kawasaki wrote:
> > On Nov 29, 2024 / 13:31, Nilay Shroff wrote:
> >> Hi,
> >>
> >> There're two patches in this series. The first patch is a preparation patch
> >> for reusing a common function nvmf_wait_for_ns from multiple nvme test scripts.
> >> The second patch adds a new nvme regression[1] test for commit 505363957fad
> >> ("nvmet: fix nvme status code when namespace is disabled").
> >
> > Hi Nilay, thank your very much for the fix actions. Much appreciated.
> >
> > I tried these blktests patches with the kernel just before the commit
> > 505363957fad, at the git hash 6825bdde4434. I expected the new test case fails,
> > but it passes. I increased the number of iterations from 10 to 100, but it still
> > passes. Do you observe the test case failure on your test systems?
> If you ran blktests at git hash 6825bdde4434 then that doesn't include ns changes
> which uses mutex_lock. So it's expected that the test wouldn't fail. Did you try
> running test using latest upstream kernel or checking out tree at commit
> 505363957fad ("nvmet: fix nvme status code when namespace is disabled")?

Yes, I see the test case creates "BUG: sleeping function called from invalid
context". Now I see that you added this new test case nvme/055 to recreate the
BUG. However, I observed the BUG at the first place with nvme/052, which can be
used to confirm the BUG fixed. So it does not look so meaningful to add the new
test case.

I think my report "blktests failures with v6.12 kernel [X]" confused you. I
wrote "It is desired to have a better fix and the test case to confirm it.", but
I should have wrote "It is desired to have a better fix and the test case to
confirm that the fix does not break the intent of the trigger commit
505363957fad." Please find the discussion about how to test the fix patch [Y].
The question was: how to "confirm that the commit 505363957fad achieves its
purpose" ?.

[X] https://lore.kernel.org/linux-block/6crydkodszx5vq4ieox3jjpwkxtu7mhbohypy24awlo5w7f4k6@to3dcng24rd4/
[Y] https://lore.kernel.org/linux-nvme/20241023052042.GB1341@lst.de/

Jirong,

I found your Tested-by tag for the commit 505363957fad. Could you share how did
you test the commit? It will help for us to have more confidence on Nilay's
patch.