From: Jeff Garzik <jgarzik@pobox.com>
To: Scott Feldman <scofeldm@cisco.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH 4/4] enic: bug fix: don't set netdev->name too early
Date: Wed, 24 Sep 2008 20:51:29 -0400 [thread overview]
Message-ID: <48DAE091.70507@pobox.com> (raw)
In-Reply-To: <20080924182353.22778.83553.stgit@palito_client100.nuovasystems.com>
Scott Feldman wrote:
> enic: bug fix: don't set netdev->name too early
>
> Bug fix: don't set netdev->name early before netdev registration. Setting
> netdev->name early with dev_alloc_name() would occasionally cause netdev
> registration to fail returning error that device was already registered.
> Since we're using netdev->name to name MSI-X vectors, we now need to
> move the request_irq after netdev registartion, so move it to ->open.
>
> Signed-off-by: Scott Feldman <scofeldm@cisco.com>
> ---
> drivers/net/enic/enic.h | 2 -
> drivers/net/enic/enic_main.c | 121 ++++++++++++++++--------------------------
> 2 files changed, 47 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
> index 9e0d484..7f677e8 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -33,7 +33,7 @@
>
> #define DRV_NAME "enic"
> #define DRV_DESCRIPTION "Cisco 10G Ethernet Driver"
> -#define DRV_VERSION "0.0.1.18163.472"
> +#define DRV_VERSION "0.0.1-18163.472-k1"
> #define DRV_COPYRIGHT "Copyright 2008 Cisco Systems, Inc"
> #define PFX DRV_NAME ": "
>
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 14e59a7..180e968 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -1251,13 +1251,28 @@ static int enic_open(struct net_device *netdev)
> unsigned int i;
> int err;
>
> + err = enic_request_intr(enic);
> + if (err) {
> + printk(KERN_ERR PFX "%s: Unable to request irq.\n",
> + netdev->name);
> + return err;
> + }
> +
> + err = enic_notify_set(enic);
> + if (err) {
> + printk(KERN_ERR PFX
> + "%s: Failed to alloc notify buffer, aborting.\n",
> + netdev->name);
> + goto err_out_free_intr;
> + }
> +
> for (i = 0; i < enic->rq_count; i++) {
> err = vnic_rq_fill(&enic->rq[i], enic_rq_alloc_buf);
> if (err) {
> printk(KERN_ERR PFX
> "%s: Unable to alloc receive buffers.\n",
> netdev->name);
> - return err;
> + goto err_out_notify_unset;
> }
> }
>
> @@ -1279,6 +1294,13 @@ static int enic_open(struct net_device *netdev)
> enic_notify_timer_start(enic);
>
> return 0;
> +
> +err_out_notify_unset:
> + vnic_dev_notify_unset(enic->vdev);
> +err_out_free_intr:
> + enic_free_intr(enic);
> +
> + return err;
> }
>
> /* rtnl lock is held, process context */
> @@ -1308,6 +1330,9 @@ static int enic_stop(struct net_device *netdev)
> return err;
> }
>
> + vnic_dev_notify_unset(enic->vdev);
> + enic_free_intr(enic);
> +
> (void)vnic_cq_service(&enic->cq[ENIC_CQ_RQ],
> -1, enic_rq_service_drop, NULL);
> (void)vnic_cq_service(&enic->cq[ENIC_CQ_WQ],
> @@ -1593,18 +1618,6 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> return -ENOMEM;
> }
>
> - /* Set the netdev name early so intr vectors are properly
> - * named and any error msgs can include netdev->name
> - */
> -
> - rtnl_lock();
> - err = dev_alloc_name(netdev, netdev->name);
> - rtnl_unlock();
> - if (err < 0) {
> - printk(KERN_ERR PFX "Unable to allocate netdev name.\n");
> - goto err_out_free_netdev;
> - }
> -
> pci_set_drvdata(pdev, netdev);
>
> SET_NETDEV_DEV(netdev, &pdev->dev);
> @@ -1619,16 +1632,14 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = pci_enable_device(pdev);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Cannot enable PCI device, aborting.\n",
> - netdev->name);
> + "Cannot enable PCI device, aborting.\n");
> goto err_out_free_netdev;
> }
>
> err = pci_request_regions(pdev, DRV_NAME);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Cannot request PCI regions, aborting.\n",
> - netdev->name);
> + "Cannot request PCI regions, aborting.\n");
> goto err_out_disable_device;
> }
>
> @@ -1644,25 +1655,22 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: No usable DMA configuration, aborting.\n",
> - netdev->name);
> + "No usable DMA configuration, aborting.\n");
> goto err_out_release_regions;
> }
> err = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Unable to obtain 32-bit DMA "
> - "for consistent allocations, aborting.\n",
> - netdev->name);
> + "Unable to obtain 32-bit DMA "
> + "for consistent allocations, aborting.\n");
> goto err_out_release_regions;
> }
> } else {
> err = pci_set_consistent_dma_mask(pdev, DMA_40BIT_MASK);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Unable to obtain 40-bit DMA "
> - "for consistent allocations, aborting.\n",
> - netdev->name);
> + "Unable to obtain 40-bit DMA "
> + "for consistent allocations, aborting.\n");
> goto err_out_release_regions;
> }
> using_dac = 1;
> @@ -1673,8 +1681,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
>
> if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
> printk(KERN_ERR PFX
> - "%s: BAR0 not memory-map'able, aborting.\n",
> - netdev->name);
> + "BAR0 not memory-map'able, aborting.\n");
> err = -ENODEV;
> goto err_out_release_regions;
> }
> @@ -1685,8 +1692,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
>
> if (!enic->bar0.vaddr) {
> printk(KERN_ERR PFX
> - "%s: Cannot memory-map BAR0 res hdr, aborting.\n",
> - netdev->name);
> + "Cannot memory-map BAR0 res hdr, aborting.\n");
> err = -ENODEV;
> goto err_out_release_regions;
> }
> @@ -1697,8 +1703,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> enic->vdev = vnic_dev_register(NULL, enic, pdev, &enic->bar0);
> if (!enic->vdev) {
> printk(KERN_ERR PFX
> - "%s: vNIC registration failed, aborting.\n",
> - netdev->name);
> + "vNIC registration failed, aborting.\n");
> err = -ENODEV;
> goto err_out_iounmap;
> }
> @@ -1709,8 +1714,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = enic_dev_open(enic);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: vNIC dev open failed, aborting.\n",
> - netdev->name);
> + "vNIC dev open failed, aborting.\n");
> goto err_out_vnic_unregister;
> }
>
> @@ -1727,8 +1731,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = vnic_dev_init(enic->vdev, 0);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: vNIC dev init failed, aborting.\n",
> - netdev->name);
> + "vNIC dev init failed, aborting.\n");
> goto err_out_dev_close;
> }
>
> @@ -1738,8 +1741,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = enic_get_vnic_config(enic);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Get vNIC configuration failed, aborting.\n",
> - netdev->name);
> + "Get vNIC configuration failed, aborting.\n");
> goto err_out_dev_close;
> }
>
> @@ -1755,18 +1757,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = enic_set_intr_mode(enic);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Failed to set intr mode, aborting.\n",
> - netdev->name);
> - goto err_out_dev_close;
> - }
> -
> - /* Request interrupt vector(s)
> - */
> -
> - err = enic_request_intr(enic);
> - if (err) {
> - printk(KERN_ERR PFX "%s: Unable to request irq.\n",
> - netdev->name);
> + "Failed to set intr mode, aborting.\n");
> goto err_out_dev_close;
> }
>
> @@ -1776,8 +1767,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = enic_alloc_vnic_resources(enic);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Failed to alloc vNIC resources, aborting.\n",
> - netdev->name);
> + "Failed to alloc vNIC resources, aborting.\n");
> goto err_out_free_vnic_resources;
> }
>
> @@ -1793,19 +1783,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> ig_vlan_strip_en);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Failed to config nic, aborting.\n",
> - netdev->name);
> - goto err_out_free_vnic_resources;
> - }
> -
> - /* Setup notification buffer area
> - */
> -
> - err = enic_notify_set(enic);
> - if (err) {
> - printk(KERN_ERR PFX
> - "%s: Failed to alloc notify buffer, aborting.\n",
> - netdev->name);
> + "Failed to config nic, aborting.\n");
> goto err_out_free_vnic_resources;
> }
>
> @@ -1832,9 +1810,8 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = enic_set_mac_addr(netdev, enic->mac_addr);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Invalid MAC address, aborting.\n",
> - netdev->name);
> - goto err_out_notify_unset;
> + "Invalid MAC address, aborting.\n");
> + goto err_out_free_vnic_resources;
> }
>
> netdev->open = enic_open;
> @@ -1888,18 +1865,14 @@ static int __devinit enic_probe(struct pci_dev *pdev,
> err = register_netdev(netdev);
> if (err) {
> printk(KERN_ERR PFX
> - "%s: Cannot register net device, aborting.\n",
> - netdev->name);
> - goto err_out_notify_unset;
> + "Cannot register net device, aborting.\n");
> + goto err_out_free_vnic_resources;
> }
>
> return 0;
>
> -err_out_notify_unset:
> - vnic_dev_notify_unset(enic->vdev);
> err_out_free_vnic_resources:
> enic_free_vnic_resources(enic);
> - enic_free_intr(enic);
> err_out_dev_close:
> vnic_dev_close(enic->vdev);
> err_out_vnic_unregister:
> @@ -1927,9 +1900,7 @@ static void __devexit enic_remove(struct pci_dev *pdev)
>
applied 1-4
prev parent reply other threads:[~2008-09-25 0:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-24 18:23 [PATCH 0/4] enic: review updates and bug fixes Scott Feldman
2008-09-24 18:23 ` [PATCH 1/4] enic: Don't indicate IPv6 pkts using soft-LRO Scott Feldman
2008-09-24 18:23 ` [PATCH 2/4] enic: fixes for review items from Ben Hutchings Scott Feldman
2008-09-24 18:23 ` [PATCH 3/4] enic: Bug fix: Free MSI intr with correct data handle Scott Feldman
2008-09-24 18:23 ` [PATCH 4/4] enic: bug fix: don't set netdev->name too early Scott Feldman
2008-09-25 0:51 ` Jeff Garzik [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48DAE091.70507@pobox.com \
--to=jgarzik@pobox.com \
--cc=netdev@vger.kernel.org \
--cc=scofeldm@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.