| Message ID | 1417663477-9124-1-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, 2014-04-12 at 03:24:37 UTC, Wei Yang wrote: > As in commit ac205b7b (PCI: make sriov work with hotplug remove) indicates, > when removing pci devices on a bus which has VFs, we need to remove them in > the reverse order. > > This patch applies this pattern on the hotplug remove path on powerpc arch. So is this is a bug fix? Where/how have you tested this? cheers
On Thu, Dec 04, 2014 at 04:40:35PM +1100, Michael Ellerman wrote: >On Thu, 2014-04-12 at 03:24:37 UTC, Wei Yang wrote: >> As in commit ac205b7b (PCI: make sriov work with hotplug remove) indicates, >> when removing pci devices on a bus which has VFs, we need to remove them in >> the reverse order. >> >> This patch applies this pattern on the hotplug remove path on powerpc arch. > >So is this is a bug fix? It hasn't trigger a bug yet. I found this issue during the code reading. When VFs are enabled and try to remove a bus with VFs, it will face a problem. So I port the change in commit ac205b7b here. > >Where/how have you tested this? I have tested after change on Power8, the EEH hotplug path works fine for PFs now. Will test this when EEH for VFs are ready. Suggest me to keep it untill EEH for VFs are ready? > >cheers
On Thu, Dec 04, 2014 at 03:21:18PM +0800, Wei Yang wrote: >On Thu, Dec 04, 2014 at 04:40:35PM +1100, Michael Ellerman wrote: >>On Thu, 2014-04-12 at 03:24:37 UTC, Wei Yang wrote: >>> As in commit ac205b7b (PCI: make sriov work with hotplug remove) indicates, >>> when removing pci devices on a bus which has VFs, we need to remove them in >>> the reverse order. >>> >>> This patch applies this pattern on the hotplug remove path on powerpc arch. >> >>So is this is a bug fix? > >It hasn't trigger a bug yet. I found this issue during the code reading. When >VFs are enabled and try to remove a bus with VFs, it will face a problem. So I >port the change in commit ac205b7b here. > >> >>Where/how have you tested this? > >I have tested after change on Power8, the EEH hotplug path works fine for PFs >now. Will test this when EEH for VFs are ready. > >Suggest me to keep it untill EEH for VFs are ready? > Please keep it and resend it (with typo fixed as I pointed) after SRIOV patchset gets merged. If SRIOV isn't enabled, we don't need the code change. By the way, it's something related to EEH for PFs. When PF and its VFs seat on same PCI bus, we should remove VFs before putting PF offline in the reversed order as you did in your code change. Otherwise, PF is put into offline and its driver disables VFs. We try redoing the removal for VFs in hotplug path, which would cause race condition. If VFs aren't existing, until your SRIOV patchset is merged, we don't have this problem. Please correct me if I understood things wrongly. Thanks, Gavin
On Fri, Dec 05, 2014 at 09:48:13AM +1100, Gavin Shan wrote: >On Thu, Dec 04, 2014 at 03:21:18PM +0800, Wei Yang wrote: >>On Thu, Dec 04, 2014 at 04:40:35PM +1100, Michael Ellerman wrote: >>>On Thu, 2014-04-12 at 03:24:37 UTC, Wei Yang wrote: >>>> As in commit ac205b7b (PCI: make sriov work with hotplug remove) indicates, >>>> when removing pci devices on a bus which has VFs, we need to remove them in >>>> the reverse order. >>>> >>>> This patch applies this pattern on the hotplug remove path on powerpc arch. >>> >>>So is this is a bug fix? >> >>It hasn't trigger a bug yet. I found this issue during the code reading. When >>VFs are enabled and try to remove a bus with VFs, it will face a problem. So I >>port the change in commit ac205b7b here. >> >>> >>>Where/how have you tested this? >> >>I have tested after change on Power8, the EEH hotplug path works fine for PFs >>now. Will test this when EEH for VFs are ready. >> >>Suggest me to keep it untill EEH for VFs are ready? >> > >Please keep it and resend it (with typo fixed as I pointed) after SRIOV patchset >gets merged. If SRIOV isn't enabled, we don't need the code change. > >By the way, it's something related to EEH for PFs. When PF and its VFs seat on >same PCI bus, we should remove VFs before putting PF offline in the reversed >order as you did in your code change. Otherwise, PF is put into offline and >its driver disables VFs. We try redoing the removal for VFs in hotplug path, >which would cause race condition. If VFs aren't existing, until your SRIOV >patchset is merged, we don't have this problem. Please correct me if I >understood things wrongly. > Current code is fine until VF is introduced. Yes, your understanding is correct. >Thanks, >Gavin
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c index 5b78917..f0f00d9 100644 --- a/arch/powerpc/kernel/pci-hotplug.c +++ b/arch/powerpc/kernel/pci-hotplug.c @@ -50,7 +50,7 @@ void pcibios_remove_pci_devices(struct pci_bus *bus) pr_debug("PCI: Removing devices on bus %04x:%02x\n", pci_domain_nr(bus), bus->number); - list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) { + list_for_each_entry_safe_reverse(dev, tmp, &bus->devices, bus_list) { pr_debug(" Removing %s...\n", pci_name(dev)); pci_stop_and_remove_bus_device(dev); }