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

[Sahil Chandna <sahilchandna@linux.microsoft.com>]

On 28-08-2026 20:29, Hamza Mahfooz wrote:
> On Fri, Aug 28, 2026 at 11:10:22AM +0530, Naman Jain wrote:
>>
>>
>> On 8/27/2026 11:04 PM, Saurabh Singh Sengar wrote:
>>> On Thu, Aug 27, 2026 at 04:07:06PM +0000, Michael Kelley wrote:
>>>> From: Long Li <longli@microsoft.com> Sent: Tuesday, August 25, 2026 10:01 AM
>>>>
>>>> [snip]
>>>>
>>>>>>
>>>>>> Basically something like this:
>>>>>>
>>>>>> #define PCI_RESPONSE_WARN_TIMEOUT_SEC 300
>>>>>>
>>>>>> static int wait_for_response(struct hv_device *hdev,
>>>>>>                    struct completion *comp) {
>>>>>>       unsigned long warn_at =
>>>>>>           jiffies + secs_to_jiffies(PCI_RESPONSE_WARN_TIMEOUT_SEC);
>>>>>>       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 (warned || time_after_eq(jiffies, warn_at))
>>>>>>                   dev_warn(&hdev->device,
>>>>>>                        "PCI response received after prolonged wait.\n");
>>>>>>               return 0;
>>>>>>           }
>>>>>>
>>>>>>           if (!warned && time_after_eq(jiffies, warn_at)) {
>>>>>>               dev_warn(&hdev->device,
>>>>>>                    "PCI still waiting for response.\n");
>>>>>>               warned = true;
>>>>>>           }
>>>>>>       }
>>>>>> }
>>>>>>
>>>>>> Regards,
>>>>>> Naman
>>>>>
>>>>> This looks better.
>>>>>
>>>>
>>>> I like getting the "response received" message if the response
>>>> eventually does come in. It's a judgment call, but I would be OK
>>>> with outputting the "still waiting" message after each wait interval
>>>> rather than doing it only once. And I would make the interval smaller
>>>> than 300 seconds. Five minutes is a long time to wait and wonder
>>>> what's going on when things are hung. 60 or 120 seconds would
>>>> be OK -- a line in dmesg every 1 or 2 minutes doesn't seem like
>>>> spamming to me when something is fundamentally broken.
>>>> And probably don't expect a VM in this broken state to keep running
>>>> for hours -- the sysadmin or automatic monitoring software will
>>>> reboot it to get it working again.
>>
>> I think this is a main point of this discussion. FWIK, this is not
>> automatically recovered as of now, ever. With these changes, we could get
>> dmesg logs completely filled with this same log, which does not add any
>> value. On the other hand, I was suggesting Sahil if this problem is not
>> recoverable by Azure fabric layer or other monitoring services, and is
>> extremely rare and the VM is unusable, perhaps we should consider adding a
>> bug/timeout in this path instead of just logging about it.
>>
>> Regards,
>> Naman
> 
Thanks everyone for review, based on dmesg logs recovered from affected
VMs this does introduce non-recoverable hang for hours, I am going to
work on this in a patch along with fixing sashiko warnings.

> Along with what everyone else has suggested I would suggest printing out the
> state of the device's VMBus channel (i.e. the child_relid and inbound and
> outbound status in particular). Since, that information is useful to the hv
> guys and should help disambiguate between "host never picked it up" vs. 
> "host picked it up but never replied."
> 
> Hamza
> 
Ack.
>>
>>>
>>> I will also vote for repeated message as long as interval between two
>>> messages is greater than 60 seconds.
>>>
>>> - Saurabh
>>>
>>>>
>>>> Just my $.02. Outputting the "still waiting" message only once is
>>>> also OK. Your call.
>>>>
>>>> Michael
>>
>>

I am going to send next version to log "stuck waiting for response"
warning only once at 2 mins boundary and will also log late completion
as per feedback.
Regards,
Sahil