| Message ID | 20180711162906.14271-5-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
> @@ -3770,7 +3770,7 @@ EXPORT_SYMBOL(blk_finish_plug); > void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > { > /* not support for RQF_PM and ->rpm_status in blk-mq yet */ > - if (q->mq_ops) > + if (q->mq_ops && !(q->tag_set->flags & BLK_MQ_F_SUPPORT_RPM)) > return; > > q->dev = dev; This should go into a block layer patch, not a scsi one. > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 41e9ac9fc138..fa4667aa4732 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -2306,7 +2306,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) > shost->tag_set.queue_depth = shost->can_queue; > shost->tag_set.cmd_size = cmd_size; > shost->tag_set.numa_node = NUMA_NO_NODE; > - shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE; > + shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | > + BLK_MQ_F_SG_MERGE | BLK_MQ_F_SUPPORT_RPM; As far as I can tell only ufs and libata support runtime PM, so we should probably only enable it for those.
On Thu, 12 Jul 2018, Ming Lei wrote: > Usually SCSI supports runtime PM, so pass BLK_MQ_F_SUPPORT_RPM to blk-mq > core for enabling block runtime PM. > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: linux-pm@vger.kernel.org > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Bart Van Assche <bart.vanassche@wdc.com> > Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com> > Cc: "Martin K. Petersen" <martin.petersen@oracle.com> > Cc: linux-scsi@vger.kernel.org > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > block/blk-core.c | 2 +- > drivers/scsi/scsi_lib.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index bf66d561980d..9e47205366ab 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -3770,7 +3770,7 @@ EXPORT_SYMBOL(blk_finish_plug); > void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > { > /* not support for RQF_PM and ->rpm_status in blk-mq yet */ Shouldn't that comment be removed? Alan Stern
Hi Christoph, On Wed, Jul 11, 2018 at 07:14:09PM +0200, Christoph Hellwig wrote: > > @@ -3770,7 +3770,7 @@ EXPORT_SYMBOL(blk_finish_plug); > > void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > > { > > /* not support for RQF_PM and ->rpm_status in blk-mq yet */ > > - if (q->mq_ops) > > + if (q->mq_ops && !(q->tag_set->flags & BLK_MQ_F_SUPPORT_RPM)) > > return; > > > > q->dev = dev; > > This should go into a block layer patch, not a scsi one. OK. > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > index 41e9ac9fc138..fa4667aa4732 100644 > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -2306,7 +2306,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) > > shost->tag_set.queue_depth = shost->can_queue; > > shost->tag_set.cmd_size = cmd_size; > > shost->tag_set.numa_node = NUMA_NO_NODE; > > - shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE; > > + shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | > > + BLK_MQ_F_SG_MERGE | BLK_MQ_F_SUPPORT_RPM; > > As far as I can tell only ufs and libata support runtime PM, so > we should probably only enable it for those. For legacy path, blk_pm_runtime_init() is always run for all SCSI devices, and this patch just follows the old way, so that we can keep runtime PM behaviour not changed from user view. That means if we want to only enable for ufs & libata, it should be in another standalone patch, instead of this one. I will address all your other comments and Alan's in V2. Thanks, Ming
On Thu, Jul 12, 2018 at 09:36:49AM +0800, Ming Lei wrote: > > As far as I can tell only ufs and libata support runtime PM, so > > we should probably only enable it for those. > > For legacy path, blk_pm_runtime_init() is always run for all SCSI devices, > and this patch just follows the old way, so that we can keep runtime PM > behaviour not changed from user view. That means if we want to only enable > for ufs & libata, it should be in another standalone patch, instead of > this one. Please add this to the series as a separate patch then.
On Thu, Jul 12, 2018 at 09:17:58AM +0200, Christoph Hellwig wrote: > On Thu, Jul 12, 2018 at 09:36:49AM +0800, Ming Lei wrote: > > > As far as I can tell only ufs and libata support runtime PM, so > > > we should probably only enable it for those. > > > > For legacy path, blk_pm_runtime_init() is always run for all SCSI devices, > > and this patch just follows the old way, so that we can keep runtime PM > > behaviour not changed from user view. That means if we want to only enable > > for ufs & libata, it should be in another standalone patch, instead of > > this one. > > Please add this to the series as a separate patch then. blk_pm_runtime_init() is called from sd/sr, that means all sd/sr device may support runtime PM. And the command of START_STOP will be run for runtime suspend/resume at least even though the host isn't involved. Thanks, Ming
diff --git a/block/blk-core.c b/block/blk-core.c index bf66d561980d..9e47205366ab 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -3770,7 +3770,7 @@ EXPORT_SYMBOL(blk_finish_plug); void blk_pm_runtime_init(struct request_queue *q, struct device *dev) { /* not support for RQF_PM and ->rpm_status in blk-mq yet */ - if (q->mq_ops) + if (q->mq_ops && !(q->tag_set->flags & BLK_MQ_F_SUPPORT_RPM)) return; q->dev = dev; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 41e9ac9fc138..fa4667aa4732 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2306,7 +2306,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) shost->tag_set.queue_depth = shost->can_queue; shost->tag_set.cmd_size = cmd_size; shost->tag_set.numa_node = NUMA_NO_NODE; - shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE; + shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | + BLK_MQ_F_SG_MERGE | BLK_MQ_F_SUPPORT_RPM; shost->tag_set.flags |= BLK_ALLOC_POLICY_TO_MQ_FLAG(shost->hostt->tag_alloc_policy); shost->tag_set.driver_data = shost;
Usually SCSI supports runtime PM, so pass BLK_MQ_F_SUPPORT_RPM to blk-mq core for enabling block runtime PM. Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: linux-pm@vger.kernel.org Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Christoph Hellwig <hch@lst.de> Cc: Bart Van Assche <bart.vanassche@wdc.com> Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: linux-scsi@vger.kernel.org Signed-off-by: Ming Lei <ming.lei@redhat.com> --- block/blk-core.c | 2 +- drivers/scsi/scsi_lib.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)