From: Wei Hu <weh@xxxxxxxxxxxxx> Sent: Monday, April 27, 2020 12:06 AM > > -----Original Message----- > > From: Michael Kelley <mikelley@xxxxxxxxxxxxx> > > > @@ -3136,7 +3166,7 @@ static int hv_pci_probe(struct hv_device *hdev, > > > > > > ret = hv_pci_allocate_bridge_windows(hbus); > > > if (ret) > > > - goto free_irq_domain; > > > + goto exit_d0; > > > > > > ret = hv_send_resources_allocated(hdev); > > > if (ret) > > > > The above is good. But there's another error case that isn't handled > > correctly. If create_root_hv_pci_bus() fails, hv_send_resources_released() > > should be called. > > > > Fixing these two error cases should probably go in a separate patch: One > > patch for the retry problem in kdump, and a separate patch for these error > > cases. > > [Wei Hu] > hv_send_resources_released() is called in the added hv_pci_bus_exit(). > If hv_send_resources_allocated() fails, is it correct to call hv_send_resources_released()? > Allocation can fail in the middle. So I am not sure if calling hv_send_resources_released() > won't cause any side effect. > Ah yes, you are right. But that brings up a separate problem. If hv_pci_allocate_bridge_windows() or hv_send_resources_allocated() fails, then the error path will call hv_pci_bus_exit(), which will call hv_send_resources_released(), even if hv_send_resources_allocated() was never called or didn't fully succeed. As you noted, hv_send_resources_allocated() does multiple steps, some of which might have succeeded, and some of which didn't. The mismatch might cause problems. That means fixing this error handling is going to be a bit more complex. Each operation needs to be individually undone, and only if it previously succeeded. Could you follow up with the Hyper-V people to see if there's a problem with doing the RESOURCES_RELEASED message on a slot where RESOURCES_ASSIGNED was not done or wasn't successful? If doing a spurious RESOURCES_RELEASED is harmless, that will make the error cleanup easier. Michael
