Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1740283 > unrolled thread

Re: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq Linus Walleij <linus.walleij@linaro.org> - 2017-09-27 01:50 +0200
    Re: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq Adrian Hunter <adrian.hunter@intel.com> - 2017-09-27 14:10 +0200
      Re: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq Linus Walleij <linus.walleij@linaro.org> - 2017-09-27 21:50 +0200
    RE: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq Avri Altman <Avri.Altman@wdc.com> - 2017-09-27 15:00 +0200

#1740283 — Re: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq

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

[toc] | [next] | [standalone]

#1740645

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.

[toc] | [prev] | [next] | [standalone]

#1740971

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

[toc] | [prev] | [next] | [standalone]

#1740696

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Linus Walleij
> Sent: Wednesday, September 27, 2017 2:42 AM
> To: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc <linux-
> mmc@vger.kernel.org>; linux-block <linux-block@vger.kernel.org>; linux-
> kernel <linux-kernel@vger.kernel.org>; Bough Chen <haibo.chen@nxp.com>;
> Alex Lemberg <Alex.Lemberg@wdc.com>; Mateusz Nowak
> <mateusz.nowak@intel.com>; Yuliy Izrailov <Yuliy.Izrailov@wdc.com>;
> Jaehoon Chung <jh80.chung@samsung.com>; Dong Aisheng
> <dongas86@gmail.com>; Das Asutosh <asutoshd@codeaurora.org>; Zhangfei
> Gao <zhangfei.gao@gmail.com>; Sahitya Tummala
> <stummala@codeaurora.org>; Harjani Ritesh <riteshh@codeaurora.org>;
> Venu Byravarasu <vbyravarasu@nvidia.com>; Shawn Lin <shawn.lin@rock-
> chips.com>; Christoph Hellwig <hch@lst.de>
> Subject: Re: [PATCH V9 09/15] mmc: core: Add parameter use_blk_mq
> 
> 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?


Module param can be changed in runtime

> 
> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web