> Subject: [PATCH] PCI: hv: add hard timeout to wait_for_response() > > It is possible that we never receive a rescind event, in which case we will > wait > indefinitely for a device that will never show up. So, assume a device is > gone if > have been polling for more than 5 seconds.
Can you explain in what situation we never receive a rescind event? If this for dealing a device unload when the vmbus is dead? Please provide more context. A kernel trace is helpful. Does this patch handle the case where the rescind event comes in right after the timeout? > > Cc: [email protected] > Fixes: c3635da2a336 ("PCI: hv: Do not wait forever on a device that has > disappeared") > Signed-off-by: Hamza Mahfooz <[email protected]> > --- > drivers/pci/controller/pci-hyperv.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci- > hyperv.c > index cfc8fa403dad..bd63efc4a210 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -52,6 +52,7 @@ > #include <linux/acpi.h> > #include <linux/sizes.h> > #include <linux/of_irq.h> > +#include <linux/jiffies.h> > #include <asm/mshyperv.h> > > /* > @@ -1038,6 +1039,8 @@ static void put_pcichild(struct hv_pci_dev *hpdev) > kfree(hpdev); > } > > +#define TIMEOUT_MS 5000 > + > /* > * 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. > @@ -1045,8 +1048,13 @@ static void put_pcichild(struct hv_pci_dev *hpdev) > static int wait_for_response(struct hv_device *hdev, > struct completion *comp) > { > + unsigned long timeout = get_jiffies_64() + > msecs_to_jiffies(TIMEOUT_MS); > + unsigned long now; > + > while (true) { > - if (hdev->channel->rescind) { > + now = get_jiffies_64(); > + if (hdev->channel->rescind || > + time_after(now, timeout)) { What if the VMBUS response comes in right after this check? The completion is allocated on the caller stack, and it will cause kernel OOP. How do you test this patch?
