2020-05-22 18:43:44

by Petr Tesařík

[permalink] [raw]
Subject: [PATCH 1/1] s390/pci: Log new handle in clp_disable_fh()

After disabling a function, the original handle is logged instead of
the disabled handle.

Fixes: 17cdec960cf77 (s390/pci: Recover handle in clp_set_pci_fn())
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/s390/pci/pci_clp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index ea794ae755ae..179bcecefdee 100644
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -309,14 +309,13 @@ int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as)

int clp_disable_fh(struct zpci_dev *zdev)
{
- u32 fh = zdev->fh;
int rc;

if (!zdev_enabled(zdev))
return 0;

rc = clp_set_pci_fn(zdev, 0, CLP_SET_DISABLE_PCI_FN);
- zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, fh, rc);
+ zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc);
return rc;
}

--
2.26.1


2020-05-28 09:26:05

by Petr Tesařík

[permalink] [raw]
Subject: Re: [PATCH 1/1] s390/pci: Log new handle in clp_disable_fh()

Hi all,

just a gentle ping.

If the current behaviour (logging the original handle) was intended,
then it was worth mentioning in the commit message for 17cdec960cf77,
which made the change, but since that's no longer an option, I'd be
happy with an explanation in email.

Petr T

On Fri, 22 May 2020 20:39:22 +0200
Petr Tesarik <[email protected]> wrote:

> After disabling a function, the original handle is logged instead of
> the disabled handle.
>
> Fixes: 17cdec960cf77 (s390/pci: Recover handle in clp_set_pci_fn())
> Signed-off-by: Petr Tesarik <[email protected]>
> ---
> arch/s390/pci/pci_clp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
> index ea794ae755ae..179bcecefdee 100644
> --- a/arch/s390/pci/pci_clp.c
> +++ b/arch/s390/pci/pci_clp.c
> @@ -309,14 +309,13 @@ int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as)
>
> int clp_disable_fh(struct zpci_dev *zdev)
> {
> - u32 fh = zdev->fh;
> int rc;
>
> if (!zdev_enabled(zdev))
> return 0;
>
> rc = clp_set_pci_fn(zdev, 0, CLP_SET_DISABLE_PCI_FN);
> - zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, fh, rc);
> + zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc);
> return rc;
> }
>


Attachments:
(No filename) (499.00 B)
Digitální podpis OpenPGP

2020-05-28 10:06:44

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH 1/1] s390/pci: Log new handle in clp_disable_fh()


On 2020-05-28 11:08, Petr Tesarik wrote:
> Hi all,
>
> just a gentle ping.
>
> If the current behaviour (logging the original handle) was intended,
> then it was worth mentioning in the commit message for 17cdec960cf77,
> which made the change, but since that's no longer an option, I'd be
> happy with an explanation in email.
>
> Petr T
>
> On Fri, 22 May 2020 20:39:22 +0200
> Petr Tesarik <[email protected]> wrote:
>
>> After disabling a function, the original handle is logged instead of
>> the disabled handle.

Hi Petr,

Sorry for the delay, no doubt, you are right, the fh in zpci_dbg is the
old one and we should use the one in the zdev struct.

Thanks,
Pierre

Reviewed-by: Pierre Morel <[email protected]>


>>
>> Fixes: 17cdec960cf77 (s390/pci: Recover handle in clp_set_pci_fn())
>> Signed-off-by: Petr Tesarik <[email protected]>
>> ---
>> arch/s390/pci/pci_clp.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
>> index ea794ae755ae..179bcecefdee 100644
>> --- a/arch/s390/pci/pci_clp.c
>> +++ b/arch/s390/pci/pci_clp.c
>> @@ -309,14 +309,13 @@ int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as)
>>
>> int clp_disable_fh(struct zpci_dev *zdev)
>> {
>> - u32 fh = zdev->fh;
>> int rc;
>>
>> if (!zdev_enabled(zdev))
>> return 0;
>>
>> rc = clp_set_pci_fn(zdev, 0, CLP_SET_DISABLE_PCI_FN);
>> - zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, fh, rc);
>> + zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc);
>> return rc;
>> }
>>
>

--
Pierre Morel
IBM Lab Boeblingen

2020-05-28 10:10:17

by Vasily Gorbik

[permalink] [raw]
Subject: Re: [PATCH 1/1] s390/pci: Log new handle in clp_disable_fh()

On Thu, May 28, 2020 at 12:01:45PM +0200, Pierre Morel wrote:
>
> On 2020-05-28 11:08, Petr Tesarik wrote:
> > Hi all,
> >
> > just a gentle ping.
> >
> > If the current behaviour (logging the original handle) was intended,
> > then it was worth mentioning in the commit message for 17cdec960cf77,
> > which made the change, but since that's no longer an option, I'd be
> > happy with an explanation in email.
> >
> > Petr T
> >
> > On Fri, 22 May 2020 20:39:22 +0200
> > Petr Tesarik <[email protected]> wrote:
> >
> > > After disabling a function, the original handle is logged instead of
> > > the disabled handle.
>
> Hi Petr,
>
> Sorry for the delay, no doubt, you are right, the fh in zpci_dbg is the old
> one and we should use the one in the zdev struct.
>
> Thanks,
> Pierre
>
> Reviewed-by: Pierre Morel <[email protected]>

Applied, thanks

2020-06-02 07:28:43

by Niklas Schnelle

[permalink] [raw]
Subject: Re: [PATCH 1/1] s390/pci: Log new handle in clp_disable_fh()

Hi Petr,

sorry for not reacting sooner, I was on holiday, saw your message but figured it wasn't super critical.
Thanks for finding this issue and thank you Pierre and Vasily for stepping in.

Best,
Niklas Schnelle

On 5/28/20 11:08 AM, Petr Tesarik wrote:
> Hi all,
>
> just a gentle ping.
>
> If the current behaviour (logging the original handle) was intended,
> then it was worth mentioning in the commit message for 17cdec960cf77,
> which made the change, but since that's no longer an option, I'd be
> happy with an explanation in email.
>
> Petr T
>
> On Fri, 22 May 2020 20:39:22 +0200
> Petr Tesarik <[email protected]> wrote:
>
>> After disabling a function, the original handle is logged instead of
>> the disabled handle.
>>
>> Fixes: 17cdec960cf77 (s390/pci: Recover handle in clp_set_pci_fn())
>> Signed-off-by: Petr Tesarik <[email protected]>
>> ---
>> arch/s390/pci/pci_clp.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
>> index ea794ae755ae..179bcecefdee 100644
>> --- a/arch/s390/pci/pci_clp.c
>> +++ b/arch/s390/pci/pci_clp.c
>> @@ -309,14 +309,13 @@ int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as)
>>
>> int clp_disable_fh(struct zpci_dev *zdev)
>> {
>> - u32 fh = zdev->fh;
>> int rc;
>>
>> if (!zdev_enabled(zdev))
>> return 0;
>>
>> rc = clp_set_pci_fn(zdev, 0, CLP_SET_DISABLE_PCI_FN);
>> - zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, fh, rc);
>> + zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc);
>> return rc;
>> }
>>
>