* [bug report] scsi: ufs: Optimize host lock on transfer requests send/compl paths
@ 2021-06-09 11:01 Dan Carpenter
2021-06-10 0:54 ` Can Guo
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2021-06-09 11:01 UTC (permalink / raw)
To: cang; +Cc: linux-scsi
Hello Can Guo,
The patch a45f937110fa: "scsi: ufs: Optimize host lock on transfer
requests send/compl paths" from May 24, 2021, leads to the following
static checker warning:
drivers/scsi/ufs/ufshcd.c:2998 ufshcd_exec_dev_cmd()
error: potentially dereferencing uninitialized 'lrbp'.
drivers/scsi/ufs/ufshcd.c
2948 static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
2949 enum dev_cmd_type cmd_type, int timeout)
2950 {
2951 struct request_queue *q = hba->cmd_queue;
2952 struct request *req;
2953 struct ufshcd_lrb *lrbp;
^^^^
2954 int err;
2955 int tag;
2956 struct completion wait;
2957
2958 down_read(&hba->clk_scaling_lock);
2959
2960 /*
2961 * Get free slot, sleep if slots are unavailable.
2962 * Even though we use wait_event() which sleeps indefinitely,
2963 * the maximum wait time is bounded by SCSI request timeout.
2964 */
2965 req = blk_get_request(q, REQ_OP_DRV_OUT, 0);
2966 if (IS_ERR(req)) {
2967 err = PTR_ERR(req);
2968 goto out_unlock;
2969 }
2970 tag = req->tag;
2971 WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
2972 /* Set the timeout such that the SCSI error handler is not activated. */
2973 req->timeout = msecs_to_jiffies(2 * timeout);
2974 blk_mq_start_request(req);
2975
2976 if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
2977 err = -EBUSY;
2978 goto out;
^^^^^^^^
2979 }
2980
2981 init_completion(&wait);
2982 lrbp = &hba->lrb[tag];
This used to be initialized before the goto
2983 WARN_ON(lrbp->cmd);
2984 err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag);
2985 if (unlikely(err))
2986 goto out_put_tag;
2987
2988 hba->dev_cmd.complete = &wait;
2989
2990 ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr);
2991 /* Make sure descriptors are ready before ringing the doorbell */
2992 wmb();
2993
2994 ufshcd_send_command(hba, tag);
2995 err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);
2996 out:
2997 ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : UFS_QUERY_COMP,
2998 (struct utp_upiu_req *)lrbp->ucd_rsp_ptr);
^^^^^^^^^^^^^^^^^
2999
3000 out_put_tag:
3001 blk_put_request(req);
3002 out_unlock:
3003 up_read(&hba->clk_scaling_lock);
3004 return err;
3005 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] scsi: ufs: Optimize host lock on transfer requests send/compl paths
2021-06-09 11:01 [bug report] scsi: ufs: Optimize host lock on transfer requests send/compl paths Dan Carpenter
@ 2021-06-10 0:54 ` Can Guo
0 siblings, 0 replies; 2+ messages in thread
From: Can Guo @ 2021-06-10 0:54 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-scsi
Hi Dan,
On 2021-06-09 19:01, Dan Carpenter wrote:
> Hello Can Guo,
>
> The patch a45f937110fa: "scsi: ufs: Optimize host lock on transfer
> requests send/compl paths" from May 24, 2021, leads to the following
> static checker warning:
>
> drivers/scsi/ufs/ufshcd.c:2998 ufshcd_exec_dev_cmd()
> error: potentially dereferencing uninitialized 'lrbp'.
>
I uploaded a fix yesterday -
https://lore.kernel.org/patchwork/patch/1443774/
Thanks for reporting it and sorry for the disturb.
Regards,
Can Guo.
> drivers/scsi/ufs/ufshcd.c
> 2948 static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
> 2949 enum dev_cmd_type cmd_type, int timeout)
> 2950 {
> 2951 struct request_queue *q = hba->cmd_queue;
> 2952 struct request *req;
> 2953 struct ufshcd_lrb *lrbp;
> ^^^^
>
> 2954 int err;
> 2955 int tag;
> 2956 struct completion wait;
> 2957
> 2958 down_read(&hba->clk_scaling_lock);
> 2959
> 2960 /*
> 2961 * Get free slot, sleep if slots are unavailable.
> 2962 * Even though we use wait_event() which sleeps
> indefinitely,
> 2963 * the maximum wait time is bounded by SCSI request
> timeout.
> 2964 */
> 2965 req = blk_get_request(q, REQ_OP_DRV_OUT, 0);
> 2966 if (IS_ERR(req)) {
> 2967 err = PTR_ERR(req);
> 2968 goto out_unlock;
> 2969 }
> 2970 tag = req->tag;
> 2971 WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
> 2972 /* Set the timeout such that the SCSI error handler is
> not activated. */
> 2973 req->timeout = msecs_to_jiffies(2 * timeout);
> 2974 blk_mq_start_request(req);
> 2975
> 2976 if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
> 2977 err = -EBUSY;
> 2978 goto out;
> ^^^^^^^^
>
> 2979 }
> 2980
> 2981 init_completion(&wait);
> 2982 lrbp = &hba->lrb[tag];
>
> This used to be initialized before the goto
>
> 2983 WARN_ON(lrbp->cmd);
> 2984 err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag);
> 2985 if (unlikely(err))
> 2986 goto out_put_tag;
> 2987
> 2988 hba->dev_cmd.complete = &wait;
> 2989
> 2990 ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND,
> lrbp->ucd_req_ptr);
> 2991 /* Make sure descriptors are ready before ringing the
> doorbell */
> 2992 wmb();
> 2993
> 2994 ufshcd_send_command(hba, tag);
> 2995 err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout);
> 2996 out:
> 2997 ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR :
> UFS_QUERY_COMP,
> 2998 (struct utp_upiu_req
> *)lrbp->ucd_rsp_ptr);
>
> ^^^^^^^^^^^^^^^^^
>
> 2999
> 3000 out_put_tag:
> 3001 blk_put_request(req);
> 3002 out_unlock:
> 3003 up_read(&hba->clk_scaling_lock);
> 3004 return err;
> 3005 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-10 0:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 11:01 [bug report] scsi: ufs: Optimize host lock on transfer requests send/compl paths Dan Carpenter
2021-06-10 0:54 ` Can Guo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.