On 6/9/21 11:20 PM, Hannes Reinecke wrote:
> On 6/9/21 1:06 AM, Bart Van Assche wrote:
>> +static void blkcg_ioprio_track(struct rq_qos *rqos, struct request *rq,
>> + struct bio *bio)
>> +{
>> + struct ioprio_blkcg *blkcg = ioprio_blkcg_from_bio(bio);
>> +
>> + /*
>> + * Except for IOPRIO_CLASS_NONE, higher I/O priority numbers
>> + * correspond to a lower priority. Hence, the max_t() below selects
>> + * the lower priority of bi_ioprio and the cgroup I/O priority class.
>> + * If the cgroup priority has been set to IOPRIO_CLASS_NONE == 0, the
>> + * bio I/O priority is not modified. If the bio I/O priority equals
>> + * IOPRIO_CLASS_NONE, the cgroup I/O priority is assigned to the bio.
>> + */
>> + bio->bi_ioprio = max_t(u16, bio->bi_ioprio,
>> + IOPRIO_PRIO_VALUE(blkcg->prio_class, 0));
>> +}
>
> Sheesh. Now that is cheeky.
> First defining a (conceptually) complex policy setting (where people
> wonder where these policies came from), which then devolve into a simple
> max() setting of the priority value.
> This _really_ could do with a better explanation in the documentation,
> as then it's far easier to understand _why_ certain policies override
> others.
> IE this comment belongs in the documentation, as explanation of the
> underlying mechanics of the ioprio policies.
Hi Hannes,
blkcg_ioprio_track() is called from the hot path so I want this function
to be fast. Since the desired behavior can be implemented with a max() I
chose max() instead of e.g. using a lookup table.
I will address your comments about the documentation and the code.
Bart.
