LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH RFC] scsi: ufs: Disable blk-mq for now
@ 2018-09-13 11:28 Adrian Hunter
  2018-09-13 12:05 ` Ming Lei
  2018-09-21  1:59 ` Martin K. Petersen
  0 siblings, 2 replies; 8+ messages in thread
From: Adrian Hunter @ 2018-09-13 11:28 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: Jens Axboe, Christoph Hellwig, Ming Lei, Johannes Thumshirn,
	Bart Van Assche, linux-scsi, linux-kernel, Stanislav Nijnikov,
	Evan Green, Vinayak Holikatti, Janek Kotas, Vivek Gautam,
	Asutosh Das, Subhash Jadavani, Sayali Lokhande, Li Wei,
	Bjorn Andersson, Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-13 11:28 [PATCH RFC] scsi: ufs: Disable blk-mq for now Adrian Hunter
@ 2018-09-13 12:05 ` Ming Lei
  2018-09-13 12:15   ` Adrian Hunter
  2018-09-21  1:59 ` Martin K. Petersen
  1 sibling, 1 reply; 8+ messages in thread
From: Ming Lei @ 2018-09-13 12:05 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Martin K . Petersen, James E . J . Bottomley, Jens Axboe,
	Christoph Hellwig, Johannes Thumshirn, Bart Van Assche,
	linux-scsi, linux-kernel, Stanislav Nijnikov, Evan Green,
	Vinayak Holikatti, Janek Kotas, Vivek Gautam, Asutosh Das,
	Subhash Jadavani, Sayali Lokhande, Li Wei, Bjorn Andersson,
	Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-13 12:05 ` Ming Lei
@ 2018-09-13 12:15   ` Adrian Hunter
  2018-09-14  1:52     ` Ming Lei
  0 siblings, 1 reply; 8+ messages in thread
From: Adrian Hunter @ 2018-09-13 12:15 UTC (permalink / raw)
  To: Ming Lei
  Cc: Martin K . Petersen, James E . J . Bottomley, Jens Axboe,
	Christoph Hellwig, Johannes Thumshirn, Bart Van Assche,
	linux-scsi, linux-kernel, Stanislav Nijnikov, Evan Green,
	Vinayak Holikatti, Janek Kotas, Vivek Gautam, Asutosh Das,
	Subhash Jadavani, Sayali Lokhande, Li Wei, Bjorn Andersson,
	Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-13 12:15   ` Adrian Hunter
@ 2018-09-14  1:52     ` Ming Lei
  2018-09-14  6:17       ` Adrian Hunter
  2018-09-20  6:58       ` Christoph Hellwig
  0 siblings, 2 replies; 8+ messages in thread
From: Ming Lei @ 2018-09-14  1:52 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Martin K . Petersen, James E . J . Bottomley, Jens Axboe,
	Christoph Hellwig, Johannes Thumshirn, Bart Van Assche,
	linux-scsi, linux-kernel, Stanislav Nijnikov, Evan Green,
	Vinayak Holikatti, Janek Kotas, Vivek Gautam, Asutosh Das,
	Subhash Jadavani, Sayali Lokhande, Li Wei, Bjorn Andersson,
	Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-14  1:52     ` Ming Lei
@ 2018-09-14  6:17       ` Adrian Hunter
  2018-09-14 13:03         ` Adrian Hunter
  2018-09-20  6:58       ` Christoph Hellwig
  1 sibling, 1 reply; 8+ messages in thread
From: Adrian Hunter @ 2018-09-14  6:17 UTC (permalink / raw)
  To: Ming Lei
  Cc: Martin K . Petersen, James E . J . Bottomley, Jens Axboe,
	Christoph Hellwig, Johannes Thumshirn, Bart Van Assche,
	linux-scsi, linux-kernel, Stanislav Nijnikov, Evan Green,
	Vinayak Holikatti, Janek Kotas, Vivek Gautam, Asutosh Das,
	Subhash Jadavani, Sayali Lokhande, Li Wei, Bjorn Andersson,
	Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-14  6:17       ` Adrian Hunter
@ 2018-09-14 13:03         ` Adrian Hunter
  0 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2018-09-14 13:03 UTC (permalink / raw)
  To: Ming Lei
  Cc: Martin K . Petersen, James E . J . Bottomley, Jens Axboe,
	Christoph Hellwig, Johannes Thumshirn, Bart Van Assche,
	linux-scsi, linux-kernel, Stanislav Nijnikov, Evan Green,
	Vinayak Holikatti, Janek Kotas, Vivek Gautam, Asutosh Das,
	Subhash Jadavani, Sayali Lokhande, Li Wei, Bjorn Andersson,
	Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-14  1:52     ` Ming Lei
  2018-09-14  6:17       ` Adrian Hunter
@ 2018-09-20  6:58       ` Christoph Hellwig
  1 sibling, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-09-20  6:58 UTC (permalink / raw)
  To: Ming Lei
  Cc: Adrian Hunter, Martin K . Petersen, James E . J . Bottomley,
	Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
	Bart Van Assche, linux-scsi, linux-kernel, Stanislav Nijnikov,
	Evan Green, Vinayak Holikatti, Janek Kotas, Vivek Gautam,
	Asutosh Das, Subhash Jadavani, Sayali Lokhande, Li Wei,
	Bjorn Andersson, Jaegeuk Kim, Alim Akhtar, Alex Lemberg

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>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
  2018-09-13 11:28 [PATCH RFC] scsi: ufs: Disable blk-mq for now Adrian Hunter
  2018-09-13 12:05 ` Ming Lei
@ 2018-09-21  1:59 ` Martin K. Petersen
  1 sibling, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2018-09-21  1:59 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Martin K . Petersen, James E . J . Bottomley, Jens Axboe,
	Christoph Hellwig, Ming Lei, Johannes Thumshirn, Bart Van Assche,
	linux-scsi, linux-kernel, Stanislav Nijnikov, Evan Green,
	Vinayak Holikatti, Janek Kotas, Vivek Gautam, Asutosh Das,
	Subhash Jadavani, Sayali Lokhande, Li Wei, Bjorn Andersson,
	Jaegeuk Kim, Alim Akhtar, Alex Lemberg


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-13 11:28 [PATCH RFC] scsi: ufs: Disable blk-mq for now Adrian Hunter
2018-09-13 12:05 ` Ming Lei
2018-09-13 12:15   ` Adrian Hunter
2018-09-14  1:52     ` Ming Lei
2018-09-14  6:17       ` Adrian Hunter
2018-09-14 13:03         ` Adrian Hunter
2018-09-20  6:58       ` Christoph Hellwig
2018-09-21  1:59 ` Martin K. Petersen

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox