Re: [PATCH v2] PCI: hv: warn when wait_for_response() waits indefinitely

[Naman Jain <namjain@linux.microsoft.com>]

On 9/3/2026 2:31 PM, Naman Jain wrote:
> 
> 
> On 9/2/2026 5:28 PM, Sahil Chandna wrote:
>> A guest can wait indefinitely in wait_for_response() for the host to
>> send either a rescind message or a packet completion. If the
>> host does not send either, the guest can remain blocked with no
>> diagnostic indicating a reason.
>> This was observed during a guest kernel upgrade in which the
>> host-side application handling the PCI channel faulted, causing the
>> guest to never receive the completion request.
>> Add a warning in wait_for_response() when the wait exceeds
>> a timeout so that such a hang is visible in the guest's kernel log
>> and can be correlated with host-side state.
>>
>> Suggested-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
>> Suggested-by: Naman Jain <namjain@linux.microsoft.com>
>> Suggested-by: Michael Kelley <mhklinux@outlook.com>
> 
> These two tags can be omitted IMO.
> 
>> Signed-off-by: Sahil Chandna <sahilchandna@linux.microsoft.com>
>> ---
>> Changes since v1:
>> - Removed periodic warning to one time warning in 2 minutes
>> - Include vmbus relid and stuck PCI msg.
>> Link to v1: https://lore.kernel.org/all/20260825051850.2438816-1- 
>> sahilchandna@linux.microsoft.com/
>> ---
>>   drivers/pci/controller/pci-hyperv.c | 48 +++++++++++++++++++++++------
>>   1 file changed, 38 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/ 
>> controller/pci-hyperv.c
>> index 89816a2bd7cd..ca9d8efd748c 100644
>> --- a/drivers/pci/controller/pci-hyperv.c
>> +++ b/drivers/pci/controller/pci-hyperv.c
>> @@ -1040,19 +1040,40 @@ static void put_pcichild(struct hv_pci_dev 
>> *hpdev)
>>
>>   /*
>>    * There is no good way to get notified from vmbus_onoffer_rescind(),
>> - * so let's use polling here, since this is not a hot path.
>> + * so let's use polling here, since this is not a hot path. If
>> + * wait_for_response() has been polling for 
>> PCI_RESPONSE_HANG_TIMEOUT_SEC
>> + * without either a rescind or completion, add a warning.
>>    */
>> +#define PCI_RESPONSE_HANG_TIMEOUT_SEC 120
>> +
>>   static int wait_for_response(struct hv_device *hdev,
>> -                 struct completion *comp)
>> +                 struct completion *comp,
>> +                 const char *msg_type)
>>   {
>> +    unsigned long delay = 
>> secs_to_jiffies(PCI_RESPONSE_HANG_TIMEOUT_SEC);
>> +    u64 timeout = get_jiffies_64() + delay;
>> +    bool warned = false;
>> +
>>       while (true) {
>>           if (hdev->channel->rescind) {
>>               dev_warn_once(&hdev->device, "The device is gone.\n");
>>               return -ENODEV;
>>           }
>>
>> -        if (wait_for_completion_timeout(comp, HZ / 10))
>> +        if (wait_for_completion_timeout(comp, HZ / 10)) {
>> +            if (warned)
>> +                dev_warn(&hdev->device,
>> +                     "Late %s completion arrived.\n", msg_type);
>>               break;
>> +        }
>> +
>> +        if (!warned && time_after64(get_jiffies_64(), timeout)) {
>> +            dev_err(&hdev->device,
>> +                "%s stuck waiting for response, relid = %u\n",
>> +                msg_type, hdev->channel->offermsg.child_relid);
>> +
>> +            warned = true;
>> +        }
>>       }
>>
>>       return 0;
>> @@ -1518,7 +1539,8 @@ static int hv_read_config_block(struct pci_dev 
>> *pdev, void *buf,
>>       if (ret)
>>           return ret;
>>
>> -    ret = wait_for_response(hbus->hdev, &comp_pkt.comp_pkt.host_event);
>> +    ret = wait_for_response(hbus->hdev, &comp_pkt.comp_pkt.host_event,
>> +                "PCI_READ_BLOCK");
> 
> Instead of hard-coding this msg type two times everywhere, would it be 
> better to simply pass corresponding variable.message_type.type?

You can ignore this, as you may have wanted to print actual name instead 
of enum value.

Regards,
Naman