Commit Message

After commit dc087f2f6a2925e81831f3016b9cbb6e470e7423
(PCI: Simplify IOV implementation and fix reference count races)
VF need to be removed via virtfn_remove to make sure ref to PF
is put back.
So we can not call stop_and_remove for VF before PF, that will
make VF get removed directly before PF's driver try to use
virtfn_remove to do it.
Solution is separating stop and remove in two iterations.
In first iteration, we stop VF driver at first with iterating devices
reversely, and later during stop PF driver, disable_sriov will use
virtfn_remove to remove VFs.
Also some driver (like mlx4_core) need VF's driver get stopped before PF.
To make it simple, separate VGA checking out and do that at first,
if there is VGA in the chain, do even try to stop or remove any device
under that bus.
Need this one for v3.11.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Yijing Wang <wangyijing@huawei.com>
Cc: Jiang Liu <liuj97@gmail.com>
---
drivers/pci/hotplug/pciehp_pci.c | 46 ++++++++++++++++++++++++++-------------
drivers/pci/remove.c | 6 +++--
include/linux/pci.h | 2 +
3 files changed, 37 insertions(+), 17 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Mon, Jul 22, 2013 at 2:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Evidently this is really part of a series, but you didn't label it> that way. It looks like this applies on top of your "PCI: Fix hotplug> remove with sriov again" patch?
Yes.
that one need be back ported to 3.9 and later.
this one need to be for 3.11 final, as Jiang's patch is in 3.11-rc0.
>> On Fri, Jul 19, 2013 at 1:14 PM, Yinghai Lu <yinghai@kernel.org> wrote:>> After commit dc087f2f6a2925e81831f3016b9cbb6e470e7423>> (PCI: Simplify IOV implementation and fix reference count races)>> VF need to be removed via virtfn_remove to make sure ref to PF>> is put back.>> I'm sure this makes sense, but it needs background. I haven't figured> out exactly what the problem is. You're describing the lowest-level> details, but not the overall picture that would make it understandable> to the rest of us.
Overall, after we reversely loop the bus_devices, VF get stop and removed,
but fail to release ref to PF, and prevent PF to be removed and freed.
>>> So we can not call stop_and_remove for VF before PF, that will>> make VF get removed directly before PF's driver try to use>> virtfn_remove to do it.>>>> Solution is separating stop and remove in two iterations.>>>> In first iteration, we stop VF driver at first with iterating devices>> reversely, and later during stop PF driver, disable_sriov will use>> virtfn_remove to remove VFs.>>>> Also some driver (like mlx4_core) need VF's driver get stopped before PF.>> If this is relevant, please give a pointer to the mlx4_core code that> requires VFs to be stopped before the PF.
that is add-on benefits.
drivers/net/ethernet/mellanox/mlx4/main.c::
static void mlx4_remove_one(struct pci_dev *pdev)
{
struct mlx4_dev *dev = pci_get_drvdata(pdev);
struct mlx4_priv *priv = mlx4_priv(dev);
int p;
if (dev) {
/* in SRIOV it is not allowed to unload the pf's
* driver while there are alive vf's */
if (mlx4_is_master(dev)) {
if (mlx4_how_many_lives_vf(dev))
printk(KERN_ERR "Removing PF when
there are assigned VF's !!!\n");
}
mlx4_how_many_lives_vf() is checking how VFs have driver loaded.
>>> To make it simple, separate VGA checking out and do that at first,>> if there is VGA in the chain, do even try to stop or remove any device>> under that bus.>> This part seems like it could be a separate patch.
ok, will separate it out in next rev.
>>> Need this one for v3.11.>> OK, but why? We need a better explanation of what problem this fixes.> It sounds like it fixes a reference counting problem in v3.11-rc1,> but I don't know what the effect of that problem is. Maybe it means a> device isn't freed when it should be? Maybe it means we can't add a> new device after hot-removing an SR-IOV device?
Yes.
>
...
>> Index: linux-2.6/drivers/pci/remove.c>> ===================================================================>> --- linux-2.6.orig/drivers/pci/remove.c>> +++ linux-2.6/drivers/pci/remove.c>> @@ -56,7 +56,7 @@ void pci_remove_bus(struct pci_bus *bus)>> }>> EXPORT_SYMBOL(pci_remove_bus);>>>> -static void pci_stop_bus_device(struct pci_dev *dev)>> +void pci_stop_bus_device(struct pci_dev *dev)>> {>> struct pci_bus *bus = dev->subordinate;>> struct pci_dev *child, *tmp;>> @@ -75,8 +75,9 @@ static void pci_stop_bus_device(struct p>>>> pci_stop_dev(dev);>> }>> +EXPORT_SYMBOL(pci_stop_bus_device);>>>> -static void pci_remove_bus_device(struct pci_dev *dev)>> +void pci_remove_bus_device(struct pci_dev *dev)>> {>> struct pci_bus *bus = dev->subordinate;>> struct pci_dev *child, *tmp;>> @@ -92,6 +93,7 @@ static void pci_remove_bus_device(struct>>>> pci_destroy_dev(dev);>> }>> +EXPORT_SYMBOL(pci_remove_bus_device);>> I really don't want to export these two symbols unless we have to.> Obviously this is the heart of your patch, so we probably *will* have> to.
Agree.
>> But maybe they can at least be confined to drivers/pci code, and not> exported to modules? I don't think there's any reason pciehp needs to> be a module. I was planning to merge a patch restricting it to be> built-in this cycle anyway.
sure, you can apply that patch (make pciehp to be built-in) at first.
Thanks
Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html