* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
@ 2018-09-14 1:52 ` Ming Lei
0 siblings, 0 replies; 15+ 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
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] 15+ 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
-1 siblings, 0 replies; 15+ 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] 15+ messages in thread
* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
@ 2018-09-14 6:17 ` Adrian Hunter
0 siblings, 0 replies; 15+ 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
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] 15+ 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
-1 siblings, 0 replies; 15+ 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] 15+ messages in thread
* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
@ 2018-09-14 13:03 ` Adrian Hunter
0 siblings, 0 replies; 15+ 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
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] 15+ messages in thread
* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
2018-09-14 1:52 ` Ming Lei
@ 2018-09-20 6:58 ` Christoph Hellwig
-1 siblings, 0 replies; 15+ 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] 15+ messages in thread
* Re: [PATCH RFC] scsi: ufs: Disable blk-mq for now
@ 2018-09-20 6:58 ` Christoph Hellwig
0 siblings, 0 replies; 15+ 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, Jaegeu
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] 15+ messages in thread