On 21/09/17 12:47, Ulf Hansson wrote:
> On 13 September 2017 at 13:40, Adrian Hunter <adrian.hunter@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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
>
> I asume the goal of adding this option is to enable us to move slowly
> forward. In general that might be a good idea, however for this
> particular case I am not sure.
>
> The main reason is simply that I find it unlikely that people and
> distributions will actually go in and change the default value, so in
> the end we will just be adding new code, which isn't really going to
> be much tested. That's what happened in scsi case.
The argument that no one is going to test anyway so we shouldn't give them
the opportunity, is not a sustainable vision for the future. Instead we
should reach out to relevant stakeholders and get them to do their testing
with blk-mq first.
>
> As I also stated earlier, I do worry about the maintenance of the mmc
> block device code, and this approach make it worse, at least short
> term.
What maintenance?
>
> To me, the scsi case is also different, because the mq support was
> added long time ago and at that point one could worry a bit of
> maturity of the blkmq in general, that I assume have been sorted out
> by know.
But blk-mq still isn't fully ready for all of scsi, so you seem to be
contradicting yourself.
>
>>
>> source "drivers/mmc/core/Kconfig"
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index ef2d8aa1e7d2..3638ed4f0f9e 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 e941342ed450..535539a9e7eb 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 54b0463443bd..5d1bd10991f7 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
>>
>
> Kind regards
> Uffe
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
