Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/486774/?format=api
{ "id": 486774, "url": "http://patchwork.ozlabs.org/api/patches/486774/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/1434729390-7698-1-git-send-email-mitch.a.williams@intel.com/", "project": { "id": 46, "url": "http://patchwork.ozlabs.org/api/projects/46/?format=api", "name": "Intel Wired Ethernet development", "link_name": "intel-wired-lan", "list_id": "intel-wired-lan.osuosl.org", "list_email": "intel-wired-lan@osuosl.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<1434729390-7698-1-git-send-email-mitch.a.williams@intel.com>", "list_archive_url": null, "date": "2015-06-19T15:56:30", "name": "[net,v2] i40evf: fix panic during MTU change", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "442ece867f9a5e79d3d638a0a1fdb39271887187", "submitter": { "id": 967, "url": "http://patchwork.ozlabs.org/api/people/967/?format=api", "name": "Mitch Williams", "email": "mitch.a.williams@intel.com" }, "delegate": { "id": 68, "url": "http://patchwork.ozlabs.org/api/users/68/?format=api", "username": "jtkirshe", "first_name": "Jeff", "last_name": "Kirsher", "email": "jeffrey.t.kirsher@intel.com" }, "mbox": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/1434729390-7698-1-git-send-email-mitch.a.williams@intel.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/486774/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/486774/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<intel-wired-lan-bounces@lists.osuosl.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Received": [ "from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\tby ozlabs.org (Postfix) with ESMTP id 6C77C14016A\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat, 20 Jun 2015 01:55:55 +1000 (AEST)", "from localhost (localhost [127.0.0.1])\n\tby silver.osuosl.org (Postfix) with ESMTP id DFB0833565;\n\tFri, 19 Jun 2015 15:55:53 +0000 (UTC)", "from silver.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id ywbTib3CsBnn; Fri, 19 Jun 2015 15:55:50 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby silver.osuosl.org (Postfix) with ESMTP id B59DB30EC2;\n\tFri, 19 Jun 2015 15:55:50 +0000 (UTC)", "from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\tby ash.osuosl.org (Postfix) with ESMTP id C68151CF521\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tFri, 19 Jun 2015 15:55:49 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby silver.osuosl.org (Postfix) with ESMTP id BF14731601\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tFri, 19 Jun 2015 15:55:49 +0000 (UTC)", "from silver.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id wxTmAAhTNkaa for <intel-wired-lan@lists.osuosl.org>;\n\tFri, 19 Jun 2015 15:55:48 +0000 (UTC)", "from mga01.intel.com (mga01.intel.com [192.55.52.88])\n\tby silver.osuosl.org (Postfix) with ESMTP id 2D49530EC2\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tFri, 19 Jun 2015 15:55:48 +0000 (UTC)", "from orsmga002.jf.intel.com ([10.7.209.21])\n\tby fmsmga101.fm.intel.com with ESMTP; 19 Jun 2015 08:55:47 -0700", "from mawilli1-mobl.jf.intel.com ([10.166.14.28])\n\tby orsmga002.jf.intel.com with ESMTP; 19 Jun 2015 08:55:47 -0700" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "domain auto-whitelisted by SQLgrey-1.7.6", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.13,644,1427785200\"; d=\"scan'208\";a=\"749699479\"", "From": "Mitch Williams <mitch.a.williams@intel.com>", "To": "intel-wired-lan@lists.osuosl.org", "Date": "Fri, 19 Jun 2015 08:56:30 -0700", "Message-Id": "<1434729390-7698-1-git-send-email-mitch.a.williams@intel.com>", "X-Mailer": "git-send-email 2.4.1", "Subject": "[Intel-wired-lan] [net PATCH v2] i40evf: fix panic during MTU change", "X-BeenThere": "intel-wired-lan@lists.osuosl.org", "X-Mailman-Version": "2.1.18-1", "Precedence": "list", "List-Id": "Intel Wired Ethernet Linux Kernel Driver Development\n\t<intel-wired-lan.lists.osuosl.org>", "List-Unsubscribe": "<http://lists.osuosl.org/mailman/options/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=unsubscribe>", "List-Archive": "<http://lists.osuosl.org/pipermail/intel-wired-lan/>", "List-Post": "<mailto:intel-wired-lan@lists.osuosl.org>", "List-Help": "<mailto:intel-wired-lan-request@lists.osuosl.org?subject=help>", "List-Subscribe": "<http://lists.osuosl.org/mailman/listinfo/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=subscribe>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "intel-wired-lan-bounces@lists.osuosl.org", "Sender": "\"Intel-wired-lan\" <intel-wired-lan-bounces@lists.osuosl.org>" }, "content": "Down was requesting queue disables, but then exited immediately\nwithout waiting for the queues to actually disable. This could\nallow any function called after i40evf_down to run immediately,\nincluding i40evf_up, and causes a memory leak.\n\nRemoving the whole reinit_locked function is the best way\nto go about this, and allows for the driver to handle the\nstate changes by requesting reset from the periodic timer.\n\nAlso, add a couple WARN_ONs in slow path to help us recognize\nif we re-introduce this issue or missed any cases.\n\nSigned-off-by: Mitch Williams <mitch.a.williams@intel.com>\nSigned-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>\n---\n drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 4 +\n drivers/net/ethernet/intel/i40evf/i40evf.h | 1 -\n drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 6 +-\n drivers/net/ethernet/intel/i40evf/i40evf_main.c | 109 +++++++++------------\n 4 files changed, 54 insertions(+), 66 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c\nindex 458fbb4..bd49232 100644\n--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c\n+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c\n@@ -488,6 +488,8 @@ int i40evf_setup_tx_descriptors(struct i40e_ring *tx_ring)\n \tif (!dev)\n \t\treturn -ENOMEM;\n \n+\t/* warn if we are about to overwrite the pointer */\n+\tWARN_ON(tx_ring->tx_bi);\n \tbi_size = sizeof(struct i40e_tx_buffer) * tx_ring->count;\n \ttx_ring->tx_bi = kzalloc(bi_size, GFP_KERNEL);\n \tif (!tx_ring->tx_bi)\n@@ -648,6 +650,8 @@ int i40evf_setup_rx_descriptors(struct i40e_ring *rx_ring)\n \tstruct device *dev = rx_ring->dev;\n \tint bi_size;\n \n+\t/* warn if we are about to overwrite the pointer */\n+\tWARN_ON(rx_ring->rx_bi);\n \tbi_size = sizeof(struct i40e_rx_buffer) * rx_ring->count;\n \trx_ring->rx_bi = kzalloc(bi_size, GFP_KERNEL);\n \tif (!rx_ring->rx_bi)\ndiff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h\nindex 1b98c25..fea3b75 100644\n--- a/drivers/net/ethernet/intel/i40evf/i40evf.h\n+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h\n@@ -264,7 +264,6 @@ extern const char i40evf_driver_version[];\n \n int i40evf_up(struct i40evf_adapter *adapter);\n void i40evf_down(struct i40evf_adapter *adapter);\n-void i40evf_reinit_locked(struct i40evf_adapter *adapter);\n void i40evf_reset(struct i40evf_adapter *adapter);\n void i40evf_set_ethtool_ops(struct net_device *netdev);\n void i40evf_update_stats(struct i40evf_adapter *adapter);\ndiff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c\nindex f4e7766..2b53c87 100644\n--- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c\n+++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c\n@@ -267,8 +267,10 @@ static int i40evf_set_ringparam(struct net_device *netdev,\n \tadapter->tx_desc_count = new_tx_count;\n \tadapter->rx_desc_count = new_rx_count;\n \n-\tif (netif_running(netdev))\n-\t\ti40evf_reinit_locked(adapter);\n+\tif (netif_running(netdev)) {\n+\t\tadapter->flags |= I40EVF_FLAG_RESET_NEEDED;\n+\t\tschedule_work(&adapter->reset_task);\n+\t}\n \n \treturn 0;\n }\ndiff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c\nindex 7c53aca..43ed9fe 100644\n--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c\n+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c\n@@ -170,7 +170,8 @@ static void i40evf_tx_timeout(struct net_device *netdev)\n \tstruct i40evf_adapter *adapter = netdev_priv(netdev);\n \n \tadapter->tx_timeout_count++;\n-\tif (!(adapter->flags & I40EVF_FLAG_RESET_PENDING)) {\n+\tif (!(adapter->flags & (I40EVF_FLAG_RESET_PENDING |\n+\t\t\t\tI40EVF_FLAG_RESET_NEEDED))) {\n \t\tadapter->flags |= I40EVF_FLAG_RESET_NEEDED;\n \t\tschedule_work(&adapter->reset_task);\n \t}\n@@ -1470,8 +1471,8 @@ static void i40evf_configure_rss(struct i40evf_adapter *adapter)\n \ti40e_flush(hw);\n }\n \n-#define I40EVF_RESET_WAIT_MS 100\n-#define I40EVF_RESET_WAIT_COUNT 200\n+#define I40EVF_RESET_WAIT_MS 10\n+#define I40EVF_RESET_WAIT_COUNT 500\n /**\n * i40evf_reset_task - Call-back task to handle hardware reset\n * @work: pointer to work_struct\n@@ -1495,10 +1496,17 @@ static void i40evf_reset_task(struct work_struct *work)\n \t\t\t\t&adapter->crit_section))\n \t\tusleep_range(500, 1000);\n \n+\ti40evf_misc_irq_disable(adapter);\n \tif (adapter->flags & I40EVF_FLAG_RESET_NEEDED) {\n-\t\tdev_info(&adapter->pdev->dev, \"Requesting reset from PF\\n\");\n+\t\tadapter->flags &= ~I40EVF_FLAG_RESET_NEEDED;\n+\t\t/* Restart the AQ here. If we have been reset but didn't\n+\t\t * detect it, or if the PF had to reinit, our AQ will be hosed.\n+\t\t */\n+\t\ti40evf_shutdown_adminq(hw);\n+\t\ti40evf_init_adminq(hw);\n \t\ti40evf_request_reset(adapter);\n \t}\n+\tadapter->flags |= I40EVF_FLAG_RESET_PENDING;\n \n \t/* poll until we see the reset actually happen */\n \tfor (i = 0; i < I40EVF_RESET_WAIT_COUNT; i++) {\n@@ -1507,10 +1515,10 @@ static void i40evf_reset_task(struct work_struct *work)\n \t\tif ((rstat_val != I40E_VFR_VFACTIVE) &&\n \t\t (rstat_val != I40E_VFR_COMPLETED))\n \t\t\tbreak;\n-\t\tmsleep(I40EVF_RESET_WAIT_MS);\n+\t\tusleep_range(500, 1000);\n \t}\n \tif (i == I40EVF_RESET_WAIT_COUNT) {\n-\t\tadapter->flags &= ~I40EVF_FLAG_RESET_PENDING;\n+\t\tdev_info(&adapter->pdev->dev, \"Never saw reset\\n\");\n \t\tgoto continue_reset; /* act like the reset happened */\n \t}\n \n@@ -1518,11 +1526,12 @@ static void i40evf_reset_task(struct work_struct *work)\n \tfor (i = 0; i < I40EVF_RESET_WAIT_COUNT; i++) {\n \t\trstat_val = rd32(hw, I40E_VFGEN_RSTAT) &\n \t\t\t I40E_VFGEN_RSTAT_VFR_STATE_MASK;\n-\t\tif ((rstat_val == I40E_VFR_VFACTIVE) ||\n-\t\t (rstat_val == I40E_VFR_COMPLETED))\n+\t\tif (rstat_val == I40E_VFR_VFACTIVE)\n \t\t\tbreak;\n \t\tmsleep(I40EVF_RESET_WAIT_MS);\n \t}\n+\t/* extra wait to make sure minimum wait is met */\n+\tmsleep(I40EVF_RESET_WAIT_MS);\n \tif (i == I40EVF_RESET_WAIT_COUNT) {\n \t\tstruct i40evf_mac_filter *f, *ftmp;\n \t\tstruct i40evf_vlan_filter *fv, *fvtmp;\n@@ -1534,11 +1543,10 @@ static void i40evf_reset_task(struct work_struct *work)\n \n \t\tif (netif_running(adapter->netdev)) {\n \t\t\tset_bit(__I40E_DOWN, &adapter->vsi.state);\n-\t\t\ti40evf_irq_disable(adapter);\n-\t\t\ti40evf_napi_disable_all(adapter);\n-\t\t\tnetif_tx_disable(netdev);\n-\t\t\tnetif_tx_stop_all_queues(netdev);\n \t\t\tnetif_carrier_off(netdev);\n+\t\t\tnetif_tx_disable(netdev);\n+\t\t\ti40evf_napi_disable_all(adapter);\n+\t\t\ti40evf_irq_disable(adapter);\n \t\t\ti40evf_free_traffic_irqs(adapter);\n \t\t\ti40evf_free_all_tx_resources(adapter);\n \t\t\ti40evf_free_all_rx_resources(adapter);\n@@ -1550,6 +1558,7 @@ static void i40evf_reset_task(struct work_struct *work)\n \t\t\tlist_del(&f->list);\n \t\t\tkfree(f);\n \t\t}\n+\n \t\tlist_for_each_entry_safe(fv, fvtmp, &adapter->vlan_filter_list,\n \t\t\t\t\t list) {\n \t\t\tlist_del(&fv->list);\n@@ -1564,22 +1573,27 @@ static void i40evf_reset_task(struct work_struct *work)\n \t\ti40evf_shutdown_adminq(hw);\n \t\tadapter->netdev->flags &= ~IFF_UP;\n \t\tclear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);\n+\t\tadapter->flags &= ~I40EVF_FLAG_RESET_PENDING;\n+\t\tdev_info(&adapter->pdev->dev, \"Reset task did not complete, VF disabled\\n\");\n \t\treturn; /* Do not attempt to reinit. It's dead, Jim. */\n \t}\n \n continue_reset:\n-\tadapter->flags &= ~I40EVF_FLAG_RESET_PENDING;\n-\n-\ti40evf_irq_disable(adapter);\n-\n \tif (netif_running(adapter->netdev)) {\n-\t\ti40evf_napi_disable_all(adapter);\n-\t\tnetif_tx_disable(netdev);\n-\t\tnetif_tx_stop_all_queues(netdev);\n \t\tnetif_carrier_off(netdev);\n+\t\tnetif_tx_stop_all_queues(netdev);\n+\t\ti40evf_napi_disable_all(adapter);\n \t}\n+\ti40evf_irq_disable(adapter);\n \n \tadapter->state = __I40EVF_RESETTING;\n+\tadapter->flags &= ~I40EVF_FLAG_RESET_PENDING;\n+\n+\t/* free the tx/rx rings and descriptors, might be better to just\n+\t * re-use them sometime in the future\n+\t */\n+\ti40evf_free_all_rx_resources(adapter);\n+\ti40evf_free_all_tx_resources(adapter);\n \n \t/* kill and reinit the admin queue */\n \tif (i40evf_shutdown_adminq(hw))\n@@ -1603,6 +1617,7 @@ continue_reset:\n \tadapter->aq_required = I40EVF_FLAG_AQ_ADD_MAC_FILTER;\n \tadapter->aq_required |= I40EVF_FLAG_AQ_ADD_VLAN_FILTER;\n \tclear_bit(__I40EVF_IN_CRITICAL_TASK, &adapter->crit_section);\n+\ti40evf_misc_irq_enable(adapter);\n \n \tmod_timer(&adapter->watchdog_timer, jiffies + 2);\n \n@@ -1624,7 +1639,9 @@ continue_reset:\n \t\t\tgoto reset_err;\n \n \t\ti40evf_irq_enable(adapter, true);\n-\t}\n+\t} else\n+\t\tadapter->state = __I40EVF_DOWN;\n+\n \treturn;\n reset_err:\n \tdev_err(&adapter->pdev->dev, \"failed to allocate resources during reinit\\n\");\n@@ -1667,6 +1684,11 @@ static void i40evf_adminq_task(struct work_struct *work)\n \t\t\tmemset(event.msg_buf, 0, I40EVF_MAX_AQ_BUF_SIZE);\n \t} while (pending);\n \n+\tif ((adapter->flags &\n+\t (I40EVF_FLAG_RESET_PENDING | I40EVF_FLAG_RESET_NEEDED)) ||\n+\t adapter->state == __I40EVF_RESETTING)\n+\t\tgoto freedom;\n+\n \t/* check for error indications */\n \tval = rd32(hw, hw->aq.arq.len);\n \toldval = val;\n@@ -1702,6 +1724,7 @@ static void i40evf_adminq_task(struct work_struct *work)\n \tif (oldval != val)\n \t\twr32(hw, hw->aq.asq.len, val);\n \n+freedom:\n \tkfree(event.msg_buf);\n out:\n \t/* re-enable Admin queue interrupt cause */\n@@ -1897,47 +1920,6 @@ static struct net_device_stats *i40evf_get_stats(struct net_device *netdev)\n }\n \n /**\n- * i40evf_reinit_locked - Software reinit\n- * @adapter: board private structure\n- *\n- * Reinititalizes the ring structures in response to a software configuration\n- * change. Roughly the same as close followed by open, but skips releasing\n- * and reallocating the interrupts.\n- **/\n-void i40evf_reinit_locked(struct i40evf_adapter *adapter)\n-{\n-\tstruct net_device *netdev = adapter->netdev;\n-\tint err;\n-\n-\tWARN_ON(in_interrupt());\n-\n-\ti40evf_down(adapter);\n-\n-\t/* allocate transmit descriptors */\n-\terr = i40evf_setup_all_tx_resources(adapter);\n-\tif (err)\n-\t\tgoto err_reinit;\n-\n-\t/* allocate receive descriptors */\n-\terr = i40evf_setup_all_rx_resources(adapter);\n-\tif (err)\n-\t\tgoto err_reinit;\n-\n-\ti40evf_configure(adapter);\n-\n-\terr = i40evf_up_complete(adapter);\n-\tif (err)\n-\t\tgoto err_reinit;\n-\n-\ti40evf_irq_enable(adapter, true);\n-\treturn;\n-\n-err_reinit:\n-\tdev_err(&adapter->pdev->dev, \"failed to allocate resources during reinit\\n\");\n-\ti40evf_close(netdev);\n-}\n-\n-/**\n * i40evf_change_mtu - Change the Maximum Transfer Unit\n * @netdev: network interface device structure\n * @new_mtu: new value for maximum frame size\n@@ -1952,9 +1934,10 @@ static int i40evf_change_mtu(struct net_device *netdev, int new_mtu)\n \tif ((new_mtu < 68) || (max_frame > I40E_MAX_RXBUFFER))\n \t\treturn -EINVAL;\n \n-\t/* must set new MTU before calling down or up */\n \tnetdev->mtu = new_mtu;\n-\ti40evf_reinit_locked(adapter);\n+\tadapter->flags |= I40EVF_FLAG_RESET_NEEDED;\n+\tschedule_work(&adapter->reset_task);\n+\n \treturn 0;\n }\n \n", "prefixes": [ "net", "v2" ] }