Re: [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()

[Kayra Cizmeci <kayracizmeci@gmail.com>]

> Argh, so this is bandwidth. Task gets throttled, but before it gets back
> to userspace to actually dequeue, it gets hit by a wakeup again while it
> is still current?

> Hmm, no, in that case it can't actually be on throttled_tasks. So it did
> get dequeued, but it never quite scheduled out.

> I hate this bandwidth stuff :-)

Ask me...

> Anyway, this is different from the initial use-case, where a
> sched_entity would get dequeue and re-enqueued while walking the cgroup
> hierarchy, and thus had to appear like nothing happened in between.

> In this case we're fine to advance time.

> Anyway... the normal block/wakeup paths are >< close to also hitting
> this in the case where balancing drops the rq->lock after dequeue. The
> only reason ttwu can't tickle this is because it has that ->on_cpu
> stuff. However, we cannot use that on the unthrottle path :/

> Bah.

> Does this work?

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 5c98d8dfce5d..0edd8bfb6363 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8250,7 +8250,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  	weight = enqueue_hierarchy(p, flags);
>  	reweight_eevdf(cfs_rq, se, weight, false);
>  	place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> -	__enqueue_entity(cfs_rq, se);
> +
> +	/*
> +	 * This can actually happen with CFS_BANDWIDTH, in that case a task
> +	 * can get throttled, dequeued and then unthrottled, woken *before* it
> +	 * schedules out and does put_prev_task().
> +	 */
> +	if (cfs_rq->curr != se)
> +		__enqueue_entity(cfs_rq, se);
>  
>  	if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
>  		dl_server_start(&rq->fair_server);

This burns my brain. I *think* it should work. 
But at this point, I don't think me thinking
alone is enough on this subject. So,
I'll try to test this... I'll post
the results tommorow or today if I even could.

Thanks!

Kayra