| Message ID | 20250210075812.3900646-4-hayashi.kunihiko@socionext.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | Fix some issues related to an interrupt type in pci_endpoint_test | expand |
On Mon, Feb 10, 2025 at 04:58:10PM +0900, Kunihiko Hayashi wrote: > There are two variables that indicate the interrupt type to be used > in the next test execution, "irq_type" as global and test->irq_type. > > The global is referenced from pci_endpoint_test_get_irq() to preserve > the current type for ioctl(PCITEST_GET_IRQTYPE). > > The type set in this function 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" as follows: > > # pcitest -i 0 > SET IRQ TYPE TO LEGACY: OKAY > # pcitest -I > GET IRQ TYPE: MSI > > Fix this issue by propagating the current type to the global "irq_type". > > Cc: stable@vger.kernel.org > Fixes: b2ba9225e031 ("misc: pci_endpoint_test: Avoid using module parameter to determine irqtype") > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > --- > drivers/misc/pci_endpoint_test.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > index f13fa32ef91a..6a0972e7674f 100644 > --- a/drivers/misc/pci_endpoint_test.c > +++ b/drivers/misc/pci_endpoint_test.c > @@ -829,6 +829,7 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test, > return ret; > } > > + irq_type = test->irq_type; It feels a bit silly to add this line, when you remove this exact line in the next patch. Perhaps just drop this patch? Kind regards, Niklas
Hi Niklas, On 2025/02/11 1:01, Niklas Cassel wrote: > On Mon, Feb 10, 2025 at 04:58:10PM +0900, Kunihiko Hayashi wrote: >> There are two variables that indicate the interrupt type to be used >> in the next test execution, "irq_type" as global and test->irq_type. >> >> The global is referenced from pci_endpoint_test_get_irq() to preserve >> the current type for ioctl(PCITEST_GET_IRQTYPE). >> >> The type set in this function 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" as follows: >> >> # pcitest -i 0 >> SET IRQ TYPE TO LEGACY: OKAY >> # pcitest -I >> GET IRQ TYPE: MSI >> >> Fix this issue by propagating the current type to the global "irq_type". >> >> Cc: stable@vger.kernel.org >> Fixes: b2ba9225e031 ("misc: pci_endpoint_test: Avoid using module > parameter to determine irqtype") >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> >> --- >> drivers/misc/pci_endpoint_test.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/misc/pci_endpoint_test.c > b/drivers/misc/pci_endpoint_test.c >> index f13fa32ef91a..6a0972e7674f 100644 >> --- a/drivers/misc/pci_endpoint_test.c >> +++ b/drivers/misc/pci_endpoint_test.c >> @@ -829,6 +829,7 @@ static int pci_endpoint_test_set_irq(struct > pci_endpoint_test *test, >> return ret; >> } >> >> + irq_type = test->irq_type; > > It feels a bit silly to add this line, when you remove this exact line in > the next patch. Perhaps just drop this patch? This fix will be removed in patch 4/5, so it seems no means. However, there is an issue in the stable version, as mentioned in the commit message, so I fix it here. I'll treat it separately if necessary. Thank you, --- Best Regards Kunihiko Hayashi
On Thu, Feb 13, 2025 at 07:21:45PM +0900, Kunihiko Hayashi wrote: > Hi Niklas, > > On 2025/02/11 1:01, Niklas Cassel wrote: > > On Mon, Feb 10, 2025 at 04:58:10PM +0900, Kunihiko Hayashi wrote: > > > There are two variables that indicate the interrupt type to be used > > > in the next test execution, "irq_type" as global and test->irq_type. > > > > > > The global is referenced from pci_endpoint_test_get_irq() to preserve > > > the current type for ioctl(PCITEST_GET_IRQTYPE). > > > > > > The type set in this function 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" as follows: > > > > > > # pcitest -i 0 > > > SET IRQ TYPE TO LEGACY: OKAY > > > # pcitest -I > > > GET IRQ TYPE: MSI > > > > > > Fix this issue by propagating the current type to the global "irq_type". > > > > > > Cc: stable@vger.kernel.org > > > Fixes: b2ba9225e031 ("misc: pci_endpoint_test: Avoid using module > > parameter to determine irqtype") > > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > > > --- > > > drivers/misc/pci_endpoint_test.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/misc/pci_endpoint_test.c > > b/drivers/misc/pci_endpoint_test.c > > > index f13fa32ef91a..6a0972e7674f 100644 > > > --- a/drivers/misc/pci_endpoint_test.c > > > +++ b/drivers/misc/pci_endpoint_test.c > > > @@ -829,6 +829,7 @@ static int pci_endpoint_test_set_irq(struct > > pci_endpoint_test *test, > > > return ret; > > > } > > > + irq_type = test->irq_type; > > > > It feels a bit silly to add this line, when you remove this exact line in > > the next patch. Perhaps just drop this patch? > > This fix will be removed in patch 4/5, so it seems no means. > However, there is an issue in the stable version, as mentioned in the > commit message, so I fix it here. Ok. Having a small fix first that can be backported, which is then followed by a bigger cleanup, makes sense in this case. Reviewed-by: Niklas Cassel <cassel@kernel.org>
On Mon, Feb 10, 2025 at 04:58:10PM +0900, Kunihiko Hayashi wrote: > There are two variables that indicate the interrupt type to be used > in the next test execution, "irq_type" as global and test->irq_type. > > The global is referenced from pci_endpoint_test_get_irq() to preserve > the current type for ioctl(PCITEST_GET_IRQTYPE). > > The type set in this function 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" as follows: > > # pcitest -i 0 > SET IRQ TYPE TO LEGACY: OKAY > # pcitest -I > GET IRQ TYPE: MSI > Could you please post the failure with kselftest that got merged into v6.14-rc1? > Fix this issue by propagating the current type to the global "irq_type". > > Cc: stable@vger.kernel.org > Fixes: b2ba9225e031 ("misc: pci_endpoint_test: Avoid using module parameter to determine irqtype") > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > --- > drivers/misc/pci_endpoint_test.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c > index f13fa32ef91a..6a0972e7674f 100644 > --- a/drivers/misc/pci_endpoint_test.c > +++ b/drivers/misc/pci_endpoint_test.c > @@ -829,6 +829,7 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test, > return ret; > } > > + irq_type = test->irq_type; > return 0; > } > > -- > 2.25.1 >
Hi Manivannan, On 2025/02/15 2:25, Manivannan Sadhasivam wrote: > On Mon, Feb 10, 2025 at 04:58:10PM +0900, Kunihiko Hayashi wrote: >> There are two variables that indicate the interrupt type to be used >> in the next test execution, "irq_type" as global and test->irq_type. >> >> The global is referenced from pci_endpoint_test_get_irq() to preserve >> the current type for ioctl(PCITEST_GET_IRQTYPE). >> >> The type set in this function 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" as follows: >> >> # pcitest -i 0 >> SET IRQ TYPE TO LEGACY: OKAY >> # pcitest -I >> GET IRQ TYPE: MSI >> > > Could you please post the failure with kselftest that got merged into > v6.14-rc1? The kselftest doesn't call GET_IRQTYPE, so current kselftest doesn't fail. If necessary, I can add GET_IRQTYPE test after SET_IRQTYPE of each interrupt test prior to this patch. pci_ep_ioctl(PCITEST_SET_IRQTYPE, 0); ASSERT_EQ(0, ret) TH_LOG("Can't set Legacy IRQ type"); + pci_ep_ioctl(PCITEST_GET_IRQTYPE, 0); + ASSERT_EQ(0, ret) TH_LOG("Can't get Legacy IRQ type"); However, pci_ep_ioctl() returns zero if OK, the return value should be changed to the actual return value. #define pci_ep_ioctl(cmd, arg) \ ({ \ ret = ioctl(self->fd, cmd, arg); \ - ret = ret < 0 ? -errno : 0; \ + ret = ret < 0 ? -errno : ret; \ }) Before applying the patch, this test fails. # RUN pci_ep_basic.LEGACY_IRQ_TEST ... # pci_endpoint_test.c:104:LEGACY_IRQ_TEST:Expected 0 (0) == ret (1) # pci_endpoint_test.c:104:LEGACY_IRQ_TEST:Can't get Legacy IRQ type # LEGACY_IRQ_TEST: Test terminated by assertion # FAIL pci_ep_basic.LEGACY_IRQ_TEST Thank you, --- Best Regards Kunihiko Hayashi
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c index f13fa32ef91a..6a0972e7674f 100644 --- a/drivers/misc/pci_endpoint_test.c +++ b/drivers/misc/pci_endpoint_test.c @@ -829,6 +829,7 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test, return ret; } + irq_type = test->irq_type; return 0; }
There are two variables that indicate the interrupt type to be used in the next test execution, "irq_type" as global and test->irq_type. The global is referenced from pci_endpoint_test_get_irq() to preserve the current type for ioctl(PCITEST_GET_IRQTYPE). The type set in this function 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" as follows: # pcitest -i 0 SET IRQ TYPE TO LEGACY: OKAY # pcitest -I GET IRQ TYPE: MSI Fix this issue by propagating the current type to the global "irq_type". Cc: stable@vger.kernel.org Fixes: b2ba9225e031 ("misc: pci_endpoint_test: Avoid using module parameter to determine irqtype") Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> --- drivers/misc/pci_endpoint_test.c | 1 + 1 file changed, 1 insertion(+)