| Message ID | 20200308145648.28675-2-beanhuo@micron.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: ufs: fix lrbp pointer incorrect initialization issue | expand |
On 2020-03-08 07:56, Bean Huo wrote: > The parameter cmd of ufshcd_init_cmd_priv() wasn't given a correct tag > value while the SCSI layer calls back ufshcd_init_cmd_priv(), this results > in all pointers of lrbp in UFS driver point to first the lrbp. > > As this is just observed, the patch is for reference so others > who want to use the latest UFS driver can avoid this issue. Any recommend > is welcomed. How about also removing ufshcd_init_cmd_priv(), like in the untested patch below? Thanks, Bart. Subject: [PATCH] ufs: Fix LRB initialization There are two issues with the ufshcd_init_lrb() call in ufshcd_init_cmd_priv(): - cmd->tag is set from inside scsi_mq_prep_fn() and hence is not yet set when ufshcd_init_cmd_priv() is set. - Inside ufshcd_init_cmd_priv() the tag can only be derived from the SCSI command pointer if no scheduler has been associated with the UFS block device. If no scheduler is associated with a block device, the relationship between hctx->tags->static_rqs[] and rq->tag is static. If a scheduler has been configured, a single tag can be associated with a different struct request if a request is reallocated. Fixes: 34656dda81ac ("ufs: Let the SCSI core allocate per-command UFS data") --- drivers/scsi/ufs/ufshcd.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e987fa3a77c7..1589ba70672f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2473,6 +2473,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) WARN_ON_ONCE(!ufshcd_is_scsi(cmd->request)); + ufshcd_init_lrb(hba, lrbp, tag); + if (!down_read_trylock(&hba->clk_scaling_lock)) return SCSI_MLQUEUE_HOST_BUSY; @@ -2707,6 +2709,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, init_completion(&wait); lrbp = ufshcd_req_to_lrb(req); + ufshcd_init_lrb(hba, lrbp, tag); err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag); if (unlikely(err)) goto out_put_tag; @@ -3504,14 +3507,6 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) } } -static int ufshcd_init_cmd_priv(struct Scsi_Host *shost, struct scsi_cmnd *cmd) -{ - struct ufs_hba *hba = shost_priv(shost); - - ufshcd_init_lrb(hba, scsi_cmd_priv(cmd), cmd->tag); - return 0; -} - /** * ufshcd_dme_link_startup - Notify Unipro to perform link startup * @hba: per adapter instance @@ -5900,6 +5895,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba, init_completion(&wait); lrbp = ufshcd_req_to_lrb(req); + ufshcd_init_lrb(hba, lrbp, tag); lrbp->sense_bufflen = 0; lrbp->sense_buffer = NULL; lrbp->task_tag = tag; @@ -7180,7 +7176,6 @@ static struct scsi_host_template ufshcd_driver_template = { .module = THIS_MODULE, .name = UFSHCD, .proc_name = UFSHCD, - .init_cmd_priv = ufshcd_init_cmd_priv, .queuecommand = ufshcd_queuecommand, .slave_alloc = ufshcd_slave_alloc, .slave_configure = ufshcd_slave_configure,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e987fa3a77c7..396512a9234f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2471,6 +2471,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) BUG(); } + ufshcd_init_lrb(hba, lrbp, tag); + WARN_ON_ONCE(!ufshcd_is_scsi(cmd->request)); if (!down_read_trylock(&hba->clk_scaling_lock)) @@ -2707,6 +2709,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, init_completion(&wait); lrbp = ufshcd_req_to_lrb(req); + ufshcd_init_lrb(hba, lrbp, tag); err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag); if (unlikely(err)) goto out_put_tag; @@ -5900,6 +5903,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba, init_completion(&wait); lrbp = ufshcd_req_to_lrb(req); + ufshcd_init_lrb(hba, lrbp, tag); lrbp->sense_bufflen = 0; lrbp->sense_buffer = NULL; lrbp->task_tag = tag;
The parameter cmd of ufshcd_init_cmd_priv() wasn't given a correct tag value while the SCSI layer calls back ufshcd_init_cmd_priv(), this results in all pointers of lrbp in UFS driver point to first the lrbp. As this is just observed, the patch is for reference so others who want to use the latest UFS driver can avoid this issue. Any recommend is welcomed. Fixes: 34656dda81ac ("scsi: ufs: Let the SCSI core allocate per-command UFS data") --- drivers/scsi/ufs/ufshcd.c | 4 ++++ 1 file changed, 4 insertions(+)