On 09/02/2023 18:53, Bart Van Assche wrote:
Do not set RQF_PM explicitly since scsi_alloc_request() sets it indirectly
if BLK_MQ_REQ_PM is set. The call chain for the code that sets RQF_PM is
as follows:
scsi_alloc_request()
blk_mq_alloc_request()
__blk_mq_alloc_requests()
blk_mq_rq_ctx_init()
if (data->flags & BLK_MQ_REQ_PM)
data->rq_flags |= RQF_PM;
Cc: Mike Christie <michael.christie@xxxxxxxxxx>
Cc: John Garry <john.g.garry@xxxxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
To me, this looks ok. JFYI, this is where I understood that Christoph
mentioned that we are required to set both flags:
https://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/20221115081355.GB17445@xxxxxx/__;!!ACWV5N9M2RV99hQ!JQJXru7NACJ7vH7xvV-26xPrgI4gDRZ8NP0Q9uYYKB_5RYq949y6HINgGq1D6prwH3V9FU21ntPhxpoi--Pe_qd2tz0$
However, to me, we currently set RQF_PM after alloc the req, so I don't
see why just using BLK_MQ_REQ_PM in the scsi_alloc_request() call won't
suffice (since it would lead to RQF_PM being set).
Anyway, feel free to add:
Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>
---
drivers/ufs/core/ufshcd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0cfe112ff8c3..3512dcc152c8 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9141,7 +9141,8 @@ static int ufshcd_execute_start_stop(struct scsi_device *sdev,
scmd->allowed = 0/*retries*/;
scmd->flags |= SCMD_FAIL_IF_RECOVERING;
req->timeout = 1 * HZ;
- req->rq_flags |= RQF_PM | RQF_QUIET;
+ req->rq_flags |= RQF_QUIET;
+ WARN_ON_ONCE(!(req->rq_flags & RQF_PM));
blk_execute_rq(req, /*at_head=*/true);
