| Message ID | 20180913112848.7807-1-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [RFC] scsi: ufs: Disable blk-mq for now | expand |
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
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.
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
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.
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. >
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>
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!
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;
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(+)