Messages in this thread | | | From | Paul Turner <> | Date | Tue, 1 Mar 2011 23:08:05 -0800 | Subject | Re: [PATCH] sched: next buddy hint on sleep and preempt path |
| |
On Tue, Mar 1, 2011 at 10:47 PM, Mike Galbraith <efault@gmx.de> wrote: > On Tue, 2011-03-01 at 21:43 -0800, Paul Turner wrote: >> On Tue, Mar 1, 2011 at 3:33 PM, Venkatesh Pallipadi <venki@google.com> wrote: > >> > for_each_sched_entity(se) { >> > cfs_rq = cfs_rq_of(se); >> > dequeue_entity(cfs_rq, se, flags); >> > >> > /* Don't dequeue parent if it has other entities besides us */ >> > - if (cfs_rq->load.weight) >> > + if (cfs_rq->load.weight) { >> > + /* >> > + * Bias pick_next to pick a task from this cfs_rq, as >> > + * p is sleeping when it is within its sched_slice. >> > + */ >> > + if (task_flags & DEQUEUE_SLEEP && se->parent) >> > + set_next_buddy(se->parent); >> >> re-using the last_buddy would seem like a more natural fit here; also >> doesn't have a clobber race with a wakeup > > Hm, that would break last_buddy no? A preempted task won't get the CPU > back after light preempting thread deactivates. (it's disabled atm > unless heavily overloaded anyway, but..)
Ommm yeah.. we're actually a little snookered in this case since the pre-empting thread's sleep will be voluntary which will try to return time to its hierarchy.
I suppose keeping the last_buddy is preferable to the occasional clobber.
> > This wants a tweak either way though. > > static inline struct task_struct *task_of(struct sched_entity *se) > { > #ifdef CONFIG_SCHED_DEBUG > WARN_ON_ONCE(!entity_is_task(se)); > #endif > return container_of(se, struct task_struct, se); > } > > static void set_next_buddy(struct sched_entity *se) > { > if (likely(task_of(se)->policy != SCHED_IDLE)) { > for_each_sched_entity(se) > cfs_rq_of(se)->next = se; > } > } >
yeah good catch, the if's do need to be broken out so that setting buddies it works properly with non-task entities
> -- 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/
| |