Re: [PATCH] sched: fix priority leakage in pick_next_highest_task_rt()

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, May 17, 2011 at 10:53:22PM +0800, Hillf Danton wrote:
> On Tue, May 17, 2011 at 10:28 AM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> > On Mon, May 16, 2011 at 8:55 PM, Hillf Danton <dhillf@gmail.com> wrote:
> >> When picking the second highest RT task for a given runqueue, if no
> >> task found after scanning the queue of priority == idx, the next idx
> >> should also be checked even in case that next is already existing, or
> >> the window of priority leakage could be opened.
> >
> > I don't see what kind of problem you patch will fix.
> > And mind explaining how priority leakage could happen?
> >
> Hi Yong
> 
> If no task is found after scanning the list at array->queue + idx,
> what should we operate on next?
> And why is the list scanned?
> 

The patch looks correct.

The code looks like so:

	for_each_leaf_rt_rq(rt_rq, rq) {
		array = &rt_rq->active;
		idx = sched_find_first_bit(array->bitmap);
next_idx:
		if (idx >= MAX_RT_PRIO)
			continue;
		if (next && next->prio < idx)
			continue;
		list_for_each_entry(rt_se, array->queue + idx, run_list) {
			struct task_struct *p;

			if (!rt_entity_is_task(rt_se))
				continue;

			p = rt_task_of(rt_se);
			if (pick_rt_task(rq, p, cpu)) {
				next = p;
				break;
			}
		}
		if (!next) {
			idx = find_next_bit(array->bitmap, MAX_RT_PRIO, idx+1);
			goto next_idx;
		}
	}

What we are doing is looking for the next highest prio task that we can
migrate. When we find the next highest priority task that can migrate,
we pick it. But the issue comes with the cgroups. If we are looping
through the cgroups, and we pick a task in one cgroup, but when we check
the next cgroup, if it has a higher priority task, but that task can't
migrate, but the next one, also of higher priority, can, that "if (!next)"
wont catch it.

Although, I don't know the cgroup code very well, and I wonder what it
means to pull a task from a run queue onto another run queue that has
dropped in priority.

But, anyway, for the patch:

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve