From: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Sent: Tuesday, March 26, 2019 12:55 PM On Mon, Mar 04, 2019 at 09:34:49PM +0000, Dexuan Cui wrote:
When we hot-remove a device, usually the host sends us a PCI_EJECT
message,
and a PCI_BUS_RELATIONS message with bus_rel->device_count == 0. But
when
we do the quick hot-add/hot-remove test, the host may not send us the PCI_EJECT message, if the guest has not fully finished the initialization by sending the PCI_RESOURCES_ASSIGNED* message to the host, so it's potentially unsafe to only depend on the pci_destroy_slot() in hv_eject_device_work(), though create_root_hv_pci_bus() -> hv_pci_assign_slots() is not called in this case. Note: in this case, the host still sends the guest a PCI_BUS_RELATIONS message with bus_rel->device_count == 0.
And, in the quick hot-add/hot-remove test, we can have such a race: before pci_devices_present_work() -> new_pcichild_device() adds the new device into hbus->children, we may have already received the PCI_EJECT message, and hence the taklet handler hv_pci_onchannelcallback() may fail to find the "hpdev" by get_pcichild_wslot(hbus, dev_message->wslot.slot), so hv_pci_eject_device() is NOT called; later create_root_hv_pci_bus() -> hv_pci_assign_slots() creates the slot, and the PCI_BUS_RELATIONS message with bus_rel->device_count == 0 removes the device from hbus->children,
and
we end up being unable to remove the slot in hv_pci_remove() -> hv_pci_remove_slots().
The patch removes the slot in pci_devices_present_work() when the device is removed. This can address the above race. Note 1: pci_devices_present_work() and hv_eject_device_work() run in the singled-threaded hbus->wq, so there is not a double-remove issue for the slot. Note 2: we can't offload hv_pci_eject_device() from hv_pci_onchannelcallback() to the workqueue, because we need hv_pci_onchannelcallback() synchronously call hv_pci_eject_device() to poll the channel's ringbuffer to work around the "hangs in hv_compose_msi_msg()" issue: see commit de0aa7b2f97d ("PCI: hv: Fix 2 hang issues in
hv_compose_msi_msg()")
This commit log is unreadable, sorry. Indentation, punctuation and formatting are just a mess, try to read it, you will notice by yourself.
I basically reformatted it completely and pushed the series to pci/controller-fixes but that's the last time I do it since I am not an editor, next time I won't merge it.
Hi Lorenzo, Thank you for helping improve my changelogs! I did learn a lot after carefully comparing the improved version with my original version. :-)
I'll try my best to write a good changelog for my future patches.
More importantly, these patches are marked for stable, given the series of fixes that triggered this series please ensure it was tested thoroughly because it is honestly complicate to understand and I do not want to backport further fixes to stable kernels on top of this.
I did the hot-add/hot-remove test in a loop for several thousand times, and the patchset worked as expected and didn't show any issue.
Please have a look and report back.
Thanks, Lorenzo
Thanks again!
Thanks, -- Dexuan