[PATCH RFC] scsi: ufs: Disable blk-mq for now

[PATCH RFC] scsi: ufs: Disable blk-mq for now

[Adrian Hunter]

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 <adrian.hunter@intel.com>
---
 drivers/scsi/ufs/ufshcd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 40548bae8efa..a4d36497a047 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -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;
-- 
2.17.1

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Ming Lei]

On Thu, Sep 13, 2018 at 02:28:48PM +0300, Adrian Hunter wrote:
> 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

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Adrian Hunter]

On 13/09/18 15:05, Ming Lei wrote:
> On Thu, Sep 13, 2018 at 02:28:48PM +0300, Adrian Hunter wrote:
>> 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.

> 
> 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.

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Ming Lei]

On Thu, Sep 13, 2018 at 03:15:39PM +0300, Adrian Hunter wrote:
> On 13/09/18 15:05, Ming Lei wrote:
> > On Thu, Sep 13, 2018 at 02:28:48PM +0300, Adrian Hunter wrote:
> >> 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.
> 
> > 
> > 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.

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

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Adrian Hunter]

On 14/09/18 04:52, Ming Lei wrote:
> On Thu, Sep 13, 2018 at 03:15:39PM +0300, Adrian Hunter wrote:
>> On 13/09/18 15:05, Ming Lei wrote:
>>> On Thu, Sep 13, 2018 at 02:28:48PM +0300, Adrian Hunter wrote:
>>>> 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.
>>
>>>
>>> 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.
> 
> 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")

> 
> 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.

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Adrian Hunter]

On 14/09/18 09:17, Adrian Hunter wrote:
> On 14/09/18 04:52, Ming Lei wrote:
>> On Thu, Sep 13, 2018 at 03:15:39PM +0300, Adrian Hunter wrote:
>>> On 13/09/18 15:05, Ming Lei wrote:
>>>> On Thu, Sep 13, 2018 at 02:28:48PM +0300, Adrian Hunter wrote:
>>>>> 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.
>>>
>>>>
>>>> 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.
>>
>> 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")
> 
>>
>> 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 <adrian.hunter@intel.com>

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

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Christoph Hellwig]

On Fri, Sep 14, 2018 at 09:52:38AM +0800, Ming Lei wrote:
> 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 <hch@lst.de>

Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now

[Martin K. Petersen]

Adrian,

> 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!

-- 
Martin K. Petersen	Oracle Linux Engineering