Re: [PATCH RFC 22/39] mmc: block: Prepare CQE data

Re: [PATCH RFC 22/39] mmc: block: Prepare CQE data

[Linus Walleij]

On Fri, Mar 3, 2017 at 1:22 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 15/02/17 15:49, Linus Walleij wrote:
>> On Fri, Feb 10, 2017 at 1:55 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>>> Enhance mmc_blk_data_prep() to support CQE requests.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>
>> Hey:
>>
>>> +#include <linux/ioprio.h>
>> (...)
>>> +       if (IOPRIO_PRIO_CLASS(req_get_ioprio(req)) == IOPRIO_CLASS_RT)
>>> +               brq->data.flags |= MMC_DATA_PRIO;
>>
>> What is this?
>
> It is the command queue priority.
>
> The command queue supports 2 priorities: "high" (1) and "simple" (0).  The
> eMMC will give "high" priority tasks priority over "simple" priority tasks.
>
> So here we give priority to IOPRIO_CLASS_RT which seems appropriate.

So if I understand correctly, you are obtaining the block layer scheduling
priorities, that can (only?) be set by ionice has from the command line?

We need to discuss this with the block maintainers.

I'm not so sure about the future of this. The IOPRIO is only used with the CFQ
scheduler, only two other sites in the kernel use this and MQ and its schedulers
surely does not have ionice handling as far as I know.

The BFQ does not use it, AFAIK it is using different heuristics to prioritize
block traffic, and that does not include using ionice hints.

Is ionice:ing something we're really going to do going forward?
Should this be repurposed so that the block scheduler use this prio to
communicate to the driver layer to prioritize certain traffic?

Yours,
Linus Walleij

Re: [PATCH RFC 22/39] mmc: block: Prepare CQE data

[Adrian Hunter]

On 10/03/17 00:39, Linus Walleij wrote:
> On Fri, Mar 3, 2017 at 1:22 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 15/02/17 15:49, Linus Walleij wrote:
>>> On Fri, Feb 10, 2017 at 1:55 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>
>>>> Enhance mmc_blk_data_prep() to support CQE requests.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>
>>> Hey:
>>>
>>>> +#include <linux/ioprio.h>
>>> (...)
>>>> +       if (IOPRIO_PRIO_CLASS(req_get_ioprio(req)) == IOPRIO_CLASS_RT)
>>>> +               brq->data.flags |= MMC_DATA_PRIO;
>>>
>>> What is this?
>>
>> It is the command queue priority.
>>
>> The command queue supports 2 priorities: "high" (1) and "simple" (0).  The
>> eMMC will give "high" priority tasks priority over "simple" priority tasks.
>>
>> So here we give priority to IOPRIO_CLASS_RT which seems appropriate.
> 
> So if I understand correctly, you are obtaining the block layer scheduling
> priorities, that can (only?) be set by ionice has from the command line?

AFAICS it is the ioprio_set() system call .

> 
> We need to discuss this with the block maintainers.
> 
> I'm not so sure about the future of this. The IOPRIO is only used with the CFQ
> scheduler, only two other sites in the kernel use this and MQ and its schedulers
> surely does not have ionice handling as far as I know.
> 
> The BFQ does not use it, AFAIK it is using different heuristics to prioritize
> block traffic, and that does not include using ionice hints.
> 
> Is ionice:ing something we're really going to do going forward?
> Should this be repurposed so that the block scheduler use this prio to
> communicate to the driver layer to prioritize certain traffic?

That seems like a separate issue.  At the moment, I/O priorities are what we
have, and giving priority to RT seems appropriate.

Re: [PATCH RFC 22/39] mmc: block: Prepare CQE data

[Linus Walleij]

On Fri, Mar 10, 2017 at 9:29 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 10/03/17 00:39, Linus Walleij wrote:
>> On Fri, Mar 3, 2017 at 1:22 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 15/02/17 15:49, Linus Walleij wrote:
>>>> On Fri, Feb 10, 2017 at 1:55 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>
>>>>> Enhance mmc_blk_data_prep() to support CQE requests.
>>>>>
>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>
>>>> Hey:
>>>>
>>>>> +#include <linux/ioprio.h>
>>>> (...)
>>>>> +       if (IOPRIO_PRIO_CLASS(req_get_ioprio(req)) == IOPRIO_CLASS_RT)
>>>>> +               brq->data.flags |= MMC_DATA_PRIO;
>>>>
>>>> What is this?
>>>
>>> It is the command queue priority.
>>>
>>> The command queue supports 2 priorities: "high" (1) and "simple" (0).  The
>>> eMMC will give "high" priority tasks priority over "simple" priority tasks.
>>>
>>> So here we give priority to IOPRIO_CLASS_RT which seems appropriate.
>>
>> So if I understand correctly, you are obtaining the block layer scheduling
>> priorities, that can (only?) be set by ionice has from the command line?
>
> AFAICS it is the ioprio_set() system call .

OK to be clear, correct, it is set by that system call.

And as far as I can tell there is exactly one program in the world that issues
that syscall, and that is ionice(1).

It is described to the world as a way for users to give hints to the
I/O scheduler
about how to manage requests. For example a user can hint that some IO
is a backup job and not important and some IO is a media player and really
important.

And since the block layer only handles this hint in the legacy blk CFQ scheduler
I want to know from the block layer developers if this priority has any future.
BFQ does not use this hint at all, I think Paolo described the ionice command
as problematic, and we need to understand if user-provided IO scheduling hints
is something that is going to be used in the future.

Yours,
Linus Walleij