| Message ID | 1506083824-4024-10-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 22, 2017 at 2:36 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > Until mmc has blk-mq support fully implemented and tested, add a > parameter use_blk_mq, default to false unless config option MMC_MQ_DEFAULT > is selected. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > +config MMC_MQ_DEFAULT > + bool "MMC: use blk-mq I/O path by default" > + depends on MMC && BLOCK I would say: default y Why not. SCSI is starting to enable this by default so IMO we should not take the intermediate step of having this as optional. Otherwise it never gets tested. Set it to default y and after two kernel releases, if nothing happens, we simply delete the old block layer path. > +#ifdef CONFIG_MMC_MQ_DEFAULT > +bool mmc_use_blk_mq = true; > +#else > +bool mmc_use_blk_mq = false; > +#endif > +module_param_named(use_blk_mq, mmc_use_blk_mq, bool, S_IWUSR | S_IRUGO); Are people really modprobing this so it needs to be a module parameter? Maybe I'm the only developer stupid enough to just recompile and reboot the whole kernel, I guess this makes sense if you're testing on the same machine you're developing on (no cross-compilation and remote target) which I guess is what some Intel people are doing with their laptops. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/09/17 02:42, Linus Walleij wrote: > On Fri, Sep 22, 2017 at 2:36 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> Until mmc has blk-mq support fully implemented and tested, add a >> parameter use_blk_mq, default to false unless config option MMC_MQ_DEFAULT >> is selected. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > >> +config MMC_MQ_DEFAULT >> + bool "MMC: use blk-mq I/O path by default" >> + depends on MMC && BLOCK > > I would say: > default y > > Why not. SCSI is starting to enable this by default so IMO we should SCSI didn't manage it yet. > not take the > intermediate step of having this as optional. Otherwise it never gets tested. The argument that we don't have to take any responsibility for getting things tested is a poor one. Anyway, you can always send a patch later to change the default, so why the hurry to do it now? > > Set it to default y and after two kernel releases, if nothing happens, we simply > delete the old block layer path. > >> +#ifdef CONFIG_MMC_MQ_DEFAULT >> +bool mmc_use_blk_mq = true; >> +#else >> +bool mmc_use_blk_mq = false; >> +#endif >> +module_param_named(use_blk_mq, mmc_use_blk_mq, bool, S_IWUSR | S_IRUGO); > > Are people really modprobing this so it needs to be a module parameter? Irrespective of modprobe, the parameter can be changed without re-compiling. > > Maybe I'm the only developer stupid enough to just recompile and reboot Just change the parameter and unbind and rebind the host controller. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogbGludXgtbW1jLW93bmVy QHZnZXIua2VybmVsLm9yZyBbbWFpbHRvOmxpbnV4LW1tYy0NCj4gb3duZXJAdmdlci5rZXJuZWwu b3JnXSBPbiBCZWhhbGYgT2YgTGludXMgV2FsbGVpag0KPiBTZW50OiBXZWRuZXNkYXksIFNlcHRl bWJlciAyNywgMjAxNyAyOjQyIEFNDQo+IFRvOiBBZHJpYW4gSHVudGVyIDxhZHJpYW4uaHVudGVy QGludGVsLmNvbT4NCj4gQ2M6IFVsZiBIYW5zc29uIDx1bGYuaGFuc3NvbkBsaW5hcm8ub3JnPjsg bGludXgtbW1jIDxsaW51eC0NCj4gbW1jQHZnZXIua2VybmVsLm9yZz47IGxpbnV4LWJsb2NrIDxs aW51eC1ibG9ja0B2Z2VyLmtlcm5lbC5vcmc+OyBsaW51eC0NCj4ga2VybmVsIDxsaW51eC1rZXJu ZWxAdmdlci5rZXJuZWwub3JnPjsgQm91Z2ggQ2hlbiA8aGFpYm8uY2hlbkBueHAuY29tPjsNCj4g QWxleCBMZW1iZXJnIDxBbGV4LkxlbWJlcmdAd2RjLmNvbT47IE1hdGV1c3ogTm93YWsNCj4gPG1h dGV1c3oubm93YWtAaW50ZWwuY29tPjsgWXVsaXkgSXpyYWlsb3YgPFl1bGl5Lkl6cmFpbG92QHdk Yy5jb20+Ow0KPiBKYWVob29uIENodW5nIDxqaDgwLmNodW5nQHNhbXN1bmcuY29tPjsgRG9uZyBB aXNoZW5nDQo+IDxkb25nYXM4NkBnbWFpbC5jb20+OyBEYXMgQXN1dG9zaCA8YXN1dG9zaGRAY29k ZWF1cm9yYS5vcmc+OyBaaGFuZ2ZlaQ0KPiBHYW8gPHpoYW5nZmVpLmdhb0BnbWFpbC5jb20+OyBT YWhpdHlhIFR1bW1hbGENCj4gPHN0dW1tYWxhQGNvZGVhdXJvcmEub3JnPjsgSGFyamFuaSBSaXRl c2ggPHJpdGVzaGhAY29kZWF1cm9yYS5vcmc+Ow0KPiBWZW51IEJ5cmF2YXJhc3UgPHZieXJhdmFy YXN1QG52aWRpYS5jb20+OyBTaGF3biBMaW4gPHNoYXduLmxpbkByb2NrLQ0KPiBjaGlwcy5jb20+ OyBDaHJpc3RvcGggSGVsbHdpZyA8aGNoQGxzdC5kZT4NCj4gU3ViamVjdDogUmU6IFtQQVRDSCBW OSAwOS8xNV0gbW1jOiBjb3JlOiBBZGQgcGFyYW1ldGVyIHVzZV9ibGtfbXENCj4gDQo+IE9uIEZy aSwgU2VwIDIyLCAyMDE3IGF0IDI6MzYgUE0sIEFkcmlhbiBIdW50ZXIgPGFkcmlhbi5odW50ZXJA aW50ZWwuY29tPg0KPiB3cm90ZToNCj4gDQo+ID4gVW50aWwgbW1jIGhhcyBibGstbXEgc3VwcG9y dCBmdWxseSBpbXBsZW1lbnRlZCBhbmQgdGVzdGVkLCBhZGQgYQ0KPiA+IHBhcmFtZXRlciB1c2Vf YmxrX21xLCBkZWZhdWx0IHRvIGZhbHNlIHVubGVzcyBjb25maWcgb3B0aW9uDQo+ID4gTU1DX01R X0RFRkFVTFQgaXMgc2VsZWN0ZWQuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBBZHJpYW4gSHVu dGVyIDxhZHJpYW4uaHVudGVyQGludGVsLmNvbT4NCj4gDQo+ID4gK2NvbmZpZyBNTUNfTVFfREVG QVVMVA0KPiA+ICsgICAgICAgYm9vbCAiTU1DOiB1c2UgYmxrLW1xIEkvTyBwYXRoIGJ5IGRlZmF1 bHQiDQo+ID4gKyAgICAgICBkZXBlbmRzIG9uIE1NQyAmJiBCTE9DSw0KPiANCj4gSSB3b3VsZCBz YXk6DQo+IGRlZmF1bHQgeQ0KPiANCj4gV2h5IG5vdC4gU0NTSSBpcyBzdGFydGluZyB0byBlbmFi bGUgdGhpcyBieSBkZWZhdWx0IHNvIElNTyB3ZSBzaG91bGQgbm90IHRha2UNCj4gdGhlIGludGVy bWVkaWF0ZSBzdGVwIG9mIGhhdmluZyB0aGlzIGFzIG9wdGlvbmFsLiBPdGhlcndpc2UgaXQgbmV2 ZXIgZ2V0cw0KPiB0ZXN0ZWQuDQo+IA0KPiBTZXQgaXQgdG8gZGVmYXVsdCB5IGFuZCBhZnRlciB0 d28ga2VybmVsIHJlbGVhc2VzLCBpZiBub3RoaW5nIGhhcHBlbnMsIHdlIHNpbXBseQ0KPiBkZWxl dGUgdGhlIG9sZCBibG9jayBsYXllciBwYXRoLg0KPiANCj4gPiArI2lmZGVmIENPTkZJR19NTUNf TVFfREVGQVVMVA0KPiA+ICtib29sIG1tY191c2VfYmxrX21xID0gdHJ1ZTsNCj4gPiArI2Vsc2UN Cj4gPiArYm9vbCBtbWNfdXNlX2Jsa19tcSA9IGZhbHNlOw0KPiA+ICsjZW5kaWYNCj4gPiArbW9k dWxlX3BhcmFtX25hbWVkKHVzZV9ibGtfbXEsIG1tY191c2VfYmxrX21xLCBib29sLCBTX0lXVVNS IHwNCj4gPiArU19JUlVHTyk7DQo+IA0KPiBBcmUgcGVvcGxlIHJlYWxseSBtb2Rwcm9iaW5nIHRo aXMgc28gaXQgbmVlZHMgdG8gYmUgYSBtb2R1bGUgcGFyYW1ldGVyPw0KDQoNCk1vZHVsZSBwYXJh bSBjYW4gYmUgY2hhbmdlZCBpbiBydW50aW1lDQoNCj4gDQo+IE1heWJlIEknbSB0aGUgb25seSBk ZXZlbG9wZXIgc3R1cGlkIGVub3VnaCB0byBqdXN0IHJlY29tcGlsZSBhbmQgcmVib290IHRoZQ0K PiB3aG9sZSBrZXJuZWwsIEkgZ3Vlc3MgdGhpcyBtYWtlcyBzZW5zZSBpZiB5b3UncmUgdGVzdGlu ZyBvbiB0aGUgc2FtZSBtYWNoaW5lDQo+IHlvdSdyZSBkZXZlbG9waW5nIG9uIChubyBjcm9zcy1j b21waWxhdGlvbiBhbmQgcmVtb3RlIHRhcmdldCkgd2hpY2ggSSBndWVzcw0KPiBpcyB3aGF0IHNv bWUgSW50ZWwgcGVvcGxlIGFyZSBkb2luZyB3aXRoIHRoZWlyIGxhcHRvcHMuDQo+IA0KPiBZb3Vy cywNCj4gTGludXMgV2FsbGVpag0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlz dDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtbW1jIiBpbiB0aGUNCj4gYm9keSBv ZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZyBNb3JlIG1ham9yZG9tbyBp bmZvIGF0DQo+IGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 27, 2017 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 27/09/17 02:42, Linus Walleij wrote: >> On Fri, Sep 22, 2017 at 2:36 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >>> Until mmc has blk-mq support fully implemented and tested, add a >>> parameter use_blk_mq, default to false unless config option MMC_MQ_DEFAULT >>> is selected. >>> >>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >> >>> +config MMC_MQ_DEFAULT >>> + bool "MMC: use blk-mq I/O path by default" >>> + depends on MMC && BLOCK >> >> I would say: >> default y >> >> Why not. SCSI is starting to enable this by default so IMO we should > > SCSI didn't manage it yet. > >> not take the >> intermediate step of having this as optional. Otherwise it never gets tested. > > The argument that we don't have to take any responsibility for getting > things tested is a poor one. > > Anyway, you can always send a patch later to change the default, so why the > hurry to do it now? I think it should be the default so we smoke out bugs by throwing it at users during the -rc phase and the non-mq path should be the fallback. The -rc phase is for finding problems like this IMO. It is partly a personality trait I guess, I'm not very cautious in general, for good and for bad. >>> +module_param_named(use_blk_mq, mmc_use_blk_mq, bool, S_IWUSR | S_IRUGO); >> >> Are people really modprobing this so it needs to be a module parameter? > > Irrespective of modprobe, the parameter can be changed without re-compiling. > >> >> Maybe I'm the only developer stupid enough to just recompile and reboot > > Just change the parameter and unbind and rebind the host controller. Ah clever. I don't do such things, I guess I should try it out. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 September 2017 at 14:36, Adrian Hunter <adrian.hunter@intel.com> wrote: > Until mmc has blk-mq support fully implemented and tested, add a > parameter use_blk_mq, default to false unless config option MMC_MQ_DEFAULT > is selected. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Thanks, applied for next! Kind regards Uffe > --- > drivers/mmc/Kconfig | 11 +++++++++++ > drivers/mmc/core/core.c | 7 +++++++ > drivers/mmc/core/core.h | 2 ++ > drivers/mmc/core/host.c | 2 ++ > drivers/mmc/core/host.h | 4 ++++ > include/linux/mmc/host.h | 1 + > 6 files changed, 27 insertions(+) > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig > index ec21388311db..98202934bd29 100644 > --- a/drivers/mmc/Kconfig > +++ b/drivers/mmc/Kconfig > @@ -12,6 +12,17 @@ menuconfig MMC > If you want MMC/SD/SDIO support, you should say Y here and > also to your specific host controller driver. > > +config MMC_MQ_DEFAULT > + bool "MMC: use blk-mq I/O path by default" > + depends on MMC && BLOCK > + ---help--- > + This option enables the new blk-mq based I/O path for MMC block > + devices by default. With the option the mmc_core.use_blk_mq > + module/boot option defaults to Y, without it to N, but it can > + still be overridden either way. > + > + If unsure say N. > + > if MMC > > source "drivers/mmc/core/Kconfig" > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 2ff614d4ffac..10d7101164e4 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -66,6 +66,13 @@ > bool use_spi_crc = 1; > module_param(use_spi_crc, bool, 0); > > +#ifdef CONFIG_MMC_MQ_DEFAULT > +bool mmc_use_blk_mq = true; > +#else > +bool mmc_use_blk_mq = false; > +#endif > +module_param_named(use_blk_mq, mmc_use_blk_mq, bool, S_IWUSR | S_IRUGO); > + > static int mmc_schedule_delayed_work(struct delayed_work *work, > unsigned long delay) > { > diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h > index ba5a8fea0dc2..159b2ca301ec 100644 > --- a/drivers/mmc/core/core.h > +++ b/drivers/mmc/core/core.h > @@ -35,6 +35,8 @@ struct mmc_bus_ops { > int (*reset)(struct mmc_host *); > }; > > +extern bool mmc_use_blk_mq; > + > void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); > void mmc_detach_bus(struct mmc_host *host); > > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c > index ad88deb2e8f3..b624dbb6cd15 100644 > --- a/drivers/mmc/core/host.c > +++ b/drivers/mmc/core/host.c > @@ -398,6 +398,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) > host->max_blk_size = 512; > host->max_blk_count = PAGE_SIZE / 512; > > + host->use_blk_mq = mmc_use_blk_mq; > + > return host; > } > > diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h > index 77d6f60d1bf9..170fe5947087 100644 > --- a/drivers/mmc/core/host.h > +++ b/drivers/mmc/core/host.h > @@ -69,6 +69,10 @@ static inline bool mmc_card_hs400es(struct mmc_card *card) > return card->host->ios.enhanced_strobe; > } > > +static inline bool mmc_host_use_blk_mq(struct mmc_host *host) > +{ > + return host->use_blk_mq; > +} > > #endif > > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index c296f4351c1d..9109265fe529 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -378,6 +378,7 @@ struct mmc_host { > unsigned int doing_retune:1; /* re-tuning in progress */ > unsigned int retune_now:1; /* do re-tuning at next req */ > unsigned int retune_paused:1; /* re-tuning is temporarily disabled */ > + unsigned int use_blk_mq:1; /* use blk-mq */ > > int rescan_disable; /* disable card detection */ > int rescan_entered; /* used with nonremovable devices */ > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index ec21388311db..98202934bd29 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -12,6 +12,17 @@ menuconfig MMC If you want MMC/SD/SDIO support, you should say Y here and also to your specific host controller driver. +config MMC_MQ_DEFAULT + bool "MMC: use blk-mq I/O path by default" + depends on MMC && BLOCK + ---help--- + This option enables the new blk-mq based I/O path for MMC block + devices by default. With the option the mmc_core.use_blk_mq + module/boot option defaults to Y, without it to N, but it can + still be overridden either way. + + If unsure say N. + if MMC source "drivers/mmc/core/Kconfig" diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 2ff614d4ffac..10d7101164e4 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -66,6 +66,13 @@ bool use_spi_crc = 1; module_param(use_spi_crc, bool, 0); +#ifdef CONFIG_MMC_MQ_DEFAULT +bool mmc_use_blk_mq = true; +#else +bool mmc_use_blk_mq = false; +#endif +module_param_named(use_blk_mq, mmc_use_blk_mq, bool, S_IWUSR | S_IRUGO); + static int mmc_schedule_delayed_work(struct delayed_work *work, unsigned long delay) { diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index ba5a8fea0dc2..159b2ca301ec 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -35,6 +35,8 @@ struct mmc_bus_ops { int (*reset)(struct mmc_host *); }; +extern bool mmc_use_blk_mq; + void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops); void mmc_detach_bus(struct mmc_host *host); diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index ad88deb2e8f3..b624dbb6cd15 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -398,6 +398,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) host->max_blk_size = 512; host->max_blk_count = PAGE_SIZE / 512; + host->use_blk_mq = mmc_use_blk_mq; + return host; } diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h index 77d6f60d1bf9..170fe5947087 100644 --- a/drivers/mmc/core/host.h +++ b/drivers/mmc/core/host.h @@ -69,6 +69,10 @@ static inline bool mmc_card_hs400es(struct mmc_card *card) return card->host->ios.enhanced_strobe; } +static inline bool mmc_host_use_blk_mq(struct mmc_host *host) +{ + return host->use_blk_mq; +} #endif diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index c296f4351c1d..9109265fe529 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -378,6 +378,7 @@ struct mmc_host { unsigned int doing_retune:1; /* re-tuning in progress */ unsigned int retune_now:1; /* do re-tuning at next req */ unsigned int retune_paused:1; /* re-tuning is temporarily disabled */ + unsigned int use_blk_mq:1; /* use blk-mq */ int rescan_disable; /* disable card detection */ int rescan_entered; /* used with nonremovable devices */
Until mmc has blk-mq support fully implemented and tested, add a parameter use_blk_mq, default to false unless config option MMC_MQ_DEFAULT is selected. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/Kconfig | 11 +++++++++++ drivers/mmc/core/core.c | 7 +++++++ drivers/mmc/core/core.h | 2 ++ drivers/mmc/core/host.c | 2 ++ drivers/mmc/core/host.h | 4 ++++ include/linux/mmc/host.h | 1 + 6 files changed, 27 insertions(+)