Re: [patch, 2.6.10-rc2] sched: fix ->nr_uninterruptible handlingbugs

[Nick Piggin]

Peter Williams wrote:
> Nick Piggin wrote:
>
> > Ingo Molnar wrote:
> >
> > > PREEMPT_RT on SMP systems triggered weird (very high) load average
> > > values rather easily, which turned out to be a mainline kernel
> > > ->nr_uninterruptible handling bug in try_to_wake_up().
> > >
> > > the following code:
> > >
> > >         if (old_state == TASK_UNINTERRUPTIBLE) {
> > >                 old_rq->nr_uninterruptible--;
> > >
> > > potentially executes with old_rq potentially being != rq, and hence
> > > updating ->nr_uninterruptible without the lock held. Given a
> > > sufficiently concurrent preemption workload the count can get out of
> > > whack and updates might get lost, permanently skewing the global 
> > > count. Nothing except the load-average uses nr_uninterruptible() so this
> > > condition can go unnoticed quite easily.
> >
> > Hi Ingo,
> > Yes you're right.
> >
> > I have another idea. Revert back to the old code, then just transfer
> > the nr_uninterruptible count when migrating a task. That way, the
>
>
> I presume that you mean adjust rather than transfer.
>
> > rq's nr_uninterruptible field always is a measure of the number of
> > uninterruptible tasks on it. What do you think?
>
>
> To make this work you need to do the adjustment every where that a task 
> changes CPU while in the UNINTERRUPTIBLE state.  Are both run queue 
> locks always held in these circumstances?  I don't think that they are 
> in try_to_wake_up() but it may be possible to work around that.

Yeah this won't actually work of course, because a task can set itself
UNINTERRUPTIBLE and subsequently get preempted then moved CPUs before
calling schedule() itself.

And yeah I missed the original point of your fix which was due to the
task moving runqueues in try_to_wake_up. Sorry, forget about the patch :P
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/