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

[Naman Jain]

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@xxxxxxxxxxxxxxxxxxx>
> > Suggested-by: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx>
> > Suggested-by: Michael Kelley <mhklinux@xxxxxxxxxxx>
>
> These two tags can be omitted IMO.
>
> > Signed-off-by: Sahil Chandna <sahilchandna@xxxxxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxx/
> > ---
> >   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