From: Bart Van Assche <bvanassche@acm.org>
To: huobean@gmail.com, alim.akhtar@samsung.com, avri.altman@wdc.com,
asutoshd@codeaurora.org, jejb@linux.ibm.com,
martin.petersen@oracle.com, stanley.chu@mediatek.com,
beanhuo@micron.com, tomas.winkler@intel.com, cang@codeaurora.org
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/5] scsi: ufs: UFS Host Performance Booster(HPB) driver
Date: Sat, 25 Apr 2020 11:07:40 -0700 [thread overview]
Message-ID: <183528b9-f04b-40f5-269b-5897da113b97@acm.org> (raw)
In-Reply-To: <20200416203126.1210-6-beanhuo@micron.com>
On 2020-04-16 13:31, huobean@gmail.com wrote:
> +static int ufshpb_execute_cmd(struct ufshpb_lu *hpb, unsigned char *cmd)
> +{
> + struct scsi_sense_hdr sshdr;
> + struct scsi_device *sdp;
> + struct ufs_hba *hba;
> + int retries;
> + int ret = 0;
> +
> + hba = hpb->hba;
> +
> + sdp = hba->sdev_ufs_lu[hpb->lun];
> + if (!sdp) {
> + hpb_warn("%s UFSHPB cannot find scsi_device\n", __func__);
> + return -ENODEV;
> + }
> +
> + ret = scsi_device_get(sdp);
> + if (!ret && !scsi_device_online(sdp)) {
> + ret = -ENODEV;
> + scsi_device_put(sdp);
> + return ret;
> + }
> +
> + for (retries = 3; retries > 0; --retries) {
> + ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
> + msecs_to_jiffies(30000), 3, 0, RQF_PM, NULL);
> + if (ret == 0)
> + break;
> + }
> +
> + if (ret) {
> + if (driver_byte(ret) == DRIVER_SENSE &&
> + scsi_sense_valid(&sshdr)) {
> + switch (sshdr.sense_key) {
> + case ILLEGAL_REQUEST:
> + hpb_err("ILLEGAL REQUEST asc=0x%x ascq=0x%x\n",
> + sshdr.asc, sshdr.ascq);
> + break;
> + default:
> + hpb_err("Unknown return code = %x\n", ret);
> + break;
> + }
> + }
> + }
> + scsi_device_put(sdp);
> +
> + return ret;
> +}
If scsi_execute() would be changed into asynchronous SCSI command
submission, can ufshpb_execute_cmd() be called from inside the UFS
.queue_rq() callback instead of from workqueue context?
The scsi_device_get() call looks misplaced. I think that call should
happen before schedule_work() is called.
> +static int ufshpb_l2p_load_req(struct ufshpb_lu *hpb, struct request_queue *q,
> + struct ufshpb_map_req *map_req)
> +{
> + struct scsi_request *scsi_rq;
> + unsigned char cmd[16] = { };
> + struct request *req;
> + struct bio *bio;
> + int alloc_len;
> + int ret;
> +
> + bio = &map_req->bio;
> +
> + ret = ufshpb_add_bio_page(hpb, q, bio, map_req->bvec, map_req->mctx);
> + if (ret) {
> + hpb_err("ufshpb_add_bio_page() failed with ret %d\n", ret);
> + return ret;
> + }
> +
> + alloc_len = hpb->hba->hpb_geo.subregion_entry_sz;
> + /*
> + * HPB Sub-Regions are equally sized except for the last one which is
> + * smaller if the last hpb Region is not an integer multiple of
> + * bHPBSubRegionSize.
> + */
> + if (map_req->region == (hpb->lu_region_cnt - 1) &&
> + map_req->subregion == (hpb->subregions_in_last_region - 1))
> + alloc_len = hpb->last_subregion_entry_size;
> +
> + ufshpb_prepare_read_buf_cmd(cmd, map_req->region, map_req->subregion,
> + alloc_len);
> + if (!map_req->req) {
> + map_req->req = blk_get_request(q, REQ_OP_SCSI_IN, 0);
> + if (IS_ERR(map_req->req))
> + return PTR_ERR(map_req->req);
> + }
> + req = map_req->req;
> + scsi_rq = scsi_req(req);
> +
> + blk_rq_append_bio(req, &bio);
> +
> + scsi_req_init(scsi_rq);
> +
> + scsi_rq->cmd_len = COMMAND_SIZE(cmd[0]);
> + memcpy(scsi_rq->cmd, cmd, scsi_rq->cmd_len);
> + req->timeout = msecs_to_jiffies(30000);
> + req->end_io_data = (void *)map_req;
> +
> + hpb_dbg(SCHEDULE_INFO, hpb, "ISSUE READ_BUFFER : (%d-%d) retry = %d\n",
> + map_req->region, map_req->subregion, map_req->retry_cnt);
> + hpb_trace(hpb, "%s: I RB %d - %d", DRIVER_NAME, map_req->region,
> + map_req->subregion);
> +
> + blk_execute_rq_nowait(q, NULL, req, 1, ufshpb_l2p_load_req_done);
> + map_req->req_issue_t = ktime_to_ns(ktime_get());
> +
> + atomic64_inc(&hpb->status.map_req_cnt);
> +
> + return 0;
> +}
Same question here: if 'req' would be submitted asynchronously, can
ufshpb_l2p_load_req() be called directly from inside the UFS .queue_rq()
callback instead of from workqueue context?
Thanks,
Bart.
next prev parent reply other threads:[~2020-04-25 18:07 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-16 20:31 [PATCH v2 0/5] UFS Host Performance Booster (HPB v1.0) driver huobean
2020-04-16 20:31 ` [PATCH v2 1/5] scsi; ufs: add device descriptor for Host Performance Booster huobean
2020-04-22 23:11 ` Bart Van Assche
2020-04-23 11:01 ` [EXT] " Bean Huo (beanhuo)
2020-04-23 17:57 ` Bart Van Assche
2020-04-16 20:31 ` [PATCH v2 2/5] scsi: ufs: make ufshcd_read_unit_desc_param() non-static func huobean
2020-04-16 20:31 ` [PATCH v2 3/5] scsi: ufs: add ufs_features parameter in structure ufs_dev_info huobean
2020-04-22 23:13 ` Bart Van Assche
2020-04-16 20:31 ` [PATCH v2 4/5] scsi: ufs: add unit and geometry parameters for HPB huobean
2020-04-16 20:31 ` [PATCH v2 5/5] scsi: ufs: UFS Host Performance Booster(HPB) driver huobean
2020-04-16 21:35 ` Randy Dunlap
2020-04-23 0:00 ` Bart Van Assche
2020-04-24 9:51 ` [EXT] " Bean Huo (beanhuo)
2020-04-24 18:17 ` Avri Altman
2020-04-24 20:02 ` [EXT] " Bean Huo (beanhuo)
2020-04-25 8:59 ` Avri Altman
2020-04-25 17:51 ` Bart Van Assche
2020-04-27 6:13 ` Avri Altman
2020-04-28 3:36 ` Bart Van Assche
2020-04-28 8:14 ` Avri Altman
2020-04-28 11:59 ` Bean Huo
2020-04-30 7:23 ` Avri Altman
2020-04-30 12:45 ` Bean Huo
2020-05-02 16:19 ` Avri Altman
2020-04-28 9:12 ` Bean Huo
2020-04-25 18:07 ` Bart Van Assche [this message]
2020-04-26 22:03 ` Bean Huo
2020-04-22 6:43 ` [PATCH v2 0/5] UFS Host Performance Booster (HPB v1.0) driver Christoph Hellwig
2020-04-22 22:09 ` [EXT] " Bean Huo (beanhuo)
2020-04-23 8:26 ` Avri Altman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=183528b9-f04b-40f5-269b-5897da113b97@acm.org \
--to=bvanassche@acm.org \
--cc=alim.akhtar@samsung.com \
--cc=asutoshd@codeaurora.org \
--cc=avri.altman@wdc.com \
--cc=beanhuo@micron.com \
--cc=cang@codeaurora.org \
--cc=huobean@gmail.com \
--cc=jejb@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=stanley.chu@mediatek.com \
--cc=tomas.winkler@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).