Re: [PATCH RFC V2 3/3] scsi_mq: enable runtime PM

Ming Lei
2018-07-18T13:05:26Z

On Wed, Jul 18, 2018 at 8:43 PM, Hannes Reinecke <hare@suse.de> wrote:
> On 07/18/2018 02:06 PM, Ming Lei wrote:
>> On Wed, Jul 18, 2018 at 5:49 AM, Jens Axboe <axboe@kernel.dk> wrote:
>>> On 7/17/18 9:38 AM, Ming Lei wrote:
>>>> On Tue, Jul 17, 2018 at 03:34:35PM +0000, Bart Van Assche wrote:
>>>>> On Tue, 2018-07-17 at 23:30 +0800, Ming Lei wrote:
>>>>>> On Tue, Jul 17, 2018 at 03:24:11PM +0200, Christoph Hellwig wrote:
>>>>>>> On Fri, Jul 13, 2018 at 04:06:02PM +0800, Ming Lei wrote:
>>>>>>>> Usually SCSI supports runtime PM, so pass BLK_MQ_F_SUPPORT_RPM to blk-mq
>>>>>>>> core for enabling block runtime PM.
>>>>>>>
>>>>>>> I still think enabling this unconditionally for any SCSI device was
>>>>>>> a mistake, and it is even more so for blk-mq.
>>>>>>>
>>>>>>> Please only opt in for ufs, ATA first, adding others if wanted by
>>>>>>> maintainers.
>>>>>>
>>>>>> No, this way will cause regression because runtime PM works for
>>>>>> all sd/sr device actually, and it isn't related with scsi host.
>>>>>
>>>>> For which SCSI devices other than ufs and ATA do we need PM support?
>>>>
>>>> As I said, it is any sd/sr device, which can be put down by runtime PM
>>>> via START_STOP command if it isn't used for a while.
>>>
>>> Christoph is basically echoing my concerns. Why don't we just enable
>>> it on slower devices, similarly to what we do for adding
>>> randomness? Nobody wants to pay this overhead for faster devices,
>>> since most people won't care.
>>
>> IMO the problem isn't related with slow or quick device, it is related with
>> the system, especially when it cares about power consumption, such as
>> mobile phone, or laptop or servers with lots of disks attached. And we know
>> it is often to see some fast disks shipped in laptop, such as NVMe, or other
>> SSD.
>>
> But those typically have dedicated (transport/driver) mechanism to put
> the device to sleep, _and_ START STOP UNIT will be emulated for those
> devices anyway.
> I'd rather advocate to move runtime PM into the individual
> subsystems/drivers, and _not_ abuse START STOP UNIT here.

Firstly START_STOP isn't a abuse for PM purpose, please see
the description in t10.org:

"The START STOP UNIT command (see 5.17) allows an application client to
control the power condition of a logical unit. This method includes specifying
that the logical unit transition to a power condition."

Secondly the runtime PM on LUN level isn't contradictory with subsystem/driver's
runtime PM. The two can co-exits, for examples, USB storage host, ufshcd, ...

Thanks,
Ming Lei