From patchwork Tue Aug 9 23:39:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Matthew R. Ochs" X-Patchwork-Id: 9272419 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E111860231 for ; Tue, 9 Aug 2016 23:40:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B70E027D8D for ; Tue, 9 Aug 2016 23:40:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A9CBC27F85; Tue, 9 Aug 2016 23:40:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E6FBF27D8D for ; Tue, 9 Aug 2016 23:40:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932643AbcHIXkH (ORCPT ); Tue, 9 Aug 2016 19:40:07 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:5075 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932614AbcHIXkG (ORCPT ); Tue, 9 Aug 2016 19:40:06 -0400 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u79Nd0x9063821 for ; Tue, 9 Aug 2016 19:40:06 -0400 Received: from e31.co.us.ibm.com (e31.co.us.ibm.com [32.97.110.149]) by mx0b-001b2d01.pphosted.com with ESMTP id 24qm9r236r-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 09 Aug 2016 19:40:05 -0400 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 9 Aug 2016 17:40:05 -0600 Received: from d03dlp03.boulder.ibm.com (9.17.202.179) by e31.co.us.ibm.com (192.168.1.131) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 9 Aug 2016 17:39:52 -0600 X-IBM-Helo: d03dlp03.boulder.ibm.com X-IBM-MailFrom: mrochs@linux.vnet.ibm.com Received: from b03cxnp08025.gho.boulder.ibm.com (b03cxnp08025.gho.boulder.ibm.com [9.17.130.17]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 7597819D8026; Tue, 9 Aug 2016 17:39:25 -0600 (MDT) 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 u79Ndopc14615022; Tue, 9 Aug 2016 16:39:50 -0700 Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7002578037; Tue, 9 Aug 2016 17:39:50 -0600 (MDT) Received: from p8tul1-build.aus.stglabs.ibm.com (unknown [9.3.141.206]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP id BC6907803F; Tue, 9 Aug 2016 17:39:49 -0600 (MDT) From: "Matthew R. Ochs" To: linux-scsi@vger.kernel.org, James Bottomley , "Martin K. Petersen" , Uma Krishnan , "Manoj N. Kumar" , Al Viro Cc: Brian King , linuxppc-dev@lists.ozlabs.org, Ian Munsie , Andrew Donnellan , Frederic Barrat , Christophe Lombard , "Matthew R. Ochs" Subject: [PATCH 3/6] cxlflash: Add kref to context Date: Tue, 9 Aug 2016 18:39:42 -0500 X-Mailer: git-send-email 2.1.0 In-Reply-To: <1470785888-9112-1-git-send-email-mrochs@linux.vnet.ibm.com> References: <1470785888-9112-1-git-send-email-mrochs@linux.vnet.ibm.com> X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16080923-8235-0000-0000-000008F51652 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005571; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000178; SDB=6.00742430; UDB=6.00349470; IPR=6.00514987; BA=6.00004652; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012282; XFM=3.00000011; UTC=2016-08-09 23:40:04 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16080923-8236-0000-0000-000033D39CC6 Message-Id: <1470785982-9233-1-git-send-email-mrochs@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-08-09_10:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608090248 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently, context user references are tracked via the list of LUNs that have attached to the context. While convenient, this is not intuitive without a deep study of the code and is inconsistent with the existing reference tracking patterns within the kernel. This design choice can lead to future bug injection. To improve code comprehension and better protect against future bugs, add explicit reference counting to contexts and migrate the context removal code to the kref release handler. Inspired-by: Al Viro Signed-off-by: Matthew R. Ochs Acked-by: Manoj N. Kumar --- drivers/scsi/cxlflash/superpipe.c | 87 +++++++++++++++++++++++---------------- drivers/scsi/cxlflash/superpipe.h | 1 + 2 files changed, 53 insertions(+), 35 deletions(-) diff --git a/drivers/scsi/cxlflash/superpipe.c b/drivers/scsi/cxlflash/superpipe.c index 640c3a2..be7522a 100644 --- a/drivers/scsi/cxlflash/superpipe.c +++ b/drivers/scsi/cxlflash/superpipe.c @@ -808,11 +808,56 @@ static void init_context(struct ctx_info *ctxi, struct cxlflash_cfg *cfg, ctxi->file = file; ctxi->initialized = true; mutex_init(&ctxi->mutex); + kref_init(&ctxi->kref); INIT_LIST_HEAD(&ctxi->luns); INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */ } /** + * remove_context() - context kref release handler + * @kref: Kernel reference associated with context to be removed. + * + * When a context no longer has any references it can safely be removed + * from global access and destroyed. Note that it is assumed the thread + * relinquishing access to the context holds its mutex. + */ +static void remove_context(struct kref *kref) +{ + struct ctx_info *ctxi = container_of(kref, struct ctx_info, kref); + struct cxlflash_cfg *cfg = ctxi->cfg; + int lfd; + u64 ctxid = DECODE_CTXID(ctxi->ctxid); + + /* Remove context from table/error list */ + WARN_ON(!mutex_is_locked(&ctxi->mutex)); + ctxi->unavail = true; + mutex_unlock(&ctxi->mutex); + mutex_lock(&cfg->ctx_tbl_list_mutex); + mutex_lock(&ctxi->mutex); + + if (!list_empty(&ctxi->list)) + list_del(&ctxi->list); + cfg->ctx_tbl[ctxid] = NULL; + mutex_unlock(&cfg->ctx_tbl_list_mutex); + mutex_unlock(&ctxi->mutex); + + /* Context now completely uncoupled/unreachable */ + lfd = ctxi->lfd; + destroy_context(cfg, ctxi); + + /* + * As a last step, clean up external resources when not + * already on an external cleanup thread, i.e.: close(adap_fd). + * + * NOTE: this will free up the context from the CXL services, + * allowing it to dole out the same context_id on a future + * (or even currently in-flight) disk_attach operation. + */ + if (lfd != -1) + sys_close(lfd); +} + +/** * _cxlflash_disk_detach() - detaches a LUN from a context * @sdev: SCSI device associated with LUN. * @ctxi: Context owning resources. @@ -837,7 +882,6 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev, int i; int rc = 0; - int lfd; u64 ctxid = DECODE_CTXID(detach->context_id), rctxid = detach->context_id; @@ -879,40 +923,12 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev, break; } - /* Tear down context following last LUN cleanup */ - if (list_empty(&ctxi->luns)) { - ctxi->unavail = true; - mutex_unlock(&ctxi->mutex); - mutex_lock(&cfg->ctx_tbl_list_mutex); - mutex_lock(&ctxi->mutex); - - /* Might not have been in error list so conditionally remove */ - if (!list_empty(&ctxi->list)) - list_del(&ctxi->list); - cfg->ctx_tbl[ctxid] = NULL; - mutex_unlock(&cfg->ctx_tbl_list_mutex); - mutex_unlock(&ctxi->mutex); - - lfd = ctxi->lfd; - destroy_context(cfg, ctxi); - ctxi = NULL; - put_ctx = false; - - /* - * As a last step, clean up external resources when not - * already on an external cleanup thread, i.e.: close(adap_fd). - * - * NOTE: this will free up the context from the CXL services, - * allowing it to dole out the same context_id on a future - * (or even currently in-flight) disk_attach operation. - */ - if (lfd != -1) - sys_close(lfd); - } - - /* Release the sdev reference that bound this LUN to the context */ + /* + * Release the context reference and the sdev reference that + * bound this LUN to the context. + */ + put_ctx = !kref_put(&ctxi->kref, remove_context); scsi_device_put(sdev); - out: if (put_ctx) put_context(ctxi); @@ -1369,10 +1385,11 @@ static int cxlflash_disk_attach(struct scsi_device *sdev, lun_access->lli = lli; lun_access->sdev = sdev; - /* Non-NULL context indicates reuse */ + /* Non-NULL context indicates reuse (another context reference) */ if (ctxi) { dev_dbg(dev, "%s: Reusing context for LUN! (%016llX)\n", __func__, rctxid); + kref_get(&ctxi->kref); list_add(&lun_access->list, &ctxi->luns); fd = ctxi->lfd; goto out_attach; diff --git a/drivers/scsi/cxlflash/superpipe.h b/drivers/scsi/cxlflash/superpipe.h index 61404f2..5bda8b5 100644 --- a/drivers/scsi/cxlflash/superpipe.h +++ b/drivers/scsi/cxlflash/superpipe.h @@ -106,6 +106,7 @@ struct ctx_info { bool unavail; bool err_recovery_active; struct mutex mutex; /* Context protection */ + struct kref kref; struct cxl_context *ctx; struct cxlflash_cfg *cfg; struct list_head luns; /* LUNs attached to this context */