Re: [RESEND PATCH v1] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Kunihiko Hayashi]

Hi Bjorn,

Thank you for your comment.

On 2025/01/17 0:04, Bjorn Helgaas wrote:
> On Thu, Jan 16, 2025 at 11:41:45AM +0900, Kunihiko Hayashi wrote:
> > There are two variables that indicate the interrupt type to be used
> > in the next test execution, global "irq_type" and test->irq_type.
> >
> > The former is referenced from pci_endpoint_test_get_irq() to preserve
> > the current type for ioctl(PCITEST_GET_IRQTYPE).
> >
> > In pci_endpoint_test_request_irq(), since this global variable is
> > referenced when an error occurs, the unintended error message is
> > displayed.
>
> Apparently this test fails (with an error message) when it shouldn't?
> Please include the error message here. >
> "... since this global variable is referenced ..." is not quite enough
> explanation of how this causes a spurious test failure or under what
> circumstances the failure occurs.

I see. This failure path is taken if devm_request_irq() returns with
an error. The message that causes an error in the third interrupt is for
example:

  pci-endpoint-test 0000:01:00.0: Failed to request IRQ 30 for MSI 3

Before this fix, this message will show "MSI" even if the current irq
type was "legacy" or "MSI-X".

I'll add such that description in the commit.

And I discovered that not releasing interrupt correctly caused WARN()
after displaying this message. I'll fix the issue additionally.

> > And the type set in pci_endpoint_test_set_irq() isn't reflected in
> > the global "irq_type", so ioctl(PCITEST_GET_IRQTYPE) returns the
> previous
> > type. As a result, the wrong type will be displayed in "pcitest".
>
> The global "irq_type" seems a little suspect.  Is it possible to run
> multiple tests concurrently?  If so, is this usage safe from races?

The global "irq_type" is only changed in pci_endpoint_test_set_irq().
I think this function is protected by the ioctl's mutex even if running
multiple tests.

Thank you,

---
Best Regards
Kunihiko Hayashi