From patchwork Tue Jan 12 18:14: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: 362690 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 7CC3CC433E0 for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2D1CA22DFA for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406650AbhALSPb (ORCPT ); Tue, 12 Jan 2021 13:15:31 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:58122 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2391603AbhALSP0 (ORCPT ); Tue, 12 Jan 2021 13:15:26 -0500 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10CI1sFE060364 for ; Tue, 12 Jan 2021 13:14:45 -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=VpHdOnReA7ON5QlILoZ5RYiKQe+8MTDgSWA9wjvFBBo=; b=XvhID2HYVeEKd+pXdamyhxt7DOii8Kcb/5iwx6IHTgPXTcdTS7nR94MNopZfSDeA3b1r 0f49AqLgKouV7C82xoAeY5zlSBD3dFvCTnfMQpHQvVuutJLdbobSz3KUt+V1EmiF2eqx yrhVCEdV3uOtH8n54HxEAfUVzfHj/kzwn1Mue3Z8Lfs+qqIQ5zZ+A5tbYGATh/646SwZ s3+IzEimeXU5+j0z8UVYfMPFB9HLWTV5qIGx6tqttYX8naORn5PxPNi+GD4Z+ddQa2LK 47xSQtG+F32iRUo7mPUGPfujDH8n8/qTDqKrbXSVZkWIAWwYFh4w8fR3e5zvetdVM/BO /Q== Received: from ppma05wdc.us.ibm.com (1b.90.2fa9.ip4.static.sl-reverse.com [169.47.144.27]) by mx0b-001b2d01.pphosted.com with ESMTP id 361fh62u03-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:45 -0500 Received: from pps.filterd (ppma05wdc.us.ibm.com [127.0.0.1]) by ppma05wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 10CI8CJo006132 for ; Tue, 12 Jan 2021 18:14:44 GMT Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by ppma05wdc.us.ibm.com with ESMTP id 35y448yw79-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:44 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIEidV28115356 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:44 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0C176B206B; Tue, 12 Jan 2021 18:14:44 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5A325B2070; Tue, 12 Jan 2021 18:14:43 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:43 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 1/7] ibmvnic: restore state in change-param reset Date: Tue, 12 Jan 2021 10:14:35 -0800 Message-Id: <20210112181441.206545-2-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_12:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 bulkscore=0 phishscore=0 lowpriorityscore=0 spamscore=0 mlxlogscore=999 malwarescore=0 suspectscore=0 priorityscore=1501 impostorscore=0 clxscore=1015 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120103 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.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 Tue Jan 12 18:14: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: 361660 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 A95F8C433DB for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 551DE2311D for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406691AbhALSPc (ORCPT ); Tue, 12 Jan 2021 13:15:32 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:59276 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2392090AbhALSP1 (ORCPT ); Tue, 12 Jan 2021 13:15: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 10CICoN7058322 for ; Tue, 12 Jan 2021 13:14: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=PhpKVwed9z+R5dX0zRkPuI404i6VnpqDNtQmMLdIf6I=; b=Blff56MXEnH9r5KZVrnGWN++TN4UmWcShhHssGqSpWuo7S5rPSAJcp3HCFbT6Zc+m1af Id/SPWJXuX43LiYfP+iQkMVqaKtsYZNVdvnH+HPse/ls5Oaug9zxM2UhrNy07e/jtnoc d+hG4Y9PZDss3WmjbOHJ6qEx0bUR+zPoIRmDDhpQFBzdknIITwMrOLf+5PD1jyjnu5V2 avsLPwYbOk4cnN7qUTURE4hzo3pkY+5kpY4j41EWPGYlaeFx0un07Rj+1s0O7x7YFD5M BJv+yJh4lohIT1QMST7GStba/6vG1xm0QnGZ8CdSRf+/iIOUFdlb99T0khy98TYd7MAw 1Q== Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0b-001b2d01.pphosted.com with ESMTP id 361gugg1kd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:45 -0500 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 10CIC7ro008141 for ; Tue, 12 Jan 2021 18:14:45 GMT Received: from b01cxnp22035.gho.pok.ibm.com (b01cxnp22035.gho.pok.ibm.com [9.57.198.25]) by ppma02wdc.us.ibm.com with ESMTP id 35y448yws4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:45 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIEj7a26935584 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:45 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DDC08B2067; Tue, 12 Jan 2021 18:14:44 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 41815B2064; Tue, 12 Jan 2021 18:14:44 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:44 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 2/7] ibmvnic: update reset function prototypes Date: Tue, 12 Jan 2021 10:14:36 -0800 Message-Id: <20210112181441.206545-3-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_12:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 lowpriorityscore=0 phishscore=0 mlxscore=0 suspectscore=0 malwarescore=0 adultscore=0 impostorscore=0 mlxlogscore=999 clxscore=1015 priorityscore=1501 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120103 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.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 Tue Jan 12 18:14:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 361657 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 C7D10C43333 for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 95F7322DFA for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2392511AbhALSPr (ORCPT ); Tue, 12 Jan 2021 13:15:47 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:34884 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2392243AbhALSP3 (ORCPT ); Tue, 12 Jan 2021 13:15:29 -0500 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10CI3DVK123938 for ; Tue, 12 Jan 2021 13:14: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=+8w8E+74hhCNWZkwnXDHQPO5R4dijbXWimQuhAsXiWE=; b=LTeBEiK8lvf2uT29wAFAyr8n9Zu60UK5zLFWbXuxXMTGNcycw8R6+MYDNpZfgOhaZK+c Kkem+1YE/QbGO93J9GtdHwVpahDVhPhWv3qsLAMr14+GTJp+MFfqTdTePEv2oAuYvprw deTF2DMM+f5SFhgWqyBjODYg2n7kolFvFAf460j75ctIPGMvYZuiN4pzKjaICPA4thfG iBYdpGDLGmo14Kxl9Ae0jm5vtDMpZoYJDV1PjMsl6UIeht3W+A1682U0TXzQg8WiOs9F BQ6R/mOcXtx2Azhd/bngPHh2X59trYjR4V8CMhjrIejN+Gj/iw0ay82I7/SBAPqHJnj+ Jg== Received: from ppma03dal.us.ibm.com (b.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.11]) by mx0a-001b2d01.pphosted.com with ESMTP id 361f9j36c8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:47 -0500 Received: from pps.filterd (ppma03dal.us.ibm.com [127.0.0.1]) by ppma03dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 10CI82uU028865 for ; Tue, 12 Jan 2021 18:14:46 GMT Received: from b01cxnp22035.gho.pok.ibm.com (b01cxnp22035.gho.pok.ibm.com [9.57.198.25]) by ppma03dal.us.ibm.com with ESMTP id 35y4495u11-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:46 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIEjq918481466 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:45 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C4631B2066; Tue, 12 Jan 2021 18:14:45 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1F703B2064; Tue, 12 Jan 2021 18:14:45 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:44 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 3/7] ibmvnic: avoid allocating rwi entries Date: Tue, 12 Jan 2021 10:14:37 -0800 Message-Id: <20210112181441.206545-4-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_12:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 impostorscore=0 suspectscore=0 lowpriorityscore=0 priorityscore=1501 spamscore=0 bulkscore=0 phishscore=0 adultscore=0 mlxlogscore=999 mlxscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120106 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.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 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 Tue Jan 12 18:14:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 361658 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 97A8BC43331 for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 70D712311F for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2392205AbhALSPp (ORCPT ); Tue, 12 Jan 2021 13:15:45 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:60886 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2392468AbhALSP3 (ORCPT ); Tue, 12 Jan 2021 13:15:29 -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 10CICn7W058218 for ; Tue, 12 Jan 2021 13:14: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=x9nLvcCcATPiWc8GCmte9Mken5FdlFO8aFeDAIzywCI=; b=TEV7vqYPXN0HySO2W2up0v/mTVbT/4S/HtUiFOHnAlj16gfQFpxt23Hw7mt7nm8zUEg+ A5D4RgM2HqF8DlyAYMfHw2UunOYnIdpxsbvJz4zhVGkFKKBv41bHiq0KctEO645hLZSp e2pfMGGLtlgv2RUqWkSqzkOQUtcf/mG7HO6wwMunWLDRLMSBGUxCL772aPi9592ETx7H PFyKXUSBbzp3b8UVAYrbNpFxrPNcH0FZVzAu+pMac+e0m5Wi3MymgAvihCnDyF9ntJCa aB11lJIUA7IflDOfBq3baq+qWBDFxbgAOiTU3AO1sXenXSJORueiXsLBkt9XgS77BXD8 2A== Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0b-001b2d01.pphosted.com with ESMTP id 361gugg1m2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:47 -0500 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 10CICCXH008162 for ; Tue, 12 Jan 2021 18:14:47 GMT Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by ppma02wdc.us.ibm.com with ESMTP id 35y448yws8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:47 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp23034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIEkp637945618 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:46 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A0B0FB206C; Tue, 12 Jan 2021 18:14:46 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EF51BB2066; Tue, 12 Jan 2021 18:14:45 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:45 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 4/7] ibmvnic: switch order of checks in ibmvnic_reset Date: Tue, 12 Jan 2021 10:14:38 -0800 Message-Id: <20210112181441.206545-5-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_12:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 lowpriorityscore=0 phishscore=0 mlxscore=0 suspectscore=0 malwarescore=0 adultscore=0 impostorscore=0 mlxlogscore=999 clxscore=1015 priorityscore=1501 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120103 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.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 Tue Jan 12 18:14:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 361659 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 D3BB9C433E6 for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 855572311F for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406708AbhALSPd (ORCPT ); Tue, 12 Jan 2021 13:15:33 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:31930 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2392481AbhALSPa (ORCPT ); Tue, 12 Jan 2021 13:15:30 -0500 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10CI3Cp3123822 for ; Tue, 12 Jan 2021 13:14: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=PS+s9bAGg/dOoJqSawJJC6j22s+xmuHNkdH9Lx9tDTo=; b=k5xMQUbanZflKofeWyuF70GineMxDw2Qb7NiiR/D1xAmj3KQdRd2z9/OtACJ06p3O3Bg WEUBox0Psvq3YkBIO39nZQL2hfVi1ZwM+v4JkkLoEmrnrR8oZW83Kr83AmXHJeaZ/zhK KSraXclBhVvLfA46pDDK/MLDVt8lzJy+pj7AoIUKEkpJ7wRpYd83Bg3/BFZkmCd4VmFV vaOWPFk9Pa3FYrHhXjyFgxfbKdSjdpD0K8l0jrb7FF5KLPWmCOtEMHE8SrVCKLgPWsVY Z6SWW5m6GC7FP6OBJWkAzLPKcWkvbNRhD9cvGlfAPQQbKEcA0CIgPGaiFPXMlMJKlJko Lw== Received: from ppma03dal.us.ibm.com (b.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.11]) by mx0a-001b2d01.pphosted.com with ESMTP id 361f9j36db-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:49 -0500 Received: from pps.filterd (ppma03dal.us.ibm.com [127.0.0.1]) by ppma03dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 10CI82Lj028860 for ; Tue, 12 Jan 2021 18:14:48 GMT Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by ppma03dal.us.ibm.com with ESMTP id 35y4495u1b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:48 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIElDA25231788 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:47 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 73912B206B; Tue, 12 Jan 2021 18:14:47 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CC073B2065; Tue, 12 Jan 2021 18:14:46 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:46 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 5/7] ibmvnic: serialize access to work queue Date: Tue, 12 Jan 2021 10:14:39 -0800 Message-Id: <20210112181441.206545-6-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_12:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 impostorscore=0 suspectscore=0 lowpriorityscore=0 priorityscore=1501 spamscore=0 bulkscore=0 phishscore=0 adultscore=0 mlxlogscore=999 mlxscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120106 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The work queue is used to queue reset requests like CHANGE-PARAM or FAILOVER resets for the worker thread. When the adapter is being removed the adapter state is set to VNIC_REMOVING and the work queue is flushed so no new work is added. However the check for adapter being removed is racy in that the adapter can go into REMOVING state just after we check and we might end up adding work just as it is being flushed (or after). Use a new spin lock, ->remove_lock to ensure that after (while) the work queue is (being) flushed, no new work is queued. A follow-on patch will convert the existing ->state_lock from a spin lock to to a mutex to allow us to hold it for longer periods of time. Since work can be scheduled from a tasklet, we cannot block on the mutex, so use a new spin lock. Fixes: 6954a9e4192b ("ibmvnic: Flush existing work items before device removal") Signed-off-by: Sukadev Bhattiprolu --- drivers/net/ethernet/ibm/ibmvnic.c | 15 ++++++++++++++- drivers/net/ethernet/ibm/ibmvnic.h | 2 ++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index ad551418ac63..7645df37e886 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 flags; 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, flags); + /* * 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, flags); 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); @@ -5467,7 +5472,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, flags); adapter->state = VNIC_REMOVING; + spin_unlock_irqrestore(&adapter->remove_lock, flags); + 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 Tue Jan 12 18:14:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 362688 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 7B49FC4332D for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4F81C22DFA for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391608AbhALSPl (ORCPT ); Tue, 12 Jan 2021 13:15:41 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:62796 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406232AbhALSPb (ORCPT ); Tue, 12 Jan 2021 13:15:31 -0500 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 10CI2FCP124378 for ; Tue, 12 Jan 2021 13:14:50 -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=agd2CayxWlu/pjGCV/9UvhlyS7e8snVb1OmUvmyKMwo=; b=g4JeLXybVT+b3SPh7dypr+mvbmiRfpbteZoRZN176KRrV9iE7MgMxGWVU+5Y0SwhvYSs EyvF8wFroi4PDF7CslDRgKHwG/wcx7WwEuhZXA0k0XhJ/ciD6b6C7T/JRgjo+DVvn75r 4Ww3X4zICp1d967pQStzBx1TZRqNauBq7ZjbjeVKvme2qo6stkoh18QQBQqrWNicgfgt 9M0UPzCYLMZri2jgsGF7QGNUWh9HUCGZJMy/V2P8UUizgM3BCFpY2vxtctVagHoTVSIz JXiSQ/ZNi/5k60VdDUakSTlvhLqUF/IWfVhpLKNGUX21XDqj9r68pKhSFnHkprJiubgk XQ== Received: from ppma01wdc.us.ibm.com (fd.55.37a9.ip4.static.sl-reverse.com [169.55.85.253]) by mx0a-001b2d01.pphosted.com with ESMTP id 361f74k9rs-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:50 -0500 Received: from pps.filterd (ppma01wdc.us.ibm.com [127.0.0.1]) by ppma01wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 10CHvS5M015411 for ; Tue, 12 Jan 2021 18:14:49 GMT Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by ppma01wdc.us.ibm.com with ESMTP id 35y448ywha-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:49 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIEmGS7799382 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:48 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4FBB2B2073; Tue, 12 Jan 2021 18:14:48 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A9878B2064; Tue, 12 Jan 2021 18:14:47 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:47 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 6/7] ibmvnic: check adapter->state under state_lock Date: Tue, 12 Jan 2021 10:14:40 -0800 Message-Id: <20210112181441.206545-7-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_12:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 clxscore=1015 mlxlogscore=999 adultscore=0 bulkscore=0 impostorscore=0 mlxscore=0 spamscore=0 lowpriorityscore=0 phishscore=0 malwarescore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120103 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.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 | 118 ++++++++++++++++++++--------- drivers/net/ethernet/ibm/ibmvnic.h | 5 +- 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 7645df37e886..3ca92a207d16 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); @@ -5466,9 +5516,9 @@ static int ibmvnic_remove(struct vio_dev *dev) struct ibmvnic_adapter *adapter = netdev_priv(netdev); 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; } @@ -5481,7 +5531,7 @@ static int ibmvnic_remove(struct vio_dev *dev) adapter->state = VNIC_REMOVING; spin_unlock_irqrestore(&adapter->remove_lock, flags); - spin_unlock_irqrestore(&adapter->state_lock, flags); + mutex_unlock(&adapter->state_lock); flush_work(&adapter->ibmvnic_reset); flush_delayed_work(&adapter->ibmvnic_delayed_reset); @@ -5497,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 Tue Jan 12 18:14:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sukadev Bhattiprolu X-Patchwork-Id: 362689 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 0AC49C43381 for ; Tue, 12 Jan 2021 18:15:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D80522311D for ; Tue, 12 Jan 2021 18:15:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406705AbhALSPf (ORCPT ); Tue, 12 Jan 2021 13:15:35 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:33924 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2406128AbhALSPc (ORCPT ); Tue, 12 Jan 2021 13:15:32 -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 10CIEWJe054335 for ; Tue, 12 Jan 2021 13:14:50 -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=uBPyJl/MqgWhzc0KvhfRDj0fqd+5oJwYu4k9kNsxTcE=; b=Ak42KDMqeaIN3lIEUbtdSHSuuox3C8wLQo8HCeukghUkWS6ZmfpgBz78RWPXWPP9/Raa dbcVEVtNzKf7AgNUIZuXTZo/LwAyhLk/XW9e5NccupnVKtHdOBdEL/OjrX4WwuN1X13L XOY1Mr37l7Kox/CYckcqW4UbbhosGKRb2GTcPJxAO5LbqqwVRbqjdaogIwNRk0Qt4t4h 0B8CSSemTF8005w4TIeU/h3bff8oRxl8bsvjesAR75VfoWlTuTx4ov5dXcjqTcIDBtYV 2R6hxK2Rd/r3GYSVpIKGbBpWzVCyAVPyLV0kjvq/LkozuBbz2rsFS70IUcujzEz3OHM7 /g== 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 361gvbr04g-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 13:14:50 -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 10CI7Iqn030850 for ; Tue, 12 Jan 2021 18:14:49 GMT Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by ppma04wdc.us.ibm.com with ESMTP id 35y448yw9p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 12 Jan 2021 18:14:49 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 10CIEnh324248646 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 12 Jan 2021 18:14:49 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 365F7B2071; Tue, 12 Jan 2021 18:14:49 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 87750B2074; Tue, 12 Jan 2021 18:14:48 +0000 (GMT) Received: from suka-w540.ibmuc.com (unknown [9.85.179.93]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Tue, 12 Jan 2021 18:14:48 +0000 (GMT) From: Sukadev Bhattiprolu To: netdev@vger.kernel.org Cc: Dany Madden , Lijun Pan , Rick Lindsley , sukadev@linux.ibm.com Subject: [PATCH net-next v2 7/7] ibmvnic: add comments about state_lock Date: Tue, 12 Jan 2021 10:14:41 -0800 Message-Id: <20210112181441.206545-8-sukadev@linux.ibm.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210112181441.206545-1-sukadev@linux.ibm.com> References: <20210112181441.206545-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-12_15:2021-01-12,2021-01-12 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 adultscore=0 malwarescore=0 mlxlogscore=999 impostorscore=0 suspectscore=0 lowpriorityscore=0 priorityscore=1501 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101120106 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Add some comments, notes and TODOs about ->state_lock and RTNL. Signed-off-by: Sukadev Bhattiprolu --- 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 3ca92a207d16..d56e73b64da4 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, flags); + /* 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; };