On Thu 07-04-22 16:07:38, Jan Kara wrote:
> People are occasionally reporting a warning bfqq_request_over_limit()
> triggering reporting that BFQ's idea of cgroup hierarchy (and its depth)
> does not match what generic blkcg code thinks. This can actually happen
> when bfqq gets moved between BFQ groups while bfqq_request_over_limit()
> is running. Make sure the code is safe against BFQ queue being moved to
> a different BFQ group.
>
> Fixes: 76f1df88bbc2 ("bfq: Limit number of requests consumed by each cgroup")
> CC: stable@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/all/CAJCQCtTw_2C7ZSz7as5Gvq=OmnDiio=HRkQekqWpKot84sQhFA@xxxxxxxxxxxxxx/
> Reported-by: Chris Murphy <lists@xxxxxxxxxxxxxxxxx>
> Reported-by: "yukuai (C)" <yukuai3@xxxxxxxxxx>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Ping guys? Can we get the fix in please? It is pretty trivial...
Honza
> ---
> block/bfq-iosched.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index e47c75f1fa0f..272d48d8f326 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -569,7 +569,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
> struct bfq_entity *entity = &bfqq->entity;
> struct bfq_entity *inline_entities[BFQ_LIMIT_INLINE_DEPTH];
> struct bfq_entity **entities = inline_entities;
> - int depth, level;
> + int depth, level, alloc_depth = BFQ_LIMIT_INLINE_DEPTH;
> int class_idx = bfqq->ioprio_class - 1;
> struct bfq_sched_data *sched_data;
> unsigned long wsum;
> @@ -578,15 +578,21 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
> if (!entity->on_st_or_in_serv)
> return false;
>
> +retry:
> + spin_lock_irq(&bfqd->lock);
> /* +1 for bfqq entity, root cgroup not included */
> depth = bfqg_to_blkg(bfqq_group(bfqq))->blkcg->css.cgroup->level + 1;
> - if (depth > BFQ_LIMIT_INLINE_DEPTH) {
> + if (depth > alloc_depth) {
> + spin_unlock_irq(&bfqd->lock);
> + if (entities != inline_entities)
> + kfree(entities);
> entities = kmalloc_array(depth, sizeof(*entities), GFP_NOIO);
> if (!entities)
> return false;
> + alloc_depth = depth;
> + goto retry;
> }
>
> - spin_lock_irq(&bfqd->lock);
> sched_data = entity->sched_data;
> /* Gather our ancestors as we need to traverse them in reverse order */
> level = 0;
> --
> 2.34.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
