1. We don't really need such a big on-stack buffer when sending
the teardown_packet: vmbus_sendpacket() here only uses
sizeof(struct pci_message).
2. In the hot-remove case (PCI_EJECT), after we send PCI_EJECTION_COMPLETE
to the host, the host will send a RESCIND_CHANNEL message to us and the
host won't access the per-channel ringbuffer any longer, so we needn't
send PCI_RESOURCES_RELEASED/PCI_BUS_D0EXIT to the host, and we shouldn't
expect the host's completion message of PCI_BUS_D0EXIT, which will never
come.
3. We should send PCI_BUS_D0EXIT after hv_send_resources_released().
Signed-off-by: Dexuan Cui <[email protected]>
CC: Jake Oshins <[email protected]>
Cc: KY Srinivasan <[email protected]>
CC: Haiyang Zhang <[email protected]>
CC: Vitaly Kuznetsov <[email protected]>
---
I made the patch based on discussions with Jake Oshins and others.
drivers/pci/host/pci-hyperv.c | 53 +++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 93ed64a..7590ad0 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -2266,24 +2266,32 @@ static int hv_pci_probe(struct hv_device *hdev,
return ret;
}
-/**
- * hv_pci_remove() - Remove routine for this VMBus channel
- * @hdev: VMBus's tracking struct for this root PCI bus
- *
- * Return: 0 on success, -errno on failure
- */
-static int hv_pci_remove(struct hv_device *hdev)
+static void hv_pci_bus_exit(struct hv_device *hdev)
{
- int ret;
- struct hv_pcibus_device *hbus;
- union {
+ struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
+ struct {
struct pci_packet teardown_packet;
- u8 buffer[0x100];
+ u8 buffer[sizeof(struct pci_message)];
} pkt;
struct pci_bus_relations relations;
struct hv_pci_compl comp_pkt;
+ int ret;
- hbus = hv_get_drvdata(hdev);
+ /*
+ * After the host sends the RESCIND_CHANNEL message, it doesn't
+ * access the per-channel ringbuffer any longer.
+ */
+ if (hdev->channel->rescind)
+ return;
+
+ /* Delete any children which might still exist. */
+ memset(&relations, 0, sizeof(relations));
+ hv_pci_devices_present(hbus, &relations);
+
+ ret = hv_send_resources_released(hdev);
+ if (ret)
+ dev_err(&hdev->device,
+ "Couldn't send resources released packet(s)\n");
memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet));
init_completion(&comp_pkt.host_event);
@@ -2298,7 +2306,19 @@ static int hv_pci_remove(struct hv_device *hdev)
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
if (!ret)
wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ);
+}
+
+/**
+ * hv_pci_remove() - Remove routine for this VMBus channel
+ * @hdev: VMBus's tracking struct for this root PCI bus
+ *
+ * Return: 0 on success, -errno on failure
+ */
+static int hv_pci_remove(struct hv_device *hdev)
+{
+ struct hv_pcibus_device *hbus;
+ hbus = hv_get_drvdata(hdev);
if (hbus->state == hv_pcibus_installed) {
/* Remove the bus from PCI's point of view. */
pci_lock_rescan_remove();
@@ -2307,17 +2327,10 @@ static int hv_pci_remove(struct hv_device *hdev)
pci_unlock_rescan_remove();
}
- ret = hv_send_resources_released(hdev);
- if (ret)
- dev_err(&hdev->device,
- "Couldn't send resources released packet(s)\n");
+ hv_pci_bus_exit(hdev);
vmbus_close(hdev->channel);
- /* Delete any children which might still exist. */
- memset(&relations, 0, sizeof(relations));
- hv_pci_devices_present(hbus, &relations);
-
iounmap(hbus->cfg_addr);
hv_free_config_window(hbus);
pci_free_resource_list(&hbus->resources_for_children);
--
2.7.4
> -----Original Message-----
> From: Dexuan Cui
> Sent: Wednesday, November 9, 2016 11:19 PM
> To: Bjorn Helgaas <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; KY Srinivasan <[email protected]>;
> Haiyang Zhang <[email protected]>; Stephen Hemminger
> <[email protected]>; Jake Oshins <[email protected]>; Hadden
> Hoppert <[email protected]>; Vitaly Kuznetsov
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: [PATCH 2/3] PCI: hv: fix hv_pci_remove() for hot-remove
>
> 1. We don't really need such a big on-stack buffer when sending
> the teardown_packet: vmbus_sendpacket() here only uses
> sizeof(struct pci_message).
>
> 2. In the hot-remove case (PCI_EJECT), after we send
> PCI_EJECTION_COMPLETE
> to the host, the host will send a RESCIND_CHANNEL message to us and the
> host won't access the per-channel ringbuffer any longer, so we needn't
> send PCI_RESOURCES_RELEASED/PCI_BUS_D0EXIT to the host, and we
> shouldn't
> expect the host's completion message of PCI_BUS_D0EXIT, which will never
> come.
>
> 3. We should send PCI_BUS_D0EXIT after hv_send_resources_released().
>
> Signed-off-by: Dexuan Cui <[email protected]>
> CC: Jake Oshins <[email protected]>
> Cc: KY Srinivasan <[email protected]>
> CC: Haiyang Zhang <[email protected]>
> CC: Vitaly Kuznetsov <[email protected]>
>
> ---
>
> I made the patch based on discussions with Jake Oshins and others.
>
> drivers/pci/host/pci-hyperv.c | 53 +++++++++++++++++++++++++++--------
> --------
> 1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 93ed64a..7590ad0 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -2266,24 +2266,32 @@ static int hv_pci_probe(struct hv_device *hdev,
> return ret;
> }
>
> -/**
> - * hv_pci_remove() - Remove routine for this VMBus channel
> - * @hdev: VMBus's tracking struct for this root PCI bus
> - *
> - * Return: 0 on success, -errno on failure
> - */
> -static int hv_pci_remove(struct hv_device *hdev)
> +static void hv_pci_bus_exit(struct hv_device *hdev)
> {
> - int ret;
> - struct hv_pcibus_device *hbus;
> - union {
> + struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
> + struct {
> struct pci_packet teardown_packet;
> - u8 buffer[0x100];
> + u8 buffer[sizeof(struct pci_message)];
> } pkt;
> struct pci_bus_relations relations;
> struct hv_pci_compl comp_pkt;
> + int ret;
>
> - hbus = hv_get_drvdata(hdev);
> + /*
> + * After the host sends the RESCIND_CHANNEL message, it doesn't
> + * access the per-channel ringbuffer any longer.
> + */
> + if (hdev->channel->rescind)
> + return;
> +
> + /* Delete any children which might still exist. */
> + memset(&relations, 0, sizeof(relations));
> + hv_pci_devices_present(hbus, &relations);
> +
> + ret = hv_send_resources_released(hdev);
> + if (ret)
> + dev_err(&hdev->device,
> + "Couldn't send resources released packet(s)\n");
>
> memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet));
> init_completion(&comp_pkt.host_event);
> @@ -2298,7 +2306,19 @@ static int hv_pci_remove(struct hv_device *hdev)
>
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> if (!ret)
> wait_for_completion_timeout(&comp_pkt.host_event, 10 *
> HZ);
> +}
> +
> +/**
> + * hv_pci_remove() - Remove routine for this VMBus channel
> + * @hdev: VMBus's tracking struct for this root PCI bus
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int hv_pci_remove(struct hv_device *hdev)
> +{
> + struct hv_pcibus_device *hbus;
>
> + hbus = hv_get_drvdata(hdev);
> if (hbus->state == hv_pcibus_installed) {
> /* Remove the bus from PCI's point of view. */
> pci_lock_rescan_remove();
> @@ -2307,17 +2327,10 @@ static int hv_pci_remove(struct hv_device
> *hdev)
> pci_unlock_rescan_remove();
> }
>
> - ret = hv_send_resources_released(hdev);
> - if (ret)
> - dev_err(&hdev->device,
> - "Couldn't send resources released packet(s)\n");
> + hv_pci_bus_exit(hdev);
>
> vmbus_close(hdev->channel);
>
> - /* Delete any children which might still exist. */
> - memset(&relations, 0, sizeof(relations));
> - hv_pci_devices_present(hbus, &relations);
> -
> iounmap(hbus->cfg_addr);
> hv_free_config_window(hbus);
> pci_free_resource_list(&hbus->resources_for_children);
> --
> 2.7.4
This looks fine to me.
-- Jake Oshins
> -----Original Message-----
> From: Dexuan Cui
> Sent: Wednesday, November 9, 2016 11:19 PM
> To: Bjorn Helgaas <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; KY Srinivasan <[email protected]>;
> Haiyang Zhang <[email protected]>; Stephen Hemminger
> <[email protected]>; Jake Oshins <[email protected]>; Hadden
> Hoppert <[email protected]>; Vitaly Kuznetsov
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: [PATCH 2/3] PCI: hv: fix hv_pci_remove() for hot-remove
>
> 1. We don't really need such a big on-stack buffer when sending
> the teardown_packet: vmbus_sendpacket() here only uses
> sizeof(struct pci_message).
>
> 2. In the hot-remove case (PCI_EJECT), after we send
> PCI_EJECTION_COMPLETE
> to the host, the host will send a RESCIND_CHANNEL message to us and the
> host won't access the per-channel ringbuffer any longer, so we needn't
> send PCI_RESOURCES_RELEASED/PCI_BUS_D0EXIT to the host, and we
> shouldn't
> expect the host's completion message of PCI_BUS_D0EXIT, which will never
> come.
>
> 3. We should send PCI_BUS_D0EXIT after hv_send_resources_released().
>
> Signed-off-by: Dexuan Cui <[email protected]>
> CC: Jake Oshins <[email protected]>
> Cc: KY Srinivasan <[email protected]>
> CC: Haiyang Zhang <[email protected]>
> CC: Vitaly Kuznetsov <[email protected]>
Thanks Dexuan.
Acked-by: K. Y. Srinivasan <[email protected]>
>
> ---
>
> I made the patch based on discussions with Jake Oshins and others.
>
> drivers/pci/host/pci-hyperv.c | 53 +++++++++++++++++++++++++++--------
> --------
> 1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 93ed64a..7590ad0 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -2266,24 +2266,32 @@ static int hv_pci_probe(struct hv_device *hdev,
> return ret;
> }
>
> -/**
> - * hv_pci_remove() - Remove routine for this VMBus channel
> - * @hdev: VMBus's tracking struct for this root PCI bus
> - *
> - * Return: 0 on success, -errno on failure
> - */
> -static int hv_pci_remove(struct hv_device *hdev)
> +static void hv_pci_bus_exit(struct hv_device *hdev)
> {
> - int ret;
> - struct hv_pcibus_device *hbus;
> - union {
> + struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
> + struct {
> struct pci_packet teardown_packet;
> - u8 buffer[0x100];
> + u8 buffer[sizeof(struct pci_message)];
> } pkt;
> struct pci_bus_relations relations;
> struct hv_pci_compl comp_pkt;
> + int ret;
>
> - hbus = hv_get_drvdata(hdev);
> + /*
> + * After the host sends the RESCIND_CHANNEL message, it doesn't
> + * access the per-channel ringbuffer any longer.
> + */
> + if (hdev->channel->rescind)
> + return;
> +
> + /* Delete any children which might still exist. */
> + memset(&relations, 0, sizeof(relations));
> + hv_pci_devices_present(hbus, &relations);
> +
> + ret = hv_send_resources_released(hdev);
> + if (ret)
> + dev_err(&hdev->device,
> + "Couldn't send resources released packet(s)\n");
>
> memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet));
> init_completion(&comp_pkt.host_event);
> @@ -2298,7 +2306,19 @@ static int hv_pci_remove(struct hv_device *hdev)
>
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> if (!ret)
> wait_for_completion_timeout(&comp_pkt.host_event, 10 *
> HZ);
> +}
> +
> +/**
> + * hv_pci_remove() - Remove routine for this VMBus channel
> + * @hdev: VMBus's tracking struct for this root PCI bus
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int hv_pci_remove(struct hv_device *hdev)
> +{
> + struct hv_pcibus_device *hbus;
>
> + hbus = hv_get_drvdata(hdev);
> if (hbus->state == hv_pcibus_installed) {
> /* Remove the bus from PCI's point of view. */
> pci_lock_rescan_remove();
> @@ -2307,17 +2327,10 @@ static int hv_pci_remove(struct hv_device
> *hdev)
> pci_unlock_rescan_remove();
> }
>
> - ret = hv_send_resources_released(hdev);
> - if (ret)
> - dev_err(&hdev->device,
> - "Couldn't send resources released packet(s)\n");
> + hv_pci_bus_exit(hdev);
>
> vmbus_close(hdev->channel);
>
> - /* Delete any children which might still exist. */
> - memset(&relations, 0, sizeof(relations));
> - hv_pci_devices_present(hbus, &relations);
> -
> iounmap(hbus->cfg_addr);
> hv_free_config_window(hbus);
> pci_free_resource_list(&hbus->resources_for_children);
> --
> 2.7.4