scsi: ufs: Disable blk-mq for now

RFC PATCH Applied Adrian Hunter 4 participants 8 messages Sep 13, 2018 – Sep 21, 2018 SCSI

lore

Share & tools

lore

Adrian Hunter

author

2018-09-13 11:28 UTC #0 lore raw

RFC PATCH 1/1 scsi: ufs: Disable blk-mq for now

blk-mq does not support runtime pm, so disable blk-mq support for now.

Fixes d5038a13eca7 ("scsi: core: switch to scsi-mq by default")

Signed-off-by Adrian Hunter <[email protected]>

---
 drivers/scsi/ufs/ufshcd.c | 7 +++++++
 1 file changed, 7 insertions(+)

drivers/scsi/ufs/ufshcd.c +7 -0

index 40548bae8efa..a4d36497a047 100644
@@ -7975,6 +7975,13 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
 		err = -ENOMEM;
 		goto out_error;
 	}
+
+	/*
+	 * Do not use blk-mq at this time because blk-mq does not support
+	 * runtime pm.
+	 */
+	host->use_blk_mq = false;
+
 	hba = shost_priv(host);
 	hba->host = host;
 	hba->dev = dev;

Signature · 1 line

2.17.1

Ming Lei

2018-09-13 12:05 UTC #1 lore raw

Quoted 1 line · Adrian Hunter

blk-mq does not support runtime pm, so disable blk-mq support for now.

So could you describe a bit what the issue you are trying to fix?

This is host level runtime PM you are trying to address, and if blk-mq runtime isn't enabled, I guess the host won't be runtime suspended at all because some of its descendant are always active.

So seems we need to do nothing for preventing the host controller from entering runtime suspend.

Thanks,
Ming

Adrian Hunter

author

2018-09-13 12:15 UTC #2 lore raw

Quoted 4 lines · Ming Lei

Quoted 1 line · Adrian Hunter
blk-mq does not support runtime pm, so disable blk-mq support for now.
So could you describe a bit what the issue you are trying to fix?

UFS is a low-power solution, so we must be able to runtime suspend.

Quoted 7 lines

This is host level runtime PM you are trying to address, and if blk-mq runtime isn't enabled, I guess the host won't be runtime suspended at all because some of its descendant are always active.
So seems we need to do nothing for preventing the host controller from entering runtime suspend.

We don't want to prevent the host controller from runtime suspending, quite the opposite.

Ming Lei

2018-09-14 01:52 UTC #3 lore raw

Quoted 13 lines · Adrian Hunter

Quoted 4 lines · Ming Lei
Quoted 1 line · Adrian Hunter
blk-mq does not support runtime pm, so disable blk-mq support for now.
So could you describe a bit what the issue you are trying to fix?
UFS is a low-power solution, so we must be able to runtime suspend.
Quoted 5 lines
This is host level runtime PM you are trying to address, and if blk-mq runtime isn't enabled, I guess the host won't be runtime suspended at all because some of its descendant are always active.

Quoted 5 lines

Quoted 2 lines
So seems we need to do nothing for preventing the host controller from entering runtime suspend.
We don't want to prevent the host controller from runtime suspending, quite the opposite.

OK, got it.

However, in previous discussion, it is strongly objected to use per-driver/device .use_blk_mq switch, so not sure if this way can be accepted.

BTW, I just posted the runtime PM enablement patches[1] for blk-mq, and I verified that it works fine and passed blktests & xfstest & my other sanity tests, could you try it on UFS?

[1] https://marc.info/?l=linux-block&m=153684095523409&w=2

Thanks,
Ming

Adrian Hunter

author

2018-09-14 06:17 UTC #4 lore raw

Quoted 17 lines · Ming Lei

Quoted 16 lines · Adrian Hunter
Quoted 4 lines · Ming Lei
Quoted 1 line · Adrian Hunter
blk-mq does not support runtime pm, so disable blk-mq support for now.
So could you describe a bit what the issue you are trying to fix?
UFS is a low-power solution, so we must be able to runtime suspend.
Quoted 7 lines
This is host level runtime PM you are trying to address, and if blk-mq runtime isn't enabled, I guess the host won't be runtime suspended at all because some of its descendant are always active.
So seems we need to do nothing for preventing the host controller from entering runtime suspend.

Quoted 8 lines

Quoted 2 lines
We don't want to prevent the host controller from runtime suspending, quite the opposite.
OK, got it.
However, in previous discussion, it is strongly objected to use per-driver/device .use_blk_mq switch, so not sure if this way can be accepted.

It is only needed for 4.19 so far. Otherwise just revert d5038a13eca7 ("scsi: core: switch to scsi-mq by default")

Quoted 6 lines

BTW, I just posted the runtime PM enablement patches[1] for blk-mq, and I verified that it works fine and passed blktests & xfstest & my other sanity tests, could you try it on UFS?
[1] https://marc.info/?l=linux-block&m=153684095523409&w=2

I will give it a go.

Obviously, if those patches go in, we wouldn't need to disable blk-mq anymore, but that isn't until 4.20 at least.

Adrian Hunter

author Tested-by

2018-09-14 13:03 UTC #5 lore raw

Quoted 31 lines · Adrian Hunter

Quoted 17 lines · Ming Lei
Quoted 16 lines · Adrian Hunter
Quoted 4 lines · Ming Lei
Quoted 1 line · Adrian Hunter
blk-mq does not support runtime pm, so disable blk-mq support for now.
So could you describe a bit what the issue you are trying to fix?
UFS is a low-power solution, so we must be able to runtime suspend.
Quoted 7 lines
This is host level runtime PM you are trying to address, and if blk-mq runtime isn't enabled, I guess the host won't be runtime suspended at all because some of its descendant are always active.
So seems we need to do nothing for preventing the host controller from entering runtime suspend.
Quoted 8 lines
Quoted 2 lines
We don't want to prevent the host controller from runtime suspending, quite the opposite.
OK, got it.
However, in previous discussion, it is strongly objected to use per-driver/device .use_blk_mq switch, so not sure if this way can be accepted.
It is only needed for 4.19 so far. Otherwise just revert d5038a13eca7 ("scsi: core: switch to scsi-mq by default")
Quoted 1 line

Quoted 7 lines

Quoted 5 lines
BTW, I just posted the runtime PM enablement patches[1] for blk-mq, and I verified that it works fine and passed blktests & xfstest & my other sanity tests, could you try it on UFS?
[1] https://marc.info/?l=linux-block&m=153684095523409&w=2
I will give it a go.

Yes it seemed to work fine for UFS

Tested-by Adrian Hunter <[email protected]>

Quoted 4 lines

Obviously, if those patches go in, we wouldn't need to disable blk-mq anymore, but that isn't until 4.20 at least.

Christoph Hellwig

Acked-by

2018-09-20 06:58 UTC #6 lore raw

In reply to Ming Lei

Quoted 3 lines · Ming Lei

However, in previous discussion, it is strongly objected to use per-driver/device .use_blk_mq switch, so not sure if this way can be accepted.

I don't like the per-driver switch as module_parameter at all and we should never do that. We had some exceptions for drivers to force blk_mq (e.g. virtio), and given that I don't think we'll solve the blk-mq runtime-pm issue for 4.19 and also don't want to revert the default I think this patch is the best compromise for 4.19, with a revert in 4.20 once we have runtime-pm for blk-mq.

So:

Acked-by Christoph Hellwig <[email protected]>

Martin K. Petersen

Applied

2018-09-21 01:59 UTC #7 lore raw

In reply to Adrian Hunter

Adrian,

Quoted 1 line

blk-mq does not support runtime pm, so disable blk-mq support for now.

Applied to 4.19/scsi-fixes. Looking forward to getting this fixed up properly in 4.20.

Thanks!

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering