From patchwork Fri Jan 8 07:12:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005891 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA8B3C433E0 for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 77BAC233EA for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726919AbhAHHNZ (ORCPT ); Fri, 8 Jan 2021 02:13:25 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:50670 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726312AbhAHHNY (ORCPT ); Fri, 8 Jan 2021 02:13:24 -0500 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10872hAH030033 for ; Fri, 8 Jan 2021 02:12:42 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=Sb2IdOjLw6dAo8NrcBgpQUgR8Kw1O4Vy9qQ9qOWKT3g=; b=Qv9VQF2GCHo/fFSPb5KBtjh5WTXwZYWqHPoWPzA4LV7kAq51EUgIdHG77X4GeNAWT3EO 5iOdF1FDqnkZChwqMt/PqVeEPIPywXnYlAJ+o6nHQWFa8v64pavztr2sA9F/LUsTGbSb xTIrmuUh1hX6PkJJJFW6CDBF/4fkPwcvgD2nNIYmGcaIhMViavm8DrGsOVp1Ix2Ill4R m5yS9wprfyikQnbypLH1FWxd+9vJTB8SORTqsMeHqtwlTYC8+Pdr30PYf7Y2eQ4yfmaI nK6PDa3s5TsybZeyrvhcSENKE8I4JeQFg5i5McpOiXBCIEJ3fgoXK+tXOlWCe3rjMp3K fA== Received: from ppma03wdc.us.ibm.com (ba.79.3fa9.ip4.static.sl-reverse.com [169.63.121.186]) by mx0a-001b2d01.pphosted.com with ESMTP id 35xjm58dp7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:12:42 -0500 Received: from pps.filterd (ppma03wdc.us.ibm.com [127.0.0.1]) by ppma03wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1086rBqs009076 for ; Fri, 8 Jan 2021 07:12:41 GMT Received: from b03cxnp08026.gho.boulder.ibm.com (b03cxnp08026.gho.boulder.ibm.com [9.17.130.18]) by ppma03wdc.us.ibm.com with ESMTP id 35tgf9j4ge-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:41 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08026.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087CdYo22806984 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:39 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C09E178063; Fri, 8 Jan 2021 07:12:39 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DE22878060; Fri, 8 Jan 2021 07:12:38 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:38 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 1/7] ibmvnic: restore state in change-param reset Date: Thu, 7 Jan 2021 23:12:30 -0800 Message-Id: <20210108071236.123769-2-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 clxscore=1015 mlxlogscore=999 phishscore=0 lowpriorityscore=0 adultscore=0 impostorscore=0 suspectscore=0 mlxscore=0 bulkscore=0 malwarescore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080035 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Restore adapter state before returning from change-param reset. In case of errors, caller will try a hard-reset anyway. Fixes: 0cb4bc66ba5e ("ibmvnic: restore adapter state on failed reset") Signed-off-by: Sukadev Bhattiprolu --- drivers/net/ethernet/ibm/ibmvnic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index f302504faa8a..d548779561fd 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1960,7 +1960,7 @@ static int do_change_param_reset(struct ibmvnic_adapter *adapter, if (rc) { netdev_err(adapter->netdev, "Couldn't initialize crq. rc=%d\n", rc); - return rc; + goto out; } rc = ibmvnic_reset_init(adapter, true); From patchwork Fri Jan 8 07:12:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005899 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2FD53C4332B for ; Fri, 8 Jan 2021 07:13:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0CBA623447 for ; Fri, 8 Jan 2021 07:13:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727248AbhAHHNe (ORCPT ); Fri, 8 Jan 2021 02:13:34 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:58454 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726784AbhAHHNY (ORCPT ); Fri, 8 Jan 2021 02:13:24 -0500 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10874AQv111316 for ; Fri, 8 Jan 2021 02:12:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=+0fXlY1uKfkACIJFfn0hZPwItaU+frH94OcQFsCVPCY=; b=Ab64X0GqQXE7m9yqErGjEcxW3aV6KY53h+uZEf5tRKMR5zLZGbXa8nycMbfmEL7jTUB8 VVNVg8j6eDX1JmIS/b4UL8c7R6dhT22yQ8ONvmjywbID3M1il7/3UO8XKJsGMSIROGXn zBxluGG08cBS3UUMjrZorPnjaTtx7UvuGh6elrwuHtLuq+0DIlPz3lm6bAXOuRduyIZ9 wU7m+SiBfjZBJQjfXBUolLFf+mBn+XsYU02FKRKO/BdJA+vlbvgouuM/si+FrgRnkck6 vgrsocFBW2JwLvhkC4duU6AVl+WLEmNswFGw6lBQkCuHV4gHc0/D4+86a4/kXtPFlogQ xg== Received: from ppma01dal.us.ibm.com (83.d6.3fa9.ip4.static.sl-reverse.com [169.63.214.131]) by mx0a-001b2d01.pphosted.com with ESMTP id 35xj0a16w4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:12:43 -0500 Received: from pps.filterd (ppma01dal.us.ibm.com [127.0.0.1]) by ppma01dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1087BkGE021247 for ; Fri, 8 Jan 2021 07:12:42 GMT Received: from b03cxnp08027.gho.boulder.ibm.com (b03cxnp08027.gho.boulder.ibm.com [9.17.130.19]) by ppma01dal.us.ibm.com with ESMTP id 35tgfa4nyw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:42 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08027.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087Ce2611534866 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:40 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CEC1378064; Fri, 8 Jan 2021 07:12:40 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0CE7A7805C; Fri, 8 Jan 2021 07:12:40 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:39 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 2/7] ibmvnic: update reset function prototypes Date: Thu, 7 Jan 2021 23:12:31 -0800 Message-Id: <20210108071236.123769-3-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxlogscore=999 adultscore=0 clxscore=1011 mlxscore=0 lowpriorityscore=0 malwarescore=0 bulkscore=0 priorityscore=1501 spamscore=0 suspectscore=0 impostorscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080036 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org The reset functions need just the 'reset reason' parameter and not the ibmvnic_rwi list element. Update the functions so we can simplify the handling of the ->rwi_list in a follow-on patch. Fixes: 2770a7984db5 ("ibmvnic: Introduce hard reset recovery") Signed-off-by: Sukadev Bhattiprolu --- drivers/net/ethernet/ibm/ibmvnic.c | 39 ++++++++++++++++-------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index d548779561fd..cd8108dbddec 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1929,17 +1929,17 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) * events, or non-zero if we hit a fatal error and must halt. */ static int do_change_param_reset(struct ibmvnic_adapter *adapter, - struct ibmvnic_rwi *rwi, + enum ibmvnic_reset_reason reason, u32 reset_state) { struct net_device *netdev = adapter->netdev; int i, rc; netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n", - rwi->reset_reason); + reason); netif_carrier_off(netdev); - adapter->reset_reason = rwi->reset_reason; + adapter->reset_reason = reason; ibmvnic_cleanup(netdev); @@ -2015,7 +2015,7 @@ static int do_change_param_reset(struct ibmvnic_adapter *adapter, * non-zero if we hit a fatal error and must halt. */ static int do_reset(struct ibmvnic_adapter *adapter, - struct ibmvnic_rwi *rwi, u32 reset_state) + enum ibmvnic_reset_reason reason, u32 reset_state) { u64 old_num_rx_queues, old_num_tx_queues; u64 old_num_rx_slots, old_num_tx_slots; @@ -2025,7 +2025,7 @@ static int do_reset(struct ibmvnic_adapter *adapter, netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset reason %d, reset_state %d\n", adapter->state, adapter->failover_pending, - rwi->reset_reason, reset_state); + reason, reset_state); rtnl_lock(); /* @@ -2033,11 +2033,11 @@ static int do_reset(struct ibmvnic_adapter *adapter, * This will ensure ibmvnic_open() has either completed or will * block until failover is complete. */ - if (rwi->reset_reason == VNIC_RESET_FAILOVER) + if (reason == VNIC_RESET_FAILOVER) adapter->failover_pending = false; netif_carrier_off(netdev); - adapter->reset_reason = rwi->reset_reason; + adapter->reset_reason = reason; old_num_rx_queues = adapter->req_rx_queues; old_num_tx_queues = adapter->req_tx_queues; @@ -2188,16 +2188,16 @@ static int do_reset(struct ibmvnic_adapter *adapter, } static int do_hard_reset(struct ibmvnic_adapter *adapter, - struct ibmvnic_rwi *rwi, u32 reset_state) + enum ibmvnic_reset_reason reason, u32 reset_state) { struct net_device *netdev = adapter->netdev; int rc; netdev_dbg(adapter->netdev, "Hard resetting driver (%d)\n", - rwi->reset_reason); + reason); netif_carrier_off(netdev); - adapter->reset_reason = rwi->reset_reason; + adapter->reset_reason = reason; ibmvnic_cleanup(netdev); release_resources(adapter); @@ -2278,6 +2278,7 @@ static struct ibmvnic_rwi *get_next_rwi(struct ibmvnic_adapter *adapter) static void __ibmvnic_reset(struct work_struct *work) { + enum ibmvnic_reset_reason reason; struct ibmvnic_rwi *rwi; struct ibmvnic_adapter *adapter; bool saved_state = false; @@ -2294,6 +2295,7 @@ static void __ibmvnic_reset(struct work_struct *work) } rwi = get_next_rwi(adapter); + reason = rwi->reset_reason; while (rwi) { spin_lock_irqsave(&adapter->state_lock, flags); @@ -2311,9 +2313,9 @@ static void __ibmvnic_reset(struct work_struct *work) } spin_unlock_irqrestore(&adapter->state_lock, flags); - if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) { + if (reason == VNIC_RESET_CHANGE_PARAM) { /* CHANGE_PARAM requestor holds rtnl_lock */ - rc = do_change_param_reset(adapter, rwi, reset_state); + rc = do_change_param_reset(adapter, reason, reset_state); } else if (adapter->force_reset_recovery) { /* * Since we are doing a hard reset now, clear the @@ -2326,11 +2328,11 @@ static void __ibmvnic_reset(struct work_struct *work) if (adapter->wait_for_reset) { /* Previous was CHANGE_PARAM; caller locked */ adapter->force_reset_recovery = false; - rc = do_hard_reset(adapter, rwi, reset_state); + rc = do_hard_reset(adapter, reason, reset_state); } else { rtnl_lock(); adapter->force_reset_recovery = false; - rc = do_hard_reset(adapter, rwi, reset_state); + rc = do_hard_reset(adapter, reason, reset_state); rtnl_unlock(); } if (rc) { @@ -2341,9 +2343,9 @@ static void __ibmvnic_reset(struct work_struct *work) set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(60 * HZ); } - } else if (!(rwi->reset_reason == VNIC_RESET_FATAL && + } else if (!(reason == VNIC_RESET_FATAL && adapter->from_passive_init)) { - rc = do_reset(adapter, rwi, reset_state); + rc = do_reset(adapter, reason, reset_state); } kfree(rwi); adapter->last_reset_time = jiffies; @@ -2352,9 +2354,10 @@ static void __ibmvnic_reset(struct work_struct *work) netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc); rwi = get_next_rwi(adapter); + reason = rwi->reset_reason; - if (rwi && (rwi->reset_reason == VNIC_RESET_FAILOVER || - rwi->reset_reason == VNIC_RESET_MOBILITY)) + if (reason && (reason == VNIC_RESET_FAILOVER || + reason == VNIC_RESET_MOBILITY)) adapter->force_reset_recovery = true; } From patchwork Fri Jan 8 07:12:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005895 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CC255C433E6 for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 98B822343F for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727045AbhAHHN1 (ORCPT ); Fri, 8 Jan 2021 02:13:27 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:23814 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726788AbhAHHN0 (ORCPT ); Fri, 8 Jan 2021 02:13:26 -0500 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10873F9D178273 for ; Fri, 8 Jan 2021 02:12:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=CbWGvOtJyLJc0s3KpMU5cbKnA+s1tgLvLEFwJBJo6nw=; b=csAdIeIZbxnmclYboZykpzmsevoyQ3ahfgsPzeMBulk2RSoBbmZYleX6VI18sbgoFU97 Apj4P6SEzv+AL1xznUs8J5xIO5LMuJhoDItkCRoKs5NFu5adpAa6jvrZsN3gI00smJfY HL3OWsaVHTkOZeEajpstz2CDNYFbTnyCdcT+YXkH+zSiSirBjgLQpRfGvfHAl0I9MWK6 KYrJPU3gv4+JplDcl3JuHS/CzMU3Qc9Aj7fglQBFC7Tv3NzgBHR29bq9NP1G+rNstzCA 4Q1kU48FyX0m0x3SXedtkcRO1/b7lMDMHiROVeUinXtTBvekeMQgT+LhM9jvFeK9yucb Sg== Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0b-001b2d01.pphosted.com with ESMTP id 35xjkp8es9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:12:43 -0500 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1086r5fp022512 for ; Fri, 8 Jan 2021 07:12:43 GMT Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by ppma04wdc.us.ibm.com with ESMTP id 35tgfaa3hf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:43 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087CgER28115444 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:42 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EF6467805C; Fri, 8 Jan 2021 07:12:41 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 251FB7805F; Fri, 8 Jan 2021 07:12:41 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:40 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 3/7] ibmvnic: avoid allocating rwi entries Date: Thu, 7 Jan 2021 23:12:32 -0800 Message-Id: <20210108071236.123769-4-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxlogscore=999 clxscore=1015 impostorscore=0 lowpriorityscore=0 malwarescore=0 priorityscore=1501 suspectscore=0 mlxscore=0 spamscore=0 phishscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080036 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Whenever we need to schedule a reset, we allocate an rwi (reset work item?) entry and add to the list of pending resets. Since we only add one rwi for a given reason type to the list (no duplicates). we will only have a handful of reset types in the list - even in the worst case. In the common case we should just have a couple of entries at most. Rather than allocating/freeing every time (and dealing with the corner case of the allocation failing), use a fixed number of rwi entries. The extra memory needed is tiny and most of it will be used over the active life of the adapter. This also fixes a couple of tiny memory leaks. One is in ibmvnic_reset() where we don't free the rwi entries after deleting them from the list due to a transport event. The second is in __ibmvnic_reset() where if we find that the adapter is being removed, we simply break out of the loop (with rc = EBUSY) but ignore any rwi entries that remain on the list. Fixes: 2770a7984db58 ("Introduce hard reset recovery") Fixes: 36f1031c51a2 ("ibmvnic: Do not process reset during or after device removal") Signed-off-by: Sukadev Bhattiprolu --- drivers/net/ethernet/ibm/ibmvnic.c | 123 +++++++++++++++++------------ drivers/net/ethernet/ibm/ibmvnic.h | 14 ++-- 2 files changed, 78 insertions(+), 59 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index cd8108dbddec..d1c2aaed1478 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2257,29 +2257,81 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter, return rc; } -static struct ibmvnic_rwi *get_next_rwi(struct ibmvnic_adapter *adapter) +/** + * Next reset will always be the first on the list. + * When we take it off the list, we move any remaining resets so + * that the next one is again the first on the list. Most of the + * time the pending_resets[] should have a couple of types of resets + * (FAILOVER, TIMEOUT or CHANGE-PARAM and less often, MOBILITY). + */ +static enum ibmvnic_reset_reason get_pending_reset(struct ibmvnic_adapter *adapter) { - struct ibmvnic_rwi *rwi; + enum ibmvnic_reset_reason *pending_resets; + enum ibmvnic_reset_reason reason = 0; unsigned long flags; + int i; spin_lock_irqsave(&adapter->rwi_lock, flags); - if (!list_empty(&adapter->rwi_list)) { - rwi = list_first_entry(&adapter->rwi_list, struct ibmvnic_rwi, - list); - list_del(&rwi->list); - } else { - rwi = NULL; + pending_resets = &adapter->pending_resets[0]; + + reason = pending_resets[0]; + + if (reason) { + for (i = 0; i < adapter->next_reset; i++) { + pending_resets[i] = pending_resets[i+1]; + if (!pending_resets[i]) + break; + } + adapter->next_reset--; + } + + spin_unlock_irqrestore(&adapter->rwi_lock, flags); + return reason; +} + +/** + * Add a pending reset, making sure not to add duplicates. + * If @clear is set, clear all existing resets before adding. + * + * TODO: If clear (i.e force_reset_recovery) is true AND we have a + * duplicate reset, wouldn't it still make sense to clear the + * queue including the duplicate and add this reset? Preserving + * existing behavior for now. + */ +static void add_pending_reset(struct ibmvnic_adapter *adapter, + enum ibmvnic_reset_reason reason, + bool clear) +{ + enum ibmvnic_reset_reason *pending_resets; + unsigned long flags; + int i; + + spin_lock_irqsave(&adapter->rwi_lock, flags); + + pending_resets = &adapter->pending_resets[0]; + + for (i = 0; i < adapter->next_reset; i++) { + if (pending_resets[i] == reason) + goto out; + } + + if (clear) { + for (i = 0; i < adapter->next_reset; i++) { + pending_resets[i] = 0; + } + adapter->next_reset = 0; } + pending_resets[adapter->next_reset] = reason; + adapter->next_reset++; +out: spin_unlock_irqrestore(&adapter->rwi_lock, flags); - return rwi; } static void __ibmvnic_reset(struct work_struct *work) { enum ibmvnic_reset_reason reason; - struct ibmvnic_rwi *rwi; struct ibmvnic_adapter *adapter; bool saved_state = false; unsigned long flags; @@ -2294,15 +2346,13 @@ static void __ibmvnic_reset(struct work_struct *work) return; } - rwi = get_next_rwi(adapter); - reason = rwi->reset_reason; - while (rwi) { + reason = get_pending_reset(adapter); + while (reason) { spin_lock_irqsave(&adapter->state_lock, flags); if (adapter->state == VNIC_REMOVING || adapter->state == VNIC_REMOVED) { spin_unlock_irqrestore(&adapter->state_lock, flags); - kfree(rwi); rc = EBUSY; break; } @@ -2347,14 +2397,12 @@ static void __ibmvnic_reset(struct work_struct *work) adapter->from_passive_init)) { rc = do_reset(adapter, reason, reset_state); } - kfree(rwi); adapter->last_reset_time = jiffies; if (rc) netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc); - rwi = get_next_rwi(adapter); - reason = rwi->reset_reason; + reason = get_pending_reset(adapter); if (reason && (reason == VNIC_RESET_FAILOVER || reason == VNIC_RESET_MOBILITY)) @@ -2386,17 +2434,14 @@ static void __ibmvnic_delayed_reset(struct work_struct *work) static int ibmvnic_reset(struct ibmvnic_adapter *adapter, enum ibmvnic_reset_reason reason) { - struct list_head *entry, *tmp_entry; - struct ibmvnic_rwi *rwi, *tmp; struct net_device *netdev = adapter->netdev; - unsigned long flags; int ret; /* * If failover is pending don't schedule any other reset. * Instead let the failover complete. If there is already a * a failover reset scheduled, we will detect and drop the - * duplicate reset when walking the ->rwi_list below. + * duplicate reset when walking the ->pending_resets list. */ if (adapter->state == VNIC_REMOVING || adapter->state == VNIC_REMOVED || @@ -2412,36 +2457,11 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, goto err; } - spin_lock_irqsave(&adapter->rwi_lock, flags); - - list_for_each(entry, &adapter->rwi_list) { - tmp = list_entry(entry, struct ibmvnic_rwi, list); - if (tmp->reset_reason == reason) { - netdev_dbg(netdev, "Skipping matching reset, reason=%d\n", - reason); - spin_unlock_irqrestore(&adapter->rwi_lock, flags); - ret = EBUSY; - goto err; - } - } - - rwi = kzalloc(sizeof(*rwi), GFP_ATOMIC); - if (!rwi) { - spin_unlock_irqrestore(&adapter->rwi_lock, flags); - ibmvnic_close(netdev); - ret = ENOMEM; - goto err; - } - /* if we just received a transport event, - * flush reset queue and process this reset + /* If we just received a transport event, clear + * any pending resets and add just this reset. */ - if (adapter->force_reset_recovery && !list_empty(&adapter->rwi_list)) { - list_for_each_safe(entry, tmp_entry, &adapter->rwi_list) - list_del(entry); - } - rwi->reset_reason = reason; - list_add_tail(&rwi->list, &adapter->rwi_list); - spin_unlock_irqrestore(&adapter->rwi_lock, flags); + add_pending_reset(adapter, reason, adapter->force_reset_recovery); + netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason); schedule_work(&adapter->ibmvnic_reset); @@ -5363,7 +5383,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) INIT_WORK(&adapter->ibmvnic_reset, __ibmvnic_reset); INIT_DELAYED_WORK(&adapter->ibmvnic_delayed_reset, __ibmvnic_delayed_reset); - INIT_LIST_HEAD(&adapter->rwi_list); + adapter->next_reset = 0; + memset(&adapter->pending_resets, 0, sizeof(adapter->pending_resets)); spin_lock_init(&adapter->rwi_lock); spin_lock_init(&adapter->state_lock); mutex_init(&adapter->fw_lock); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index c09c3f6bba9f..1179a95a3f92 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -945,17 +945,14 @@ enum vnic_state {VNIC_PROBING = 1, VNIC_REMOVING, VNIC_REMOVED}; -enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1, +enum ibmvnic_reset_reason {VNIC_RESET_UNUSED = 0, + VNIC_RESET_FAILOVER = 1, VNIC_RESET_MOBILITY, VNIC_RESET_FATAL, VNIC_RESET_NON_FATAL, VNIC_RESET_TIMEOUT, - VNIC_RESET_CHANGE_PARAM}; - -struct ibmvnic_rwi { - enum ibmvnic_reset_reason reset_reason; - struct list_head list; -}; + VNIC_RESET_CHANGE_PARAM, + VNIC_RESET_MAX}; // must be last struct ibmvnic_tunables { u64 rx_queues; @@ -1082,7 +1079,8 @@ struct ibmvnic_adapter { enum vnic_state state; enum ibmvnic_reset_reason reset_reason; spinlock_t rwi_lock; - struct list_head rwi_list; + enum ibmvnic_reset_reason pending_resets[VNIC_RESET_MAX-1]; + short next_reset; struct work_struct ibmvnic_reset; struct delayed_work ibmvnic_delayed_reset; unsigned long resetting; From patchwork Fri Jan 8 07:12:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005903 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4291C433E0 for ; Fri, 8 Jan 2021 07:14:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A4F19233EE for ; Fri, 8 Jan 2021 07:14:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727294AbhAHHOO (ORCPT ); Fri, 8 Jan 2021 02:14:14 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:38410 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726474AbhAHHON (ORCPT ); Fri, 8 Jan 2021 02:14:13 -0500 Received: from pps.filterd (m0187473.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10873BV7127612 for ; Fri, 8 Jan 2021 02:13:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=zUPpMqrcfeQ6kYAe7PjNrCONlmchucD8YqtN3CK7V4E=; b=j8VguAAqFCBCaLLEB5LS1S4Y06NmqWCtQqtcuNWwG3DLTAcfYU0FXQwwh51NGtvRhrPs Vrs1UMhXmllFuDPmi5DoXLcxVSwb5x6RtpuN9Bx8br4U9vPjdkG9xjh8wDQq7Gv7/Urp z65ZKi41IN+RbnrGxO0NoSqOsirlBxCEy8L6Yl8fpDVL6w7kRFRGpiJcng/L5lYDriz7 z3smmowlBSYaiQ6kxUkfrC7Eqhv846EoRzArCSO7pqes2PoVr1Blzidr3Y+ghSVyia+u ZxdrQdL60JFobxCXftVEeVUy20tGPUsXD/QOOZZ9gsHOGY3m0BnUERxfxRE7ScUvpV7p Uw== Received: from ppma03wdc.us.ibm.com (ba.79.3fa9.ip4.static.sl-reverse.com [169.63.121.186]) by mx0a-001b2d01.pphosted.com with ESMTP id 35xjfarjnk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:13:31 -0500 Received: from pps.filterd (ppma03wdc.us.ibm.com [127.0.0.1]) by ppma03wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1086qvKR008827 for ; Fri, 8 Jan 2021 07:12:44 GMT Received: from b03cxnp08025.gho.boulder.ibm.com (b03cxnp08025.gho.boulder.ibm.com [9.17.130.17]) by ppma03wdc.us.ibm.com with ESMTP id 35tgf9j4gu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:44 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08025.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087Ch4020578654 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:43 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1443478064; Fri, 8 Jan 2021 07:12:43 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3BACC7805C; Fri, 8 Jan 2021 07:12:42 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:42 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 4/7] ibmvnic: switch order of checks in ibmvnic_reset Date: Thu, 7 Jan 2021 23:12:33 -0800 Message-Id: <20210108071236.123769-5-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 malwarescore=0 bulkscore=0 lowpriorityscore=0 suspectscore=0 mlxlogscore=999 clxscore=1015 priorityscore=1501 adultscore=0 spamscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080036 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org We check separately for REMOVING and PROBING in ibmvnic_reset(). Switch the order of checks to facilitate better locking when checking for REMOVING/REMOVED state. Fixes: 6a2fb0e99f9c ("ibmvnic: driver initialization for kdump/kexec") Signed-off-by: Sukadev Bhattiprolu --- drivers/net/ethernet/ibm/ibmvnic.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index d1c2aaed1478..ad551418ac63 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2437,6 +2437,12 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, struct net_device *netdev = adapter->netdev; int ret; + if (adapter->state == VNIC_PROBING) { + netdev_warn(netdev, "Adapter reset during probe\n"); + ret = adapter->init_done_rc = EAGAIN; + goto err; + } + /* * If failover is pending don't schedule any other reset. * Instead let the failover complete. If there is already a @@ -2451,12 +2457,6 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, goto err; } - if (adapter->state == VNIC_PROBING) { - netdev_warn(netdev, "Adapter reset during probe\n"); - ret = adapter->init_done_rc = EAGAIN; - goto err; - } - /* If we just received a transport event, clear * any pending resets and add just this reset. */ From patchwork Fri Jan 8 07:12:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005893 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4AC1C433E9 for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BA69E233FB for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727130AbhAHHN2 (ORCPT ); Fri, 8 Jan 2021 02:13:28 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:12232 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726312AbhAHHN1 (ORCPT ); Fri, 8 Jan 2021 02:13:27 -0500 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10872EO7071554 for ; Fri, 8 Jan 2021 02:12:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=l9VEHZNob2Wr8tif39ecPf6Fglpbho3Ep7XeMlW7RyQ=; b=mCi88BFi/ucCnbJ1YVmOiutf37Oew0Hj/AMTfdVqbOFoIvQzp1mXQ/v9wTMhNgdgiE5G 4h/7tUgNSuRZa8FmdyvuNaNcUR5pFmrlfwzv5l/XD4zHw+6t2yTyMiV5xmmwqGT3wHDU bYNoFSaVEzJuv93gwPC6BJdB6+Se1leGBW2LCLyMmKtoagUxZyKdRCgbXslDR574VmPR GNb71aHshifaNmN2iLYRikk7LQHO4IHe9ktf62dYPMwqwr7AYvucoKAaGbZwXOKAUJek d17RkmB80rQkmbwX9lK1Ah8igXFpyo4B30yrAZ3M2ZQDxUhj2PYaKDBbouoNLLczu1yW Pw== Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0b-001b2d01.pphosted.com with ESMTP id 35xjjbggd5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:12:45 -0500 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1086r5fq022512 for ; Fri, 8 Jan 2021 07:12:45 GMT Received: from b03cxnp07028.gho.boulder.ibm.com (b03cxnp07028.gho.boulder.ibm.com [9.17.130.15]) by ppma04wdc.us.ibm.com with ESMTP id 35tgfaa3hj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:45 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp07028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087CiNH28705054 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:44 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2DD507805C; Fri, 8 Jan 2021 07:12:44 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 54F7F78060; Fri, 8 Jan 2021 07:12:43 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:43 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 5/7] ibmvnic: use a lock to serialize remove/reset Date: Thu, 7 Jan 2021 23:12:34 -0800 Message-Id: <20210108071236.123769-6-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 mlxscore=0 lowpriorityscore=0 adultscore=0 phishscore=0 mlxlogscore=999 impostorscore=0 spamscore=0 clxscore=1015 malwarescore=0 bulkscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080035 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Use a separate lock to serialze ibmvnic_reset() and ibmvnic_remove() functions. ibmvnic_reset() schedules work for the worker thread and ibmvnic_remove() flushes the work before removing the adapter. We don't want any work to be scheduled once we start removing the adapter (i.e after we have already flushed the work). A follow-on patch will convert the ->state_lock from a spinklock to a mutex to allow us to hold it for longer periods of time. ibmvnic_reset() can be called from a tasklet and cannot use the mutex. Fixes: 6954a9e4192b ("ibmvnic: Flush existing work items before device removal") Signed-off-by: Sukadev Bhattiprolu --- drivers/net/ethernet/ibm/ibmvnic.c | 16 +++++++++++++++- drivers/net/ethernet/ibm/ibmvnic.h | 2 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index ad551418ac63..c7675ab0b7e3 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2435,6 +2435,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, enum ibmvnic_reset_reason reason) { struct net_device *netdev = adapter->netdev; + unsigned long rmflags; int ret; if (adapter->state == VNIC_PROBING) { @@ -2443,6 +2444,8 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, goto err; } + spin_lock_irqsave(&adapter->remove_lock, rmflags); + /* * If failover is pending don't schedule any other reset. * Instead let the failover complete. If there is already a @@ -2465,8 +2468,9 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason); schedule_work(&adapter->ibmvnic_reset); - return 0; + ret = 0; err: + spin_unlock_irqrestore(&adapter->remove_lock, rmflags); return -ret; } @@ -5387,6 +5391,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) memset(&adapter->pending_resets, 0, sizeof(adapter->pending_resets)); spin_lock_init(&adapter->rwi_lock); spin_lock_init(&adapter->state_lock); + spin_lock_init(&adapter->remove_lock); mutex_init(&adapter->fw_lock); init_completion(&adapter->init_done); init_completion(&adapter->fw_done); @@ -5459,6 +5464,7 @@ static int ibmvnic_remove(struct vio_dev *dev) { struct net_device *netdev = dev_get_drvdata(&dev->dev); struct ibmvnic_adapter *adapter = netdev_priv(netdev); + unsigned long rmflags; unsigned long flags; spin_lock_irqsave(&adapter->state_lock, flags); @@ -5467,7 +5473,15 @@ static int ibmvnic_remove(struct vio_dev *dev) return -EBUSY; } + /* If ibmvnic_reset() is scheduling a reset, wait for it to + * finish. Then prevent it from scheduling any more resets + * and have the reset functions ignore any resets that have + * already been scheduled. + */ + spin_lock_irqsave(&adapter->remove_lock, rmflags); adapter->state = VNIC_REMOVING; + spin_unlock_irqrestore(&adapter->remove_lock, rmflags); + spin_unlock_irqrestore(&adapter->state_lock, flags); flush_work(&adapter->ibmvnic_reset); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 1179a95a3f92..2779696ade09 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -1081,6 +1081,8 @@ struct ibmvnic_adapter { spinlock_t rwi_lock; enum ibmvnic_reset_reason pending_resets[VNIC_RESET_MAX-1]; short next_reset; + /* serialize ibmvnic_reset() and ibmvnic_remove() */ + spinlock_t remove_lock; struct work_struct ibmvnic_reset; struct delayed_work ibmvnic_delayed_reset; unsigned long resetting; From patchwork Fri Jan 8 07:12:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005897 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D191C43381 for ; Fri, 8 Jan 2021 07:13:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D15192343F for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727172AbhAHHNa (ORCPT ); Fri, 8 Jan 2021 02:13:30 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:40502 "EHLO mx0b-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727049AbhAHHN3 (ORCPT ); Fri, 8 Jan 2021 02:13:29 -0500 Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10876aVr089925 for ; Fri, 8 Jan 2021 02:12:47 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=y25KOf5VlPZnzIjcmectb2cjGvsMVrqOrIeV6f8w1tI=; b=LrsULCKbOBZPDD2O5i9Ii9uOU5ioHbqiXswqN2lfltdg0ZcpRqi09AduGAMvp3dvE97H FPZFiUxz4EcwB9rRjP6H0SGBSW+8+K/aDFEyWg/o/UQVu0N53nPC62ZNYAhILK3MtBbh +1NvIGVuElzakA6SdpIloY0a8MGY0UlLCC9Ltgb+tghk2PKQ6ZjHq/TFMis8Yl1yL9rq 6o6RykbDGYF0/jdeV4MeIWGYr4c7pmyOMMPYFriRjjIF/Ueg7IhNfVOL2D9U6KgFvkU1 dZYz/DugS7pJ37ZAJcxTog1apjod9aQotcxt7nwnYZtdkivpyLr3URxHRcfnjwumriSF kw== Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0a-001b2d01.pphosted.com with ESMTP id 35xh4ujdba-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:12:47 -0500 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1087CJ4O014385 for ; Fri, 8 Jan 2021 07:12:46 GMT Received: from b03cxnp08026.gho.boulder.ibm.com (b03cxnp08026.gho.boulder.ibm.com [9.17.130.18]) by ppma04dal.us.ibm.com with ESMTP id 35tgfacn9b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:46 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08026.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087Cjrs19595544 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:45 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 47A0878067; Fri, 8 Jan 2021 07:12:45 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6D97E7805C; Fri, 8 Jan 2021 07:12:44 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:44 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 6/7] ibmvnic: check adapter->state under state_lock Date: Thu, 7 Jan 2021 23:12:35 -0800 Message-Id: <20210108071236.123769-7-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 mlxscore=0 impostorscore=0 malwarescore=0 phishscore=0 spamscore=0 priorityscore=1501 clxscore=1015 lowpriorityscore=0 mlxlogscore=999 suspectscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080036 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Consider following code from __ibmvnic_reset() spin_lock_irqsave(&adapter->state_lock, flags); if (adapter->state == VNIC_REMOVING || adapter->state == VNIC_REMOVED) { spin_unlock_irqrestore(&adapter->state_lock, flags); kfree(rwi); rc = EBUSY; break; } if (!saved_state) { reset_state = adapter->state; saved_state = true; } spin_unlock_irqrestore(&adapter->state_lock, flags); and following from ibmvnic_open(): if (adapter->failover_pending) { adapter->state = VNIC_OPEN; return 0; } They have following issues: a. __ibmvnic_reset() caches the adapter->state while holding the state_lock but ibmvnic_open() sets state to OPEN without holding a lock. b. Even if adapter state changes to OPEN after __ibmvnic_reset() cached the state but before reset begins, the reset process will leave the adapter in PROBED state instead of OPEN state. The reason current code caches the adapter state is so we know what state to go back to if the reset fails. But due to recent bug fixes, the reset functions __restore__ the adapter state on both success/failure, so we no longer need to cache the state. To fix the race condition b above, use ->state_lock more consistently and throughout the open, close and reset functions. But since these may have to block, change the ->state_lock from a spinlock to mutex. A follow-on patch will audit/document the uses of ->state field outside open/close/reset. Thanks to a lot of input from Dany Madden, Lijun Pan and Rick Lindsley. Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during device reset") Signed-off-by: Sukadev Bhattiprolu --- --- drivers/net/ethernet/ibm/ibmvnic.c | 119 ++++++++++++++++++++--------- drivers/net/ethernet/ibm/ibmvnic.h | 5 +- 2 files changed, 87 insertions(+), 37 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index c7675ab0b7e3..236ec2456a38 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1150,6 +1150,8 @@ static int __ibmvnic_open(struct net_device *netdev) enum vnic_state prev_state = adapter->state; int i, rc; + WARN_ON_ONCE(!mutex_is_locked(&adapter->state_lock)); + adapter->state = VNIC_OPENING; replenish_pools(adapter); ibmvnic_napi_enable(adapter); @@ -1196,11 +1198,14 @@ static int ibmvnic_open(struct net_device *netdev) struct ibmvnic_adapter *adapter = netdev_priv(netdev); int rc; + mutex_lock(&adapter->state_lock); + /* If device failover is pending, just set device state and return. * Device operation will be handled by reset routine. */ if (adapter->failover_pending) { adapter->state = VNIC_OPEN; + mutex_unlock(&adapter->state_lock); return 0; } @@ -1228,6 +1233,8 @@ static int ibmvnic_open(struct net_device *netdev) adapter->state = VNIC_OPEN; rc = 0; } + + mutex_unlock(&adapter->state_lock); return rc; } @@ -1350,6 +1357,8 @@ static int __ibmvnic_close(struct net_device *netdev) struct ibmvnic_adapter *adapter = netdev_priv(netdev); int rc = 0; + WARN_ON_ONCE(!mutex_is_locked(&adapter->state_lock)); + adapter->state = VNIC_CLOSING; rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN); if (rc) @@ -1363,6 +1372,8 @@ static int ibmvnic_close(struct net_device *netdev) struct ibmvnic_adapter *adapter = netdev_priv(netdev); int rc; + mutex_lock(&adapter->state_lock); + netdev_dbg(netdev, "[S:%d FOP:%d FRR:%d] Closing\n", adapter->state, adapter->failover_pending, adapter->force_reset_recovery); @@ -1372,12 +1383,15 @@ static int ibmvnic_close(struct net_device *netdev) */ if (adapter->failover_pending) { adapter->state = VNIC_CLOSED; + mutex_unlock(&adapter->state_lock); return 0; } + rc = __ibmvnic_close(netdev); ibmvnic_cleanup(netdev); + mutex_unlock(&adapter->state_lock); return rc; } @@ -1929,15 +1943,24 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) * events, or non-zero if we hit a fatal error and must halt. */ static int do_change_param_reset(struct ibmvnic_adapter *adapter, - enum ibmvnic_reset_reason reason, - u32 reset_state) + enum ibmvnic_reset_reason reason) { struct net_device *netdev = adapter->netdev; + u32 reset_state; int i, rc; netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n", reason); + mutex_lock(&adapter->state_lock); + + reset_state = adapter->state; + if (reset_state == VNIC_REMOVING || reset_state == VNIC_REMOVED) { + netdev_err(netdev, "Adapter removed before change-param!\n"); + rc = IBMVNIC_NODEV; + goto out; + } + netif_carrier_off(netdev); adapter->reset_reason = reason; @@ -2007,6 +2030,9 @@ static int do_change_param_reset(struct ibmvnic_adapter *adapter, out: if (rc) adapter->state = reset_state; + + mutex_unlock(&adapter->state_lock); + return rc; } @@ -2015,19 +2041,31 @@ static int do_change_param_reset(struct ibmvnic_adapter *adapter, * non-zero if we hit a fatal error and must halt. */ static int do_reset(struct ibmvnic_adapter *adapter, - enum ibmvnic_reset_reason reason, u32 reset_state) + enum ibmvnic_reset_reason reason) { + struct net_device *netdev = adapter->netdev; u64 old_num_rx_queues, old_num_tx_queues; u64 old_num_rx_slots, old_num_tx_slots; - struct net_device *netdev = adapter->netdev; + u32 reset_state; int i, rc; + rtnl_lock(); + + mutex_lock(&adapter->state_lock); + + reset_state = adapter->state; + netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset reason %d, reset_state %d\n", adapter->state, adapter->failover_pending, reason, reset_state); - rtnl_lock(); + if (reset_state == VNIC_REMOVING || reset_state == VNIC_REMOVED) { + netdev_err(netdev, "Adapter removed before reset!\n"); + rc = IBMVNIC_NODEV; + goto out; + } + /* * Now that we have the rtnl lock, clear any pending failover. * This will ensure ibmvnic_open() has either completed or will @@ -2054,11 +2092,21 @@ static int do_reset(struct ibmvnic_adapter *adapter, /* Release the RTNL lock before link state change and * re-acquire after the link state change to allow * linkwatch_event to grab the RTNL lock and run during - * a reset. + * a reset. To reacquire RTNL, we must also drop/reacquire + * state_lock. Once we reacquire state_lock, we don't need + * to check for REMOVING since ->resetting bit is still set + * (any ibmvnic_remove() in between would have failed). + * + * We set the state to CLOSING above. If adapter is no + * longer in CLOSING state, another thread changed the + * state when we dropped the lock, so fail the reset + * and retry. */ + mutex_unlock(&adapter->state_lock); rtnl_unlock(); rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN); rtnl_lock(); + mutex_lock(&adapter->state_lock); if (rc) goto out; @@ -2180,6 +2228,8 @@ static int do_reset(struct ibmvnic_adapter *adapter, /* restore the adapter state if reset failed */ if (rc) adapter->state = reset_state; + + mutex_unlock(&adapter->state_lock); rtnl_unlock(); netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset done, rc %d\n", @@ -2188,11 +2238,24 @@ static int do_reset(struct ibmvnic_adapter *adapter, } static int do_hard_reset(struct ibmvnic_adapter *adapter, - enum ibmvnic_reset_reason reason, u32 reset_state) + enum ibmvnic_reset_reason reason) { struct net_device *netdev = adapter->netdev; + u32 reset_state; int rc; + WARN_ON_ONCE(!rtnl_is_locked()); + + mutex_lock(&adapter->state_lock); + + reset_state = adapter->state; + + if (reset_state == VNIC_REMOVING || reset_state == VNIC_REMOVED) { + netdev_err(netdev, "Adapter removed before hard reset!\n"); + rc = IBMVNIC_NODEV; + goto out; + } + netdev_dbg(adapter->netdev, "Hard resetting driver (%d)\n", reason); @@ -2254,6 +2317,7 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter, adapter->state = reset_state; netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Hard reset done, rc %d\n", adapter->state, adapter->failover_pending, rc); + mutex_unlock(&adapter->state_lock); return rc; } @@ -2333,9 +2397,6 @@ static void __ibmvnic_reset(struct work_struct *work) { enum ibmvnic_reset_reason reason; struct ibmvnic_adapter *adapter; - bool saved_state = false; - unsigned long flags; - u32 reset_state; int rc = 0; adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset); @@ -2348,24 +2409,9 @@ static void __ibmvnic_reset(struct work_struct *work) reason = get_pending_reset(adapter); while (reason) { - spin_lock_irqsave(&adapter->state_lock, flags); - - if (adapter->state == VNIC_REMOVING || - adapter->state == VNIC_REMOVED) { - spin_unlock_irqrestore(&adapter->state_lock, flags); - rc = EBUSY; - break; - } - - if (!saved_state) { - reset_state = adapter->state; - saved_state = true; - } - spin_unlock_irqrestore(&adapter->state_lock, flags); - if (reason == VNIC_RESET_CHANGE_PARAM) { /* CHANGE_PARAM requestor holds rtnl_lock */ - rc = do_change_param_reset(adapter, reason, reset_state); + rc = do_change_param_reset(adapter, reason); } else if (adapter->force_reset_recovery) { /* * Since we are doing a hard reset now, clear the @@ -2378,11 +2424,11 @@ static void __ibmvnic_reset(struct work_struct *work) if (adapter->wait_for_reset) { /* Previous was CHANGE_PARAM; caller locked */ adapter->force_reset_recovery = false; - rc = do_hard_reset(adapter, reason, reset_state); + rc = do_hard_reset(adapter, reason); } else { rtnl_lock(); adapter->force_reset_recovery = false; - rc = do_hard_reset(adapter, reason, reset_state); + rc = do_hard_reset(adapter, reason); rtnl_unlock(); } if (rc) { @@ -2395,12 +2441,16 @@ static void __ibmvnic_reset(struct work_struct *work) } } else if (!(reason == VNIC_RESET_FATAL && adapter->from_passive_init)) { - rc = do_reset(adapter, reason, reset_state); + rc = do_reset(adapter, reason); } adapter->last_reset_time = jiffies; if (rc) netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc); + if (rc == IBMVNIC_NODEV) { + rc = EBUSY; + break; + } reason = get_pending_reset(adapter); @@ -5390,7 +5440,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) adapter->next_reset = 0; memset(&adapter->pending_resets, 0, sizeof(adapter->pending_resets)); spin_lock_init(&adapter->rwi_lock); - spin_lock_init(&adapter->state_lock); + mutex_init(&adapter->state_lock); spin_lock_init(&adapter->remove_lock); mutex_init(&adapter->fw_lock); init_completion(&adapter->init_done); @@ -5465,11 +5515,10 @@ static int ibmvnic_remove(struct vio_dev *dev) struct net_device *netdev = dev_get_drvdata(&dev->dev); struct ibmvnic_adapter *adapter = netdev_priv(netdev); unsigned long rmflags; - unsigned long flags; - spin_lock_irqsave(&adapter->state_lock, flags); + mutex_lock(&adapter->state_lock); if (test_bit(0, &adapter->resetting)) { - spin_unlock_irqrestore(&adapter->state_lock, flags); + mutex_unlock(&adapter->state_lock); return -EBUSY; } @@ -5482,7 +5531,7 @@ static int ibmvnic_remove(struct vio_dev *dev) adapter->state = VNIC_REMOVING; spin_unlock_irqrestore(&adapter->remove_lock, rmflags); - spin_unlock_irqrestore(&adapter->state_lock, flags); + mutex_unlock(&adapter->state_lock); flush_work(&adapter->ibmvnic_reset); flush_delayed_work(&adapter->ibmvnic_delayed_reset); @@ -5498,7 +5547,7 @@ static int ibmvnic_remove(struct vio_dev *dev) release_stats_buffers(adapter); adapter->state = VNIC_REMOVED; - + mutex_destroy(&adapter->state_lock); rtnl_unlock(); mutex_destroy(&adapter->fw_lock); device_remove_file(&dev->dev, &dev_attr_failover); diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 2779696ade09..ac79dfa76333 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -21,6 +21,7 @@ #define IBMVNIC_STATS_TIMEOUT 1 #define IBMVNIC_INIT_FAILED 2 #define IBMVNIC_OPEN_FAILED 3 +#define IBMVNIC_NODEV 4 /* basic structures plus 100 2k buffers */ #define IBMVNIC_IO_ENTITLEMENT_DEFAULT 610305 @@ -1097,6 +1098,6 @@ struct ibmvnic_adapter { struct ibmvnic_tunables desired; struct ibmvnic_tunables fallback; - /* Used for serializatin of state field */ - spinlock_t state_lock; + /* Used for serialization of state field */ + struct mutex state_lock; }; From patchwork Fri Jan 8 07:12:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 12005901 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39630C4332E for ; Fri, 8 Jan 2021 07:13:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E84EA23716 for ; Fri, 8 Jan 2021 07:13:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727214AbhAHHNb (ORCPT ); Fri, 8 Jan 2021 02:13:31 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:27664 "EHLO mx0b-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726312AbhAHHNa (ORCPT ); Fri, 8 Jan 2021 02:13:30 -0500 Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10877Fe1032764 for ; Fri, 8 Jan 2021 02:12:49 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=7JnI0BcSSehVPyjrAi23FDxhEH/ka+SJFy+f0BDmxGA=; b=qiLeNM6MZmiI1udZPJTGA8rGWoQiQjupG4udUhWDDjdI6K6QCPuWO/x+zjQ7OCA1ZFiz +WASExP2pH7uosDrV3V5JC1W12uLdv6lQm9YjApsxtaAj0QHguDoZL9am61MqOizopQ2 qIEwOliwMQyfr3+6PEyr24fmrJwpSEmhtc2vcI3uasbJ7CeAXrTzM38uAIpzGOCHVboM Hz8hDhHZf2Wno0Yla/+jZ3Jb1Ik9tU2UR2g4Nf4NVaTWSNue7ICALmsWnTpiDedNN4iB RK5UvLPeogMRm8kGIk06U5xZf+ojKDf4K1r2J/BJf9IgJQgoxg7Ng0qIwu/ASjVrvNhw Pw== Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 35xj600t7m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 02:12:48 -0500 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 1086rPlG022574 for ; Fri, 8 Jan 2021 07:12:47 GMT Received: from b03cxnp08027.gho.boulder.ibm.com (b03cxnp08027.gho.boulder.ibm.com [9.17.130.19]) by ppma04wdc.us.ibm.com with ESMTP id 35tgfaa3hq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 08 Jan 2021 07:12:47 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08027.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1087CkZK9503386 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 8 Jan 2021 07:12:46 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5E51D7805C; Fri, 8 Jan 2021 07:12:46 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 870C778067; Fri, 8 Jan 2021 07:12:45 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.139.161]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Fri, 8 Jan 2021 07:12:45 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , sukadev@linux.ibm.com Subject: [PATCH 7/7] ibmvnic: add comments about adapter->state_lock Date: Thu, 7 Jan 2021 23:12:36 -0800 Message-Id: <20210108071236.123769-8-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210108071236.123769-1-sukadev@linux.ibm.com> References: <20210108071236.123769-1-sukadev@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737 definitions=2021-01-08_04:2021-01-07,2021-01-08 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 lowpriorityscore=0 suspectscore=0 impostorscore=0 mlxscore=0 bulkscore=0 mlxlogscore=999 spamscore=0 malwarescore=0 priorityscore=1501 clxscore=1015 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101080035 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Add some comments, notes and TODOs about ->state_lock and RTNL. Signed-off-by: Sukadev Bhattiprolu --- Note: This is fixing lot of comments so not identifying fixes. It "seems" to fit this patch set but can send to net-next if necessary. drivers/net/ethernet/ibm/ibmvnic.c | 58 ++++++++++++++++++++++++++++++ drivers/net/ethernet/ibm/ibmvnic.h | 51 +++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 236ec2456a38..1aae730ddafd 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1202,6 +1202,14 @@ static int ibmvnic_open(struct net_device *netdev) /* If device failover is pending, just set device state and return. * Device operation will be handled by reset routine. + * + * Note that ->failover_pending is not protected by ->state_lock + * because the tasklet (executing ibmvnic_handle_crq()) cannot + * block. Even otherwise this can deadlock due to CRQs issued in + * ibmvnic_open(). + * + * We check failover_pending again at the end in case of errors. + * so its okay if we miss the change to true here. */ if (adapter->failover_pending) { adapter->state = VNIC_OPEN; @@ -1380,6 +1388,9 @@ static int ibmvnic_close(struct net_device *netdev) /* If device failover is pending, just set device state and return. * Device operation will be handled by reset routine. + * + * Note that ->failover_pending is not protected by ->state_lock + * See comments in ibmvnic_open(). */ if (adapter->failover_pending) { adapter->state = VNIC_CLOSED; @@ -1930,6 +1941,14 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) if (!is_valid_ether_addr(addr->sa_data)) return -EADDRNOTAVAIL; + /* + * TODO: Can this race with a reset? The reset could briefly + * set state to PROBED causing us to skip setting the + * mac address. When reset complets, we set the old mac + * address? Can we check ->resetting bit instead and + * save the new mac address in adapter->mac_addr + * so reset function can set it when it is done? + */ if (adapter->state != VNIC_PROBED) { ether_addr_copy(adapter->mac_addr, addr->sa_data); rc = __ibmvnic_set_mac(netdev, addr->sa_data); @@ -1941,6 +1960,14 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p) /** * do_change_param_reset returns zero if we are able to keep processing reset * events, or non-zero if we hit a fatal error and must halt. + * + * Notes: + * - Regardless of success/failure, this function restores adapter state + * to what as it was on entry. In case of failure, it is assumed that + * a new hard-reset will be attempted. + * - Caller must hold the rtnl lock before calling and release upon + * return. + * */ static int do_change_param_reset(struct ibmvnic_adapter *adapter, enum ibmvnic_reset_reason reason) @@ -2039,6 +2066,11 @@ static int do_change_param_reset(struct ibmvnic_adapter *adapter, /** * do_reset returns zero if we are able to keep processing reset events, or * non-zero if we hit a fatal error and must halt. + * + * Notes: + * - Regardless of success/failure, this function restores adapter state + * to what as it was on entry. In case of failure, it is assumed that + * a new hard-reset will be attempted. */ static int do_reset(struct ibmvnic_adapter *adapter, enum ibmvnic_reset_reason reason) @@ -2237,6 +2269,17 @@ static int do_reset(struct ibmvnic_adapter *adapter, return rc; } +/** + * Perform a hard reset possibly because a prior reset encountered + * an error. + * + * Notes: + * - Regardless of success/failure, this function restores adapter state + * to what as it was on entry. In case of failure, it is assumed that + * a new hard-reset will be attempted. + * - Caller must hold the rtnl lock before calling and release upon + * return. + */ static int do_hard_reset(struct ibmvnic_adapter *adapter, enum ibmvnic_reset_reason reason) { @@ -2651,6 +2694,11 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget) frames_processed++; } + /* TODO: Can this race with reset and/or is release_rx_pools()? + * Is that why we check for VNIC_CLOSING? What if we go to + * CLOSING just after we check? We cannot take ->state_lock + * since we are in interrupt context. + */ if (adapter->state != VNIC_CLOSING && ((atomic_read(&adapter->rx_pool[scrq_num].available) < adapter->req_rx_add_entries_per_subcrq / 2) || @@ -5358,6 +5406,9 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter, bool reset) } if (adapter->from_passive_init) { + /* ibmvnic_reset_init() is always called with ->state_lock + * held except from ibmvnic_probe(), so safe to update state. + */ adapter->state = VNIC_OPEN; adapter->from_passive_init = false; return -1; @@ -5531,6 +5582,9 @@ static int ibmvnic_remove(struct vio_dev *dev) adapter->state = VNIC_REMOVING; spin_unlock_irqrestore(&adapter->remove_lock, rmflags); + /* drop state_lock so __ibmvnic_reset() can make progress + * during flush_work() + */ mutex_unlock(&adapter->state_lock); flush_work(&adapter->ibmvnic_reset); @@ -5546,6 +5600,9 @@ static int ibmvnic_remove(struct vio_dev *dev) release_stats_token(adapter); release_stats_buffers(adapter); + /* Adapter going away. There better be no one checking ->state + * or getting state_lock now TODO: Do we need the REMOVED state? + */ adapter->state = VNIC_REMOVED; mutex_destroy(&adapter->state_lock); rtnl_unlock(); @@ -5627,6 +5684,7 @@ static int ibmvnic_resume(struct device *dev) struct net_device *netdev = dev_get_drvdata(dev); struct ibmvnic_adapter *adapter = netdev_priv(netdev); + /* resuming from power-down so ignoring state_lock */ if (adapter->state != VNIC_OPEN) return 0; diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index ac79dfa76333..d79bc9444c9f 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -963,6 +963,55 @@ struct ibmvnic_tunables { u64 mtu; }; +/** + * ->state_lock: + * Mutex to serialize read/write of adapter->state field specially + * between open and reset functions. If rtnl also needs to be held, + * acquire rtnl first and then state_lock (to be consistent with + * functions that enter ibmvnic with rtnl already held). + * + * In general, ->state_lock must be held for all read/writes to the + * state field. Exceptions are: + * - checks for VNIC_PROBING state - since the adapter is itself + * under construction and because we never go _back_ to PROBING. + * + * - in debug messages involving ->state + * + * - in ibmvnic_tx_interrupt(), ibmvnic_rx_interrupt() because + * a) this is a mutex and b) no (known) impact of getting a stale + * state (i.e we will likely recover on the next interrupt). + * + * - ibmvnic_resume() - we are resuming from a power-down state? + * + * - ibmvnic_reset() - see ->remove_lock below. + * + * Besides these, there are couple of TODOs in ibmvnic_poll() and + * ibmvnic_set_mac() that need to be investigated separately. + * + * ->remove_lock + * A spin lock used to serialize ibmvnic_reset() and ibmvnic_remove(). + * ibmvnic_reset() can be called from a tasklet which cannot block, + * so it cannot use the ->state_lock. The only states ibmvnic_reset() + * checks for are PROBING, REMOVING and REMOVED. PROBING can be ignored + * as mentioned above. On entering REMOVING state, ibmvnic_reset() + * will skip scheduling resets for the adapter. + * + * ->pending_resets[], ->next_reset: + * A "queue" of pending resets, implemented as a simple array. Resets + * are not frequent and even when they do occur, we will usually have + * just 1 or 2 entries in the queue at any time. Note that we don't + * need/allow duplicates in the queue. In the worst case, we would have + * VNIC_RESET_MAX-1 entries (but that means adapter is processing all + * valid types of resets at once!) so the slight overhead of the array + * is probably acceptable. + * + * We could still use a linked list but then have to deal with allocation + * failure when scheduling a reset. We sometimes enqueue reset from a + * tasklet so cannot block when we have allocation failure. Trying to + * close the adapter on failure requires us to hold the state_lock, which + * then cannot be a mutex (tasklet cannot block) - i.e complicates locking + * just for the occasional memory allocation failure. + */ struct ibmvnic_adapter { struct vio_dev *vdev; struct net_device *netdev; @@ -1098,6 +1147,6 @@ struct ibmvnic_adapter { struct ibmvnic_tunables desired; struct ibmvnic_tunables fallback; - /* Used for serialization of state field */ + /* see block comments above */ struct mutex state_lock; };