Re: [PATCH 4/8] bfq: Limit number of requests consumed by each cgroup

Michal Koutný
2021-11-02T18:17:02Z

Hello.

On Wed, Oct 06, 2021 at 07:31:43PM +0200, Jan Kara <jack@suse.cz> wrote:
> + for (level--; level >= 0; level--) {
> + entity = entities[level];
> + if (level > 0) {
> + wsum = bfq_entity_service_tree(entity)->wsum;
> + } else {
> + int i;
> + /*
> + * For bfqq itself we take into account service trees
> + * of all higher priority classes and multiply their
> + * weights so that low prio queue from higher class
> + * gets more requests than high prio queue from lower
> + * class.
> + */
> + wsum = 0;
> + for (i = 0; i <= class_idx; i++) {
> + wsum = wsum * IOPRIO_BE_NR +
> + sched_data->service_tree[i].wsum;
> + }
> + }
> + limit = DIV_ROUND_CLOSEST(limit * entity->weight, wsum);

This scheme caught my eye. You mutliply (tree) weights by a factor
depending on the class when counting the wsum but then you don't apply
the same scaling for the evaluated entity in the numerator.

IOW, I think there should be something like
scale = (level > 0) ? 1 : int_pow(IOPRIO_BE_NR, BFQ_IOPRIO_CLASSES - bfq_class_idx(entity));
limit = DIV_ROUND_CLOSEST(limit * entity->weight * scale, wsum);

For instance, if there are two cgroups (level=1) with weights 100 and
200, and each cgroup has a single IOPRIO_CLASS_BE entity (level=0) in
it, the `limit` distribution would honor the ratio of weights from
level=1 (100:200) but it would artificially lower the absolute amount of
allowed tags. If I am not mistaken, that would be reduced by factor
1/BFQ_IOPRIO_CLASSES.

Also if I consider it more broadly, is this supposed to match/extend
bfq_io_prio_to_weight() calculation?

Michal