linux-kernel - Re: [PATCH] blk-ioprio: Introduce promote-to-rt policy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Feb 2023 10:05:23 -0800
From:   Bart Van Assche <bvanassche@....org>
To:     Hou Tao <houtao@...weicloud.com>, linux-block@...r.kernel.org
Cc:     Jan Kara <jack@...e.cz>, Jens Axboe <axboe@...nel.dk>,
        cgroups@...r.kernel.org, Tejun Heo <tj@...nel.org>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Jonathan Corbet <corbet@....net>, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, houtao1@...wei.com
Subject: Re: [PATCH] blk-ioprio: Introduce promote-to-rt policy

On 2/2/23 03:09, Hou Tao wrote:
> Hi,
> 
> On 2/2/2023 1:33 AM, Bart Van Assche wrote:
>> On 1/31/23 20:52, Hou Tao wrote:
>>>    /**
>>>     * enum prio_policy - I/O priority class policy.
>>>     * @POLICY_NO_CHANGE: (default) do not modify the I/O priority class.
>>> @@ -27,21 +34,30 @@
>>>     * @POLICY_RESTRICT_TO_BE: modify IOPRIO_CLASS_NONE and IOPRIO_CLASS_RT into
>>>     *        IOPRIO_CLASS_BE.
>>>     * @POLICY_ALL_TO_IDLE: change the I/O priority class into IOPRIO_CLASS_IDLE.
>>> - *
>>> + * @POLICY_PROMOTE_TO_RT: modify IOPRIO_CLASS_NONE and IOPRIO_CLASS_BE into
>>> + *         IOPRIO_CLASS_RT.
>>>     * See also <linux/ioprio.h>.
>>>     */
>>>    enum prio_policy {
>>> -    POLICY_NO_CHANGE    = 0,
>>> -    POLICY_NONE_TO_RT    = 1,
>>> -    POLICY_RESTRICT_TO_BE    = 2,
>>> -    POLICY_ALL_TO_IDLE    = 3,
>>> +    POLICY_NO_CHANGE    = IOPRIO_CLASS_NONE,
>>> +    POLICY_NONE_TO_RT    = IOPRIO_CLASS_RT,
>>> +    POLICY_RESTRICT_TO_BE    = IOPRIO_CLASS_BE,
>>> +    POLICY_ALL_TO_IDLE    = IOPRIO_CLASS_IDLE,
>>> +    POLICY_PROMOTE_TO_RT    = IOPRIO_CLASS_RT | IOPRIO_POL_PROMOTION,
>>> +};
>>
>> The above change complicates the ioprio code. Additionally, I'm concerned that
>> it makes the ioprio code slower. Has it been considered to keep the numerical
>> values for the existing policies, to assign the number 4 to
>> POLICY_PROMOTE_TO_RT and to use a lookup-array in blkcg_set_ioprio() to
>> convert the policy number into an IOPRIO_CLASS value?
> For the slowness, do you meaning the extra dereference of blkcg->ioprio->policy
> when policy is no-change or the handle of IOPRIO_POL_PROMOTION in
> blkcg_set_ioprio()? It seems other functions (e.g., ioprio_show_prio_policy()
> and ioprio_set_prio_policy()) are not on the hot path. Using a lookup array in
> blkcg_set_ioprio() to do the conversion will also be OK, although it will
> introduce an extra lookup each time when policy is not no-change. I don't have
> strong preference. If you are OK with lookup array in blkcg_set_ioprio(), will
> do it in v2.

Hi Hou,

I prefer the lookup array because with the lookup array approach the 
IOPRIO_POL_PROMOTION constant is no longer needed and because I expect 
that this will result in code that is easier to read. Additionally, the 
lookup array will be small so the compiler may be clever enough to 
optimize it away.

Thanks,

Bart.