Messages in this thread | | | Date | Wed, 18 May 2011 10:17:36 +0800 | Subject | Re: [PATCH] sched: fix priority leakage in pick_next_highest_task_rt() | From | Yong Zhang <> |
| |
On Wed, May 18, 2011 at 9:14 AM, Steven Rostedt <rostedt@goodmis.org> wrote: > 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.
Yup, I misread the patch at the first time.
Now I think Hillf's patch is correct.
Thanks for your explanation Steven.
Thanks, Yong
> > 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 > >
-- Only stand for myself -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |