| Message ID | 1397359333-8378-1-git-send-email-weiyang@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hello. On 13-04-2014 7:22, Wei Yang wrote: > pci_match_id() just match the static pci_device_id, which may return NULL if > someone binds the driver to a device manually using > /sys/bus/pci/drivers/.../new_id. > This patch wrap up a helper function __mlx4_remove_one() which does the tear > down function but preserve the drv_data. Functions like > mlx4_pci_err_detected() and mlx4_restart_one() will call this one with out > releasing drvdata. > Fixes: 97a5221 "net/mlx4_core: pass pci_device_id.driver_data to __mlx4_init_one during reset". > CC: Bjorn Helgaas <bhelgaas@google.com> > CC: Amir Vadai <amirv@mellanox.com> > CC: Jack Morgenstein <jackm@dev.mellanox.co.il> > CC: Or Gerlitz <ogerlitz@mellanox.com> > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> > Acked-by: Jack Morgenstein <jackm@dev.mellanox.co.il> [...] > diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c > index f0ae95f..b6d60e2 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/main.c > +++ b/drivers/net/ethernet/mellanox/mlx4/main.c [...] > @@ -2604,85 +2598,112 @@ err_disable_pdev: > > static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id) > { > + struct mlx4_priv *priv; > + struct mlx4_dev *dev; > + > printk_once(KERN_INFO "%s", mlx4_version); > > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + return -ENOMEM; > + } {} not needed here. [...] > + /* 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)) Could be folded into single *if*. > + printk(KERN_ERR "Removing PF when there are assigned VF's !!!\n"); > + } > + mlx4_stop_sense(dev); > + mlx4_unregister_device(dev); [...] > + for (p = 1; p <= dev->caps.num_ports; p++) { > + mlx4_cleanup_port_info(&priv->port[p]); > + mlx4_CLOSE_PORT(dev, p); > + } > + > + if (mlx4_is_master(dev)) > + mlx4_free_resource_tracker(dev, > + RES_TR_FREE_SLAVES_ONLY); Please re-align this line so that it starts right under 'dev'. [...] > > - if (!mlx4_is_slave(dev)) > - mlx4_free_ownership(dev); > + if (mlx4_is_master(dev)) > + mlx4_free_resource_tracker(dev, > + RES_TR_FREE_STRUCTS_ONLY); Likewise. [...] > @@ -2755,11 +2776,13 @@ static pci_ers_result_t mlx4_pci_err_detected(struct pci_dev *pdev, > > static pci_ers_result_t mlx4_pci_slot_reset(struct pci_dev *pdev) > { > - const struct pci_device_id *id; > - int ret; > + struct mlx4_dev *dev = pci_get_drvdata(pdev); > + struct mlx4_priv *priv = mlx4_priv(dev); > + int pci_dev_data; Doesn't seem that this variable is necessary. > + int ret; > > - id = pci_match_id(mlx4_pci_table, pdev); > - ret = __mlx4_init_one(pdev, id->driver_data); > + pci_dev_data = priv->pci_dev_data; > + ret = __mlx4_init_one(pdev, pci_dev_data); > > return ret ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED; > } [...] WBR, Sergei -- 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 Sun, Apr 13, 2014 at 03:06:10PM +0400, Sergei Shtylyov wrote: >Hello. Thanks for the comments. Changed accordingly and will send another version. > >On 13-04-2014 7:22, Wei Yang wrote: > >>pci_match_id() just match the static pci_device_id, which may return NULL if >>someone binds the driver to a device manually using >>/sys/bus/pci/drivers/.../new_id. > >>This patch wrap up a helper function __mlx4_remove_one() which does the tear >>down function but preserve the drv_data. Functions like >>mlx4_pci_err_detected() and mlx4_restart_one() will call this one with out >>releasing drvdata. > >>Fixes: 97a5221 "net/mlx4_core: pass pci_device_id.driver_data to __mlx4_init_one during reset". > >>CC: Bjorn Helgaas <bhelgaas@google.com> >>CC: Amir Vadai <amirv@mellanox.com> >>CC: Jack Morgenstein <jackm@dev.mellanox.co.il> >>CC: Or Gerlitz <ogerlitz@mellanox.com> >>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> >>Acked-by: Jack Morgenstein <jackm@dev.mellanox.co.il> > >[...] > >>diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c >>index f0ae95f..b6d60e2 100644 >>--- a/drivers/net/ethernet/mellanox/mlx4/main.c >>+++ b/drivers/net/ethernet/mellanox/mlx4/main.c >[...] >>@@ -2604,85 +2598,112 @@ err_disable_pdev: >> >> static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id) >> { >>+ struct mlx4_priv *priv; >>+ struct mlx4_dev *dev; >>+ >> printk_once(KERN_INFO "%s", mlx4_version); >> >>+ priv = kzalloc(sizeof(*priv), GFP_KERNEL); >>+ if (!priv) { >>+ return -ENOMEM; >>+ } > > {} not needed here. > >[...] >>+ /* 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)) > > Could be folded into single *if*. > >>+ printk(KERN_ERR "Removing PF when there are assigned VF's !!!\n"); >>+ } >>+ mlx4_stop_sense(dev); >>+ mlx4_unregister_device(dev); >[...] >>+ for (p = 1; p <= dev->caps.num_ports; p++) { >>+ mlx4_cleanup_port_info(&priv->port[p]); >>+ mlx4_CLOSE_PORT(dev, p); >>+ } >>+ >>+ if (mlx4_is_master(dev)) >>+ mlx4_free_resource_tracker(dev, >>+ RES_TR_FREE_SLAVES_ONLY); > > Please re-align this line so that it starts right under 'dev'. > >[...] >> >>- if (!mlx4_is_slave(dev)) >>- mlx4_free_ownership(dev); >>+ if (mlx4_is_master(dev)) >>+ mlx4_free_resource_tracker(dev, >>+ RES_TR_FREE_STRUCTS_ONLY); > > Likewise. > >[...] >>@@ -2755,11 +2776,13 @@ static pci_ers_result_t mlx4_pci_err_detected(struct pci_dev *pdev, >> >> static pci_ers_result_t mlx4_pci_slot_reset(struct pci_dev *pdev) >> { >>- const struct pci_device_id *id; >>- int ret; >>+ struct mlx4_dev *dev = pci_get_drvdata(pdev); >>+ struct mlx4_priv *priv = mlx4_priv(dev); >>+ int pci_dev_data; > > Doesn't seem that this variable is necessary. > >>+ int ret; >> >>- id = pci_match_id(mlx4_pci_table, pdev); >>- ret = __mlx4_init_one(pdev, id->driver_data); >>+ pci_dev_data = priv->pci_dev_data; >>+ ret = __mlx4_init_one(pdev, pci_dev_data); >> >> return ret ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED; >> } >[...] > >WBR, Sergei
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c index f0ae95f..b6d60e2 100644 --- a/drivers/net/ethernet/mellanox/mlx4/main.c +++ b/drivers/net/ethernet/mellanox/mlx4/main.c @@ -2301,13 +2301,8 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data) /* Allow large DMA segments, up to the firmware limit of 1 GB */ dma_set_max_seg_size(&pdev->dev, 1024 * 1024 * 1024); - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) { - err = -ENOMEM; - goto err_release_regions; - } - - dev = &priv->dev; + dev = pci_get_drvdata(pdev); + priv = mlx4_priv(dev); dev->pdev = pdev; INIT_LIST_HEAD(&priv->ctx_list); spin_lock_init(&priv->ctx_lock); @@ -2535,8 +2530,7 @@ slave_start: mlx4_sense_init(dev); mlx4_start_sense(dev); - priv->pci_dev_data = pci_dev_data; - pci_set_drvdata(pdev, dev); + priv->removed = 0; return 0; @@ -2604,85 +2598,112 @@ err_disable_pdev: static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id) { + struct mlx4_priv *priv; + struct mlx4_dev *dev; + printk_once(KERN_INFO "%s", mlx4_version); + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + return -ENOMEM; + } + + dev = &priv->dev; + pci_set_drvdata(pdev, dev); + priv->pci_dev_data = id->driver_data; + return __mlx4_init_one(pdev, id->driver_data); } -static void mlx4_remove_one(struct pci_dev *pdev) +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 pci_dev_data; 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_stop_sense(dev); - mlx4_unregister_device(dev); + if (priv->removed) + return; - for (p = 1; p <= dev->caps.num_ports; p++) { - mlx4_cleanup_port_info(&priv->port[p]); - mlx4_CLOSE_PORT(dev, p); - } + pci_dev_data = priv->pci_dev_data; - if (mlx4_is_master(dev)) - mlx4_free_resource_tracker(dev, - RES_TR_FREE_SLAVES_ONLY); - - mlx4_cleanup_counters_table(dev); - mlx4_cleanup_qp_table(dev); - mlx4_cleanup_srq_table(dev); - mlx4_cleanup_cq_table(dev); - mlx4_cmd_use_polling(dev); - mlx4_cleanup_eq_table(dev); - mlx4_cleanup_mcg_table(dev); - mlx4_cleanup_mr_table(dev); - mlx4_cleanup_xrcd_table(dev); - mlx4_cleanup_pd_table(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_stop_sense(dev); + mlx4_unregister_device(dev); - if (mlx4_is_master(dev)) - mlx4_free_resource_tracker(dev, - RES_TR_FREE_STRUCTS_ONLY); - - iounmap(priv->kar); - mlx4_uar_free(dev, &priv->driver_uar); - mlx4_cleanup_uar_table(dev); - if (!mlx4_is_slave(dev)) - mlx4_clear_steering(dev); - mlx4_free_eq_table(dev); - if (mlx4_is_master(dev)) - mlx4_multi_func_cleanup(dev); - mlx4_close_hca(dev); - if (mlx4_is_slave(dev)) - mlx4_multi_func_cleanup(dev); - mlx4_cmd_cleanup(dev); - - if (dev->flags & MLX4_FLAG_MSI_X) - pci_disable_msix(pdev); - if (dev->flags & MLX4_FLAG_SRIOV) { - mlx4_warn(dev, "Disabling SR-IOV\n"); - pci_disable_sriov(pdev); - } + for (p = 1; p <= dev->caps.num_ports; p++) { + mlx4_cleanup_port_info(&priv->port[p]); + mlx4_CLOSE_PORT(dev, p); + } + + if (mlx4_is_master(dev)) + mlx4_free_resource_tracker(dev, + RES_TR_FREE_SLAVES_ONLY); + + mlx4_cleanup_counters_table(dev); + mlx4_cleanup_qp_table(dev); + mlx4_cleanup_srq_table(dev); + mlx4_cleanup_cq_table(dev); + mlx4_cmd_use_polling(dev); + mlx4_cleanup_eq_table(dev); + mlx4_cleanup_mcg_table(dev); + mlx4_cleanup_mr_table(dev); + mlx4_cleanup_xrcd_table(dev); + mlx4_cleanup_pd_table(dev); - if (!mlx4_is_slave(dev)) - mlx4_free_ownership(dev); + if (mlx4_is_master(dev)) + mlx4_free_resource_tracker(dev, + RES_TR_FREE_STRUCTS_ONLY); - kfree(dev->caps.qp0_tunnel); - kfree(dev->caps.qp0_proxy); - kfree(dev->caps.qp1_tunnel); - kfree(dev->caps.qp1_proxy); - kfree(dev->dev_vfs); + iounmap(priv->kar); + mlx4_uar_free(dev, &priv->driver_uar); + mlx4_cleanup_uar_table(dev); + if (!mlx4_is_slave(dev)) + mlx4_clear_steering(dev); + mlx4_free_eq_table(dev); + if (mlx4_is_master(dev)) + mlx4_multi_func_cleanup(dev); + mlx4_close_hca(dev); + if (mlx4_is_slave(dev)) + mlx4_multi_func_cleanup(dev); + mlx4_cmd_cleanup(dev); - kfree(priv); - pci_release_regions(pdev); - pci_disable_device(pdev); - pci_set_drvdata(pdev, NULL); + if (dev->flags & MLX4_FLAG_MSI_X) + pci_disable_msix(pdev); + if (dev->flags & MLX4_FLAG_SRIOV) { + mlx4_warn(dev, "Disabling SR-IOV\n"); + pci_disable_sriov(pdev); } + + if (!mlx4_is_slave(dev)) + mlx4_free_ownership(dev); + + kfree(dev->caps.qp0_tunnel); + kfree(dev->caps.qp0_proxy); + kfree(dev->caps.qp1_tunnel); + kfree(dev->caps.qp1_proxy); + kfree(dev->dev_vfs); + + pci_release_regions(pdev); + pci_disable_device(pdev); + memset(priv, 0, sizeof(*priv)); + priv->pci_dev_data = pci_dev_data; + priv->removed = 1; +} + +static void mlx4_remove_one(struct pci_dev *pdev) +{ + struct mlx4_dev *dev = pci_get_drvdata(pdev); + struct mlx4_priv *priv = mlx4_priv(dev); + + __mlx4_remove_one(pdev); + kfree(priv); + pci_set_drvdata(pdev, NULL); } int mlx4_restart_one(struct pci_dev *pdev) @@ -2692,7 +2713,7 @@ int mlx4_restart_one(struct pci_dev *pdev) int pci_dev_data; pci_dev_data = priv->pci_dev_data; - mlx4_remove_one(pdev); + __mlx4_remove_one(pdev); return __mlx4_init_one(pdev, pci_dev_data); } @@ -2747,7 +2768,7 @@ MODULE_DEVICE_TABLE(pci, mlx4_pci_table); static pci_ers_result_t mlx4_pci_err_detected(struct pci_dev *pdev, pci_channel_state_t state) { - mlx4_remove_one(pdev); + __mlx4_remove_one(pdev); return state == pci_channel_io_perm_failure ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET; @@ -2755,11 +2776,13 @@ static pci_ers_result_t mlx4_pci_err_detected(struct pci_dev *pdev, static pci_ers_result_t mlx4_pci_slot_reset(struct pci_dev *pdev) { - const struct pci_device_id *id; - int ret; + struct mlx4_dev *dev = pci_get_drvdata(pdev); + struct mlx4_priv *priv = mlx4_priv(dev); + int pci_dev_data; + int ret; - id = pci_match_id(mlx4_pci_table, pdev); - ret = __mlx4_init_one(pdev, id->driver_data); + pci_dev_data = priv->pci_dev_data; + ret = __mlx4_init_one(pdev, pci_dev_data); return ret ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED; } diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h index cf8be41..f9c4651 100644 --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h @@ -800,6 +800,7 @@ struct mlx4_priv { spinlock_t ctx_lock; int pci_dev_data; + int removed; struct list_head pgdir_list; struct mutex pgdir_mutex;