* [PATCH 1/2] scsi: ufs: Unlock on a couple error paths
@ 2019-12-13 10:48 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-12-13 10:48 UTC (permalink / raw)
To: Alim Akhtar, Bart Van Assche
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
We introduced a few new error paths, but we can't return directly, we
first have to unlock "hba->clk_scaling_lock" first.
Fixes: a276c19e3e98 ("scsi: ufs: Avoid busy-waiting by eliminating tag conflicts")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/scsi/ufs/ufshcd.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e5cd57e68c46..bf981f0ea74c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2617,8 +2617,10 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
* the maximum wait time is bounded by SCSI request timeout.
*/
req = blk_get_request(q, REQ_OP_DRV_OUT, 0);
- if (IS_ERR(req))
- return PTR_ERR(req);
+ if (IS_ERR(req)) {
+ err = PTR_ERR(req);
+ goto out_unlock;
+ }
tag = req->tag;
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
@@ -2646,6 +2648,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
out_put_tag:
blk_put_request(req);
+out_unlock:
up_read(&hba->clk_scaling_lock);
return err;
}
@@ -5854,8 +5857,10 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
down_read(&hba->clk_scaling_lock);
req = blk_get_request(q, REQ_OP_DRV_OUT, 0);
- if (IS_ERR(req))
- return PTR_ERR(req);
+ if (IS_ERR(req)) {
+ err = PTR_ERR(req);
+ goto out_unlock;
+ }
tag = req->tag;
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
@@ -5934,6 +5939,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
}
blk_put_request(req);
+out_unlock:
up_read(&hba->clk_scaling_lock);
return err;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] scsi: ufs: Unlock on a couple error paths
@ 2019-12-13 10:48 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-12-13 10:48 UTC (permalink / raw)
To: Alim Akhtar, Bart Van Assche
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
We introduced a few new error paths, but we can't return directly, we
first have to unlock "hba->clk_scaling_lock" first.
Fixes: a276c19e3e98 ("scsi: ufs: Avoid busy-waiting by eliminating tag conflicts")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/scsi/ufs/ufshcd.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e5cd57e68c46..bf981f0ea74c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2617,8 +2617,10 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
* the maximum wait time is bounded by SCSI request timeout.
*/
req = blk_get_request(q, REQ_OP_DRV_OUT, 0);
- if (IS_ERR(req))
- return PTR_ERR(req);
+ if (IS_ERR(req)) {
+ err = PTR_ERR(req);
+ goto out_unlock;
+ }
tag = req->tag;
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
@@ -2646,6 +2648,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
out_put_tag:
blk_put_request(req);
+out_unlock:
up_read(&hba->clk_scaling_lock);
return err;
}
@@ -5854,8 +5857,10 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
down_read(&hba->clk_scaling_lock);
req = blk_get_request(q, REQ_OP_DRV_OUT, 0);
- if (IS_ERR(req))
- return PTR_ERR(req);
+ if (IS_ERR(req)) {
+ err = PTR_ERR(req);
+ goto out_unlock;
+ }
tag = req->tag;
WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag));
@@ -5934,6 +5939,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
}
blk_put_request(req);
+out_unlock:
up_read(&hba->clk_scaling_lock);
return err;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] scsi: ufs: Simplify a condition
2019-12-13 10:48 ` Dan Carpenter
@ 2019-12-13 10:49 ` Dan Carpenter
-1 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-12-13 10:49 UTC (permalink / raw)
To: Alim Akhtar, Subhash Jadavani
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler,
linux-scsi, kernel-janitors
We know that "check_for_bkops" is non-zero on this side of the ||
because it was checked on the other side.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/scsi/ufs/ufshcd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf981f0ea74c..c299c5feaf1a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7684,8 +7684,7 @@ static int ufshcd_link_state_transition(struct ufs_hba *hba,
* turning off the link would also turn off the device.
*/
else if ((req_link_state = UIC_LINK_OFF_STATE) &&
- (!check_for_bkops || (check_for_bkops &&
- !hba->auto_bkops_enabled))) {
+ (!check_for_bkops || !hba->auto_bkops_enabled)) {
/*
* Let's make sure that link is in low power mode, we are doing
* this currently by putting the link in Hibern8. Otherway to
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] scsi: ufs: Simplify a condition
@ 2019-12-13 10:49 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-12-13 10:49 UTC (permalink / raw)
To: Alim Akhtar, Subhash Jadavani
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Bart Van Assche, Venkat Gopalakrishnan, Tomas Winkler,
linux-scsi, kernel-janitors
We know that "check_for_bkops" is non-zero on this side of the ||
because it was checked on the other side.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/scsi/ufs/ufshcd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf981f0ea74c..c299c5feaf1a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7684,8 +7684,7 @@ static int ufshcd_link_state_transition(struct ufs_hba *hba,
* turning off the link would also turn off the device.
*/
else if ((req_link_state == UIC_LINK_OFF_STATE) &&
- (!check_for_bkops || (check_for_bkops &&
- !hba->auto_bkops_enabled))) {
+ (!check_for_bkops || !hba->auto_bkops_enabled)) {
/*
* Let's make sure that link is in low power mode, we are doing
* this currently by putting the link in Hibern8. Otherway to
--
2.11.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Unlock on a couple error paths
2019-12-13 10:48 ` Dan Carpenter
@ 2019-12-13 20:04 ` Bart Van Assche
-1 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2019-12-13 20:04 UTC (permalink / raw)
To: Dan Carpenter, Alim Akhtar
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
On 12/13/19 5:48 AM, Dan Carpenter wrote:
> We introduced a few new error paths, but we can't return directly, we
> first have to unlock "hba->clk_scaling_lock" first.
This may have escaped from my attention due to having swapped the order
of the patch that removed that locking and the patch this patch is a fix
for. Anyway, thanks for this patch.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Unlock on a couple error paths
@ 2019-12-13 20:04 ` Bart Van Assche
0 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2019-12-13 20:04 UTC (permalink / raw)
To: Dan Carpenter, Alim Akhtar
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
On 12/13/19 5:48 AM, Dan Carpenter wrote:
> We introduced a few new error paths, but we can't return directly, we
> first have to unlock "hba->clk_scaling_lock" first.
This may have escaped from my attention due to having swapped the order
of the patch that removed that locking and the patch this patch is a fix
for. Anyway, thanks for this patch.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: Simplify a condition
2019-12-13 10:49 ` Dan Carpenter
@ 2019-12-13 20:05 ` Bart Van Assche
-1 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2019-12-13 20:05 UTC (permalink / raw)
To: Dan Carpenter, Alim Akhtar, Subhash Jadavani
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
On 12/13/19 5:49 AM, Dan Carpenter wrote:
> We know that "check_for_bkops" is non-zero on this side of the ||
> because it was checked on the other side.
How about also removing the superfluous parentheses? Anyway:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: Simplify a condition
@ 2019-12-13 20:05 ` Bart Van Assche
0 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2019-12-13 20:05 UTC (permalink / raw)
To: Dan Carpenter, Alim Akhtar, Subhash Jadavani
Cc: Avri Altman, Pedro Sousa, James E.J. Bottomley,
Martin K. Petersen, Bean Huo, Stanley Chu, Can Guo,
Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
On 12/13/19 5:49 AM, Dan Carpenter wrote:
> We know that "check_for_bkops" is non-zero on this side of the ||
> because it was checked on the other side.
How about also removing the superfluous parentheses? Anyway:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: Simplify a condition
2019-12-13 20:05 ` Bart Van Assche
@ 2019-12-14 4:28 ` Dan Carpenter
-1 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-12-14 4:28 UTC (permalink / raw)
To: Bart Van Assche
Cc: Alim Akhtar, Subhash Jadavani, Avri Altman, Pedro Sousa,
James E.J. Bottomley, Martin K. Petersen, Bean Huo, Stanley Chu,
Can Guo, Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
On Fri, Dec 13, 2019 at 01:05:55PM -0700, Bart Van Assche wrote:
> On 12/13/19 5:49 AM, Dan Carpenter wrote:
> > We know that "check_for_bkops" is non-zero on this side of the ||
> > because it was checked on the other side.
>
> How about also removing the superfluous parentheses? Anyway:
>
Around "(req_link_state = UIC_LINK_OFF_STATE)"? I considered it but
some people like them...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: Simplify a condition
@ 2019-12-14 4:28 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2019-12-14 4:28 UTC (permalink / raw)
To: Bart Van Assche
Cc: Alim Akhtar, Subhash Jadavani, Avri Altman, Pedro Sousa,
James E.J. Bottomley, Martin K. Petersen, Bean Huo, Stanley Chu,
Can Guo, Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
On Fri, Dec 13, 2019 at 01:05:55PM -0700, Bart Van Assche wrote:
> On 12/13/19 5:49 AM, Dan Carpenter wrote:
> > We know that "check_for_bkops" is non-zero on this side of the ||
> > because it was checked on the other side.
>
> How about also removing the superfluous parentheses? Anyway:
>
Around "(req_link_state == UIC_LINK_OFF_STATE)"? I considered it but
some people like them...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/2] scsi: ufs: Simplify a condition
2019-12-13 10:49 ` Dan Carpenter
(?)
(?)
@ 2019-12-15 6:06 ` Avri Altman
-1 siblings, 0 replies; 13+ messages in thread
From: Avri Altman @ 2019-12-15 6:06 UTC (permalink / raw)
To: Dan Carpenter, Alim Akhtar, Subhash Jadavani
Cc: Pedro Sousa, James E.J. Bottomley, Martin K. Petersen, Bean Huo,
Stanley Chu, Can Guo, Bart Van Assche, Venkat Gopalakrishnan,
Tomas Winkler, linux-scsi, kernel-janitors
>
> We know that "check_for_bkops" is non-zero on this side of the || because it
> was checked on the other side.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
> bf981f0ea74c..c299c5feaf1a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7684,8 +7684,7 @@ static int ufshcd_link_state_transition(struct ufs_hba
> *hba,
> * turning off the link would also turn off the device.
> */
> else if ((req_link_state == UIC_LINK_OFF_STATE) &&
> - (!check_for_bkops || (check_for_bkops &&
> - !hba->auto_bkops_enabled))) {
> + (!check_for_bkops || !hba->auto_bkops_enabled)) {
> /*
> * Let's make sure that link is in low power mode, we are doing
> * this currently by putting the link in Hibern8. Otherway to
> --
> 2.11.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Unlock on a couple error paths
2019-12-13 10:48 ` Dan Carpenter
@ 2019-12-17 3:04 ` Martin K. Petersen
-1 siblings, 0 replies; 13+ messages in thread
From: Martin K. Petersen @ 2019-12-17 3:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alim Akhtar, Bart Van Assche, Avri Altman, Pedro Sousa,
James E.J. Bottomley, Martin K. Petersen, Bean Huo, Stanley Chu,
Can Guo, Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
Dan,
> We introduced a few new error paths, but we can't return directly, we
> first have to unlock "hba->clk_scaling_lock" first.
Applied #1 + #2 to 5.6/scsi-queue. Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Unlock on a couple error paths
@ 2019-12-17 3:04 ` Martin K. Petersen
0 siblings, 0 replies; 13+ messages in thread
From: Martin K. Petersen @ 2019-12-17 3:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alim Akhtar, Bart Van Assche, Avri Altman, Pedro Sousa,
James E.J. Bottomley, Martin K. Petersen, Bean Huo, Stanley Chu,
Can Guo, Venkat Gopalakrishnan, Tomas Winkler, linux-scsi,
kernel-janitors
Dan,
> We introduced a few new error paths, but we can't return directly, we
> first have to unlock "hba->clk_scaling_lock" first.
Applied #1 + #2 to 5.6/scsi-queue. Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-12-17 3:05 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-13 10:48 [PATCH 1/2] scsi: ufs: Unlock on a couple error paths Dan Carpenter
2019-12-13 10:48 ` Dan Carpenter
2019-12-13 10:49 ` [PATCH 2/2] scsi: ufs: Simplify a condition Dan Carpenter
2019-12-13 10:49 ` Dan Carpenter
2019-12-13 20:05 ` Bart Van Assche
2019-12-13 20:05 ` Bart Van Assche
2019-12-14 4:28 ` Dan Carpenter
2019-12-14 4:28 ` Dan Carpenter
2019-12-15 6:06 ` Avri Altman
2019-12-13 20:04 ` [PATCH 1/2] scsi: ufs: Unlock on a couple error paths Bart Van Assche
2019-12-13 20:04 ` Bart Van Assche
2019-12-17 3:04 ` Martin K. Petersen
2019-12-17 3:04 ` Martin K. Petersen
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.