Re: [PATCH] PCI: Make reset warning messages different

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Wed, Oct 15, 2014 at 09:43:32PM -0600, Bjorn Helgaas wrote:
>On Wed, Oct 15, 2014 at 6:19 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> On Wed, Oct 15, 2014 at 05:48:35PM -0600, Bjorn Helgaas wrote:
>>>On Wed, Oct 15, 2014 at 5:41 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>>> We have same warning message for FLR and AF FLR and users can't
>>>> know which type of resets the PCI device is taking when there are
>>>> pending transactions. The patch makes them different for FLR and
>>>> AF FLR cases.
>>>>
>>>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>>> ---
>>>>  drivers/pci/pci.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>> index 625a4ac..2d708cd 100644
>>>> --- a/drivers/pci/pci.c
>>>> +++ b/drivers/pci/pci.c
>>>> @@ -3144,7 +3144,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>>>>                 return 0;
>>>>
>>>>         if (!pci_wait_for_pending_transaction(dev))
>>>> -               dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
>>>> +               dev_err(&dev->dev, "Force FLR with pending transaction\n");
>>>>
>>>>         pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
>>>>
>>>> @@ -3178,7 +3178,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe)
>>>>                                  PCI_AF_STATUS_TP << 8))
>>>>                 goto clear;
>>>>
>>>> -       dev_err(&dev->dev, "transaction is not cleared; proceeding with reset anyway\n");
>>>> +       dev_err(&dev->dev, "Force AF FLR with pending transaction\n");
>>>
>>>Making the text different is fine, but I don't think "FLR" and "AF
>>>FLR" are meaningful except to extremely technical people.  So I think
>>>"reset" needs to stay spelled out in the message.
>>>
>>
>> Agree, it's worthy to keep "reset". How about something like this:
>>
>>    "Force function level reset with pending transaction"         - FLR
>>    "Force AF function level reset with pending transaction"      - AF FLR
>>
>> If above messages look good to you, I'll send out another revision.
>
>How about something like "timed out waiting for pending transaction;
>performing function level reset"?
>
>"Force reset with pending transaction" sounds like a pending
>transaction might be the mechanism we're using to perform the reset.
>

Yep, I'll change according to your suggestion.

>Out of curiosity, is there some issue you tripped over where it's
>important for users to know this difference?
>

I saw this message when issuing FLR (not AF case) to (EEH) frozen device.
Since 0xFF's is always returned from the frozen device, this message was
surely printed. However, I didn't know it was FLR or AF-FLR from the
original message.

>Just FYI, I'll be on vacation the rest of this week.
>

Ok. Have a good vacation.

Thanks,
Gavin

>Bjorn
>