[PATCH v3 1/5] misc: pci_endpoint_test: Avoid issue of interrupts remaining after request_irq error

[PATCH v3 1/5] misc: pci_endpoint_test: Avoid issue of interrupts remaining after request_irq error

[Kunihiko Hayashi]

After devm_request_irq() fails with error in
pci_endpoint_test_request_irq(), pci_endpoint_test_free_irq_vectors() is
called assuming that all IRQs have been released.

However some requested IRQs remain unreleased, so there are still
/proc/irq/* entries remaining and we encounters WARN() with the following
message:

    remove_proc_entry: removing non-empty directory 'irq/30', leaking at
    least 'pci-endpoint-test.0'
    WARNING: CPU: 0 PID: 202 at fs/proc/generic.c:719 remove_proc_entry
    +0x190/0x19c

And show the call trace that led to this issue:

    [   12.050005] Call trace:
    [   12.051226]  remove_proc_entry+0x190/0x19c (P)
    [   12.053448]  unregister_irq_proc+0xd0/0x104
    [   12.055541]  free_desc+0x4c/0xd0
    [   12.057155]  irq_free_descs+0x68/0x90
    [   12.058984]  irq_domain_free_irqs+0x15c/0x1bc
    [   12.061161]  msi_domain_free_locked.part.0+0x184/0x1d4
    [   12.063728]  msi_domain_free_irqs_all_locked+0x64/0x8c
    [   12.066296]  pci_msi_teardown_msi_irqs+0x48/0x54
    [   12.068604]  pci_free_msi_irqs+0x18/0x38
    [   12.070564]  pci_free_irq_vectors+0x64/0x8c
    [   12.072654]  pci_endpoint_test_ioctl+0x870/0x1068
    [   12.075006]  __arm64_sys_ioctl+0xb0/0xe8
    [   12.076967]  invoke_syscall+0x48/0x110
    [   12.078841]  el0_svc_common.constprop.0+0x40/0xe8
    [   12.081192]  do_el0_svc+0x20/0x2c
    [   12.082848]  el0_svc+0x30/0xd0
    [   12.084376]  el0t_64_sync_handler+0x144/0x168
    [   12.086553]  el0t_64_sync+0x198/0x19c
    [   12.088383] ---[ end trace 0000000000000000 ]---

To solve this issue, set the number of remaining IRQs to test->num_irqs
and release IRQs in advance by calling pci_endpoint_test_release_irq().

Cc: stable@vger.kernel.org
Fixes: e03327122e2c ("pci_endpoint_test: Add 2 ioctl commands")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/misc/pci_endpoint_test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index d5ac71a49386..bbcccd425700 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -259,6 +259,9 @@ static int pci_endpoint_test_request_irq(struct pci_endpoint_test *test)
 		break;
 	}
 
+	test->num_irqs = i;
+	pci_endpoint_test_release_irq(test);
+
 	return ret;
 }
 
-- 
2.25.1

Re: [PATCH v3 1/5] misc: pci_endpoint_test: Avoid issue of interrupts remaining after request_irq error

[Manivannan Sadhasivam]

On Mon, Feb 10, 2025 at 04:58:08PM +0900, Kunihiko Hayashi wrote:
> After devm_request_irq() fails with error in
> pci_endpoint_test_request_irq(), pci_endpoint_test_free_irq_vectors() is
> called assuming that all IRQs have been released.
> 
> However some requested IRQs remain unreleased, so there are still
> /proc/irq/* entries remaining and we encounters WARN() with the following

s/we encounters/this results in WARN()

> message:
> 
>     remove_proc_entry: removing non-empty directory 'irq/30', leaking at
>     least 'pci-endpoint-test.0'
>     WARNING: CPU: 0 PID: 202 at fs/proc/generic.c:719 remove_proc_entry
>     +0x190/0x19c
> 
> And show the call trace that led to this issue:

You can remove this backtrace.

> 
>     [   12.050005] Call trace:
>     [   12.051226]  remove_proc_entry+0x190/0x19c (P)
>     [   12.053448]  unregister_irq_proc+0xd0/0x104
>     [   12.055541]  free_desc+0x4c/0xd0
>     [   12.057155]  irq_free_descs+0x68/0x90
>     [   12.058984]  irq_domain_free_irqs+0x15c/0x1bc
>     [   12.061161]  msi_domain_free_locked.part.0+0x184/0x1d4
>     [   12.063728]  msi_domain_free_irqs_all_locked+0x64/0x8c
>     [   12.066296]  pci_msi_teardown_msi_irqs+0x48/0x54
>     [   12.068604]  pci_free_msi_irqs+0x18/0x38
>     [   12.070564]  pci_free_irq_vectors+0x64/0x8c
>     [   12.072654]  pci_endpoint_test_ioctl+0x870/0x1068
>     [   12.075006]  __arm64_sys_ioctl+0xb0/0xe8
>     [   12.076967]  invoke_syscall+0x48/0x110
>     [   12.078841]  el0_svc_common.constprop.0+0x40/0xe8
>     [   12.081192]  do_el0_svc+0x20/0x2c
>     [   12.082848]  el0_svc+0x30/0xd0
>     [   12.084376]  el0t_64_sync_handler+0x144/0x168
>     [   12.086553]  el0t_64_sync+0x198/0x19c
>     [   12.088383] ---[ end trace 0000000000000000 ]---
> 
> To solve this issue, set the number of remaining IRQs to test->num_irqs
> and release IRQs in advance by calling pci_endpoint_test_release_irq().
> 
> Cc: stable@vger.kernel.org
> Fixes: e03327122e2c ("pci_endpoint_test: Add 2 ioctl commands")
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/misc/pci_endpoint_test.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index d5ac71a49386..bbcccd425700 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -259,6 +259,9 @@ static int pci_endpoint_test_request_irq(struct pci_endpoint_test *test)
>  		break;
>  	}
>  
> +	test->num_irqs = i;
> +	pci_endpoint_test_release_irq(test);
> +
>  	return ret;
>  }
>  
> -- 
> 2.25.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 1/5] misc: pci_endpoint_test: Avoid issue of interrupts remaining after request_irq error

[Kunihiko Hayashi]

Hi Manivannan,

On 2025/02/15 2:21, Manivannan Sadhasivam wrote:
> On Mon, Feb 10, 2025 at 04:58:08PM +0900, Kunihiko Hayashi wrote:
>> After devm_request_irq() fails with error in
>> pci_endpoint_test_request_irq(), pci_endpoint_test_free_irq_vectors() is
>> called assuming that all IRQs have been released.
>>
>> However some requested IRQs remain unreleased, so there are still
>> /proc/irq/* entries remaining and we encounters WARN() with the following
> 
> s/we encounters/this results in WARN()

I see. I'll fix it.

>> message:
>>
>>      remove_proc_entry: removing non-empty directory 'irq/30', leaking at
>>      least 'pci-endpoint-test.0'
>>      WARNING: CPU: 0 PID: 202 at fs/proc/generic.c:719 remove_proc_entry
>>      +0x190/0x19c
>>
>> And show the call trace that led to this issue:
> 
> You can remove this backtrace.

I'll add more information instead.

> 
>>
>>      [   12.050005] Call trace:
>>      [   12.051226]  remove_proc_entry+0x190/0x19c (P)
>>      [   12.053448]  unregister_irq_proc+0xd0/0x104
>>      [   12.055541]  free_desc+0x4c/0xd0
>>      [   12.057155]  irq_free_descs+0x68/0x90
>>      [   12.058984]  irq_domain_free_irqs+0x15c/0x1bc
>>      [   12.061161]  msi_domain_free_locked.part.0+0x184/0x1d4
>>      [   12.063728]  msi_domain_free_irqs_all_locked+0x64/0x8c
>>      [   12.066296]  pci_msi_teardown_msi_irqs+0x48/0x54
>>      [   12.068604]  pci_free_msi_irqs+0x18/0x38
>>      [   12.070564]  pci_free_irq_vectors+0x64/0x8c
>>      [   12.072654]  pci_endpoint_test_ioctl+0x870/0x1068
>>      [   12.075006]  __arm64_sys_ioctl+0xb0/0xe8
>>      [   12.076967]  invoke_syscall+0x48/0x110
>>      [   12.078841]  el0_svc_common.constprop.0+0x40/0xe8
>>      [   12.081192]  do_el0_svc+0x20/0x2c
>>      [   12.082848]  el0_svc+0x30/0xd0
>>      [   12.084376]  el0t_64_sync_handler+0x144/0x168
>>      [   12.086553]  el0t_64_sync+0x198/0x19c
>>      [   12.088383] ---[ end trace 0000000000000000 ]---
>>
>> To solve this issue, set the number of remaining IRQs to test->num_irqs
>> and release IRQs in advance by calling pci_endpoint_test_release_irq().
>>
>> Cc: stable@vger.kernel.org
>> Fixes: e03327122e2c ("pci_endpoint_test: Add 2 ioctl commands")
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thank you,

---
Best Regards
Kunihiko Hayashi

[PATCH v3 2/5] misc: pci_endpoint_test: Fix disyplaying irq_type after request_irq error

[Kunihiko Hayashi]

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.

For example, the following message shows "MSI 3" even if the current
irq type becomes "MSI-X".

    # pcitest -i 2
    pci-endpoint-test 0000:01:00.0: Failed to request IRQ 30 for MSI 3
    SET IRQ TYPE TO MSI-X:          NOT OKAY

Fix this issue by using test->irq_type instead of global "irq_type".

Cc: stable@vger.kernel.org
Fixes: b2ba9225e031 ("misc: pci_endpoint_test: Avoid using module parameter to determine irqtype")
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/misc/pci_endpoint_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index bbcccd425700..f13fa32ef91a 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -242,7 +242,7 @@ static int pci_endpoint_test_request_irq(struct pci_endpoint_test *test)
 	return 0;
 
 fail:
-	switch (irq_type) {
+	switch (test->irq_type) {
 	case IRQ_TYPE_INTX:
 		dev_err(dev, "Failed to request IRQ %d for Legacy\n",
 			pci_irq_vector(pdev, i));
-- 
2.25.1

[PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Kunihiko Hayashi]

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;
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Niklas Cassel]

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

Re: [PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Kunihiko Hayashi]

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

Re: [PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Niklas Cassel]

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>

Re: [PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Manivannan Sadhasivam]

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
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Kunihiko Hayashi]

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

Re: [PATCH v3 3/5] misc: pci_endpoint_test: Fix irq_type to convey the correct type

[Manivannan Sadhasivam]

On Mon, Feb 17, 2025 at 08:26:44PM +0900, Kunihiko Hayashi wrote:
> 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");
> 

Sure.

> 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;           \
>  })
> 

Ok.

> 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
> 

Looks good to me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்