Received: by 2002:a05:6358:1087:b0:cb:c9d3:cd90 with SMTP id j7csp3259101rwi; Fri, 28 Oct 2022 18:49:11 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4HSQvtSGBaWffOCKOGH6rQeHHAysxlVbt8aJ6AZ+yLiOVIPr+5czcJiq/Ku1oCyACo+ZXb X-Received: by 2002:a17:906:6a1a:b0:7a1:a9e5:7869 with SMTP id qw26-20020a1709066a1a00b007a1a9e57869mr2012826ejc.708.1667008151264; Fri, 28 Oct 2022 18:49:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667008151; cv=none; d=google.com; s=arc-20160816; b=bf8LYIzXZuucsXIMVm6ZnSL5kLBeEZI7U3JQkcH8As/Go/R1j2tgZMRlGM0xp4aOhW 5h+L9Yq6Z8AkUh45Sl0GXj3ky2iAA+qAn3RjhPHzk/7izPfOj5Nzc4d8mWNOW1TibIh9 Hv04tnvzjJWGh3nFLECMO5tYXXl00n0YKjhJ8q4DT56/Y0nStlvcyCH3SuOmRMNeL18s Cyo63GdQy+CxvYMdMTfXcqbC5NTvezQRtNCoc8AyBY1hOzn9fJ9TymgPHCbrIUZtIMxM 1PLFbfWo/fLa/vrioheEqhSolH16gwrcOa170y9ivR9MaojgOjuoaKe/XBBqQr/KBe7V 4ecA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:cc:references:to :subject; bh=UH1ID1DI8oDSIbbd1taBBbbDO6ncQeSg5PXMPKcpcPo=; b=HYZnmjI8vhmfwqmNDyyYTbyo8hRQVOB1r41Pojtppt6z0lYullvF/s7CObFlgb1DTO 8uI9qIi2+J7f7ZxKXzOrsEk7MI6a9QlRLFQmJdSF/MkIeFcAgo1IHKfBDgf8QVrDhPH8 1zyuJgD1+NOq1QvKBI7xLowHqzB7k51L5ub2Co4n1YQAn58pjMKByxGbYHrtAissU1Ps 2RczB62hE6p1B1mxixMiEns7Wyjq7uMKKxg0bJRsq4qKAEAA1bhXhwuBuBoPwNa6FWU3 waift+OP6pf6l3Vg0tDoyuDtFWc+yyKOnN1OGq2MWGDp7e+zmUQQU+P27FVCkIaKBy1f GjXA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=hisilicon.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id qw9-20020a1709066a0900b007adb388df30si126822ejc.804.2022.10.28.18.48.45; Fri, 28 Oct 2022 18:49:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=hisilicon.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229781AbiJ2BPy (ORCPT + 99 others); Fri, 28 Oct 2022 21:15:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42602 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229752AbiJ2BPw (ORCPT ); Fri, 28 Oct 2022 21:15:52 -0400 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 05CEB19ABC9; Fri, 28 Oct 2022 18:15:48 -0700 (PDT) Received: from kwepemi500016.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4MzhGV1GqdzpW75; Sat, 29 Oct 2022 09:12:18 +0800 (CST) Received: from [10.40.193.166] (10.40.193.166) by kwepemi500016.china.huawei.com (7.221.188.220) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Sat, 29 Oct 2022 09:15:46 +0800 Subject: Re: [PATCH RFC v3 21/22] scsi: libsas: Queue internal abort commands as requests To: John Garry , , , , , , , , , , References: <1666693096-180008-1-git-send-email-john.garry@huawei.com> <1666693096-180008-22-git-send-email-john.garry@huawei.com> CC: , , , , From: "chenxiang (M)" Message-ID: <0cbc86ea-09c9-2654-a795-9230e4b00a1e@hisilicon.com> Date: Sat, 29 Oct 2022 09:15:45 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1666693096-180008-22-git-send-email-john.garry@huawei.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.40.193.166] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To kwepemi500016.china.huawei.com (7.221.188.220) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,NICE_REPLY_A, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi John, For internal abort commands, it allocates and deliver requests through sdev->request_queue. Is it possible that we still need to send internal abort commands even if sdev is freed? I notices that in sas_destruct_devices, it calls sas_rphy_delete() to remove target, and then call i->dft->lldd_dev_gone() which will call internal abort commands. 在 2022/10/25 18:18, John Garry 写道: > Like what we did for SMP commands, send internal abort commands through > the block layer. > > At this point we can delete special handling in sas_task_abort() for SAS > "internal" commands, as every slow task now has a request associated. > > Function sas_task_internal_timedout() is no longer referenced, so delete > it. > > Signed-off-by: John Garry > --- > drivers/scsi/hisi_sas/hisi_sas_main.c | 36 +++++++++---------- > drivers/scsi/libsas/sas_expander.c | 2 +- > drivers/scsi/libsas/sas_init.c | 12 +++++-- > drivers/scsi/libsas/sas_internal.h | 3 +- > drivers/scsi/libsas/sas_scsi_host.c | 52 ++++++--------------------- > include/scsi/libsas.h | 1 - > 6 files changed, 38 insertions(+), 68 deletions(-) > > diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c > index fe2752d24fe8..65475775c844 100644 > --- a/drivers/scsi/hisi_sas/hisi_sas_main.c > +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c > @@ -465,7 +465,7 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags) > struct hisi_sas_port *port; > struct hisi_hba *hisi_hba; > struct hisi_sas_slot *slot; > - struct request *rq = NULL; > + struct request *rq; > struct device *dev; > int rc; > > @@ -485,6 +485,21 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags) > > hisi_hba = dev_to_hisi_hba(device); > dev = hisi_hba->dev; > + rq = sas_task_find_rq(task); > + if (rq) { > + unsigned int dq_index; > + u32 blk_tag; > + > + blk_tag = blk_mq_unique_tag(rq); > + dq_index = blk_mq_unique_tag_to_hwq(blk_tag); > + dq = &hisi_hba->dq[dq_index]; > + } else { > + struct Scsi_Host *shost = hisi_hba->shost; > + struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT]; > + int queue = qmap->mq_map[raw_smp_processor_id()]; > + > + dq = &hisi_hba->dq[queue]; > + } > > switch (task->task_proto) { > case SAS_PROTOCOL_SSP: > @@ -519,22 +534,6 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags) > > return -ECOMM; > } > - > - rq = sas_task_find_rq(task); > - if (rq) { > - unsigned int dq_index; > - u32 blk_tag; > - > - blk_tag = blk_mq_unique_tag(rq); > - dq_index = blk_mq_unique_tag_to_hwq(blk_tag); > - dq = &hisi_hba->dq[dq_index]; > - } else { > - struct Scsi_Host *shost = hisi_hba->shost; > - struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT]; > - int queue = qmap->mq_map[raw_smp_processor_id()]; > - > - dq = &hisi_hba->dq[queue]; > - } > break; > case SAS_PROTOCOL_INTERNAL_ABORT: > if (!hisi_hba->hw->prep_abort) > @@ -543,13 +542,10 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags) > if (test_bit(HISI_SAS_HW_FAULT_BIT, &hisi_hba->flags)) > return -EIO; > > - hisi_hba = dev_to_hisi_hba(device); > - > if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags))) > return -EINVAL; > > port = to_hisi_sas_port(sas_port); > - dq = &hisi_hba->dq[task->abort_task.qid]; > break; > default: > dev_err(hisi_hba->dev, "task prep: unknown/unsupported proto (0x%x)\n", > diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c > index cc41127ea5cc..e852f1565fe7 100644 > --- a/drivers/scsi/libsas/sas_expander.c > +++ b/drivers/scsi/libsas/sas_expander.c > @@ -50,7 +50,7 @@ static int smp_execute_task_sg(struct domain_device *dev, > break; > } > > - task = sas_alloc_slow_task_rq(dev, GFP_KERNEL); > + task = sas_alloc_slow_task_rq(dev, GFP_KERNEL, -1U); > if (!task) { > res = -ENOMEM; > break; > diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c > index 5f9e71a54799..c3f602bd2c4c 100644 > --- a/drivers/scsi/libsas/sas_init.c > +++ b/drivers/scsi/libsas/sas_init.c > @@ -56,7 +56,7 @@ struct sas_task *sas_alloc_slow_task(gfp_t flags) > return task; > } > > -struct sas_task *sas_alloc_slow_task_rq(struct domain_device *device, gfp_t flags) > +struct sas_task *sas_alloc_slow_task_rq(struct domain_device *device, gfp_t flags, unsigned int qid) > { > struct sas_ha_struct *sas_ha = device->port->ha; > struct Scsi_Host *shost = sas_ha->core.shost; > @@ -86,8 +86,14 @@ struct sas_task *sas_alloc_slow_task_rq(struct domain_device *device, gfp_t flag > task->dev = device; > task->task_done = sas_task_complete_internal; > > - rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, > - BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT); > + if (qid == -1U) { > + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, > + BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT); > + } else { > + rq = scsi_alloc_request_hwq(sdev->request_queue, REQ_OP_DRV_IN, > + BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT, > + qid); > + } > if (IS_ERR(rq)) { > sas_free_task(task); > return NULL; > diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h > index 9b58948c57c2..af4a4bf88830 100644 > --- a/drivers/scsi/libsas/sas_internal.h > +++ b/drivers/scsi/libsas/sas_internal.h > @@ -54,7 +54,8 @@ void sas_free_event(struct asd_sas_event *event); > > struct sas_task *sas_alloc_task(gfp_t flags); > struct sas_task *sas_alloc_slow_task(gfp_t flags); > -struct sas_task *sas_alloc_slow_task_rq(struct domain_device *device, gfp_t flags); > +struct sas_task *sas_alloc_slow_task_rq(struct domain_device *device, gfp_t flags, > + unsigned int qid); > void sas_free_task(struct sas_task *task); > > int sas_register_ports(struct sas_ha_struct *sas_ha); > diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c > index e6ee8dd56a45..a93e019a7dbf 100644 > --- a/drivers/scsi/libsas/sas_scsi_host.c > +++ b/drivers/scsi/libsas/sas_scsi_host.c > @@ -920,23 +920,6 @@ void sas_task_complete_internal(struct sas_task *task) > scsi_done(scmd); > } > > -void sas_task_internal_timedout(struct timer_list *t) > -{ > - struct sas_task_slow *slow = from_timer(slow, t, timer); > - struct sas_task *task = slow->task; > - bool is_completed = true; > - unsigned long flags; > - > - spin_lock_irqsave(&task->task_state_lock, flags); > - if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) { > - task->task_state_flags |= SAS_TASK_STATE_ABORTED; > - is_completed = false; > - } > - spin_unlock_irqrestore(&task->task_state_lock, flags); > - > - if (!is_completed) > - complete(&task->slow_task->completion); > -} > enum blk_eh_timer_return sas_internal_timeout(struct scsi_cmnd *scmd) > { > struct sas_task *task = TO_SAS_TASK(scmd); > @@ -978,28 +961,23 @@ static int sas_execute_internal_abort(struct domain_device *device, > int res, retry; > > for (retry = 0; retry < TASK_RETRY; retry++) { > - task = sas_alloc_slow_task(GFP_KERNEL); > + struct request *rq; > + > + task = sas_alloc_slow_task_rq(device, GFP_KERNEL, qid); > if (!task) > return -ENOMEM; > > task->dev = device; > task->task_proto = SAS_PROTOCOL_INTERNAL_ABORT; > - task->task_done = sas_task_internal_done; > - task->slow_task->timer.function = sas_task_internal_timedout; > - task->slow_task->timer.expires = jiffies + TASK_TIMEOUT; > - add_timer(&task->slow_task->timer); > + task->task_done = sas_task_complete_internal; > > task->abort_task.tag = tag; > task->abort_task.type = type; > - task->abort_task.qid = qid; > > - res = i->dft->lldd_execute_task(task, GFP_KERNEL); > - if (res) { > - del_timer_sync(&task->slow_task->timer); > - pr_err("Executing internal abort failed %016llx (%d)\n", > - SAS_ADDR(device->sas_addr), res); > - break; > - } > + rq = scsi_cmd_to_rq(task->uldd_task); > + rq->timeout = TASK_TIMEOUT; > + > + blk_execute_rq_nowait(rq, true); > > wait_for_completion(&task->slow_task->completion); > res = TMF_RESP_FUNC_FAILED; > @@ -1069,7 +1047,7 @@ int sas_execute_tmf(struct domain_device *device, void *parameter, > for (retry = 0; retry < TASK_RETRY; retry++) { > struct request *rq; > > - task = sas_alloc_slow_task_rq(device, GFP_KERNEL); > + task = sas_alloc_slow_task_rq(device, GFP_KERNEL, -1U); > if (!task) > return -ENOMEM; > > @@ -1251,17 +1229,7 @@ void sas_task_abort(struct sas_task *task) > { > struct scsi_cmnd *sc = task->uldd_task; > > - /* Escape for libsas internal commands */ > - if (!sc) { > - struct sas_task_slow *slow = task->slow_task; > - > - if (!slow) > - return; > - if (!del_timer(&slow->timer)) > - return; > - slow->timer.function(&slow->timer); > - return; > - } > + WARN_ON_ONCE(!sc); > > if (dev_is_sata(task->dev) && !task->slow_task) > sas_ata_task_abort(task); > diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h > index f02156ccd376..60543d8b01d4 100644 > --- a/include/scsi/libsas.h > +++ b/include/scsi/libsas.h > @@ -565,7 +565,6 @@ enum sas_internal_abort { > > struct sas_internal_abort_task { > enum sas_internal_abort type; > - unsigned int qid; > u16 tag; > }; >