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 <hamzamahf...@linux.microsoft.com>
Suggested-by: Naman Jain <namj...@linux.microsoft.com>
Suggested-by: Michael Kelley <mhkli...@outlook.com>

These two tags can be omitted IMO.

Signed-off-by: Sahil Chandna <sahilchan...@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- 
sahilchan...@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