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

[Bjorn Helgaas <bhelgaas@google.com>]

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.

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

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

Bjorn