On Wed, Apr 13, 2016 at 03:28:03PM +0200, Peter Zijlstra wrote:
> On Mon, Apr 11, 2016 at 07:31:57PM +0300, Michael S. Tsirkin wrote:
> > +static bool expected_to_run_fair(struct cfs_rq *cfs_rq, s64 t)
> > +{
> > + struct sched_entity *left;
> > + struct sched_entity *curr = cfs_rq->curr;
> > +
> > + if (!curr || !curr->on_rq)
> > + return false;
> > +
> > + left = __pick_first_entity(cfs_rq);
> > + if (!left)
> > + return true;
> > +
> > + return (s64)(curr->vruntime + calc_delta_fair(t, curr) -
> > + left->vruntime) < 0;
> > +}
> >
> > The reason it seems easier is because that way we can reuse
> > calc_delta_fair and don't have to do the reverse translation
> > from vruntime to nsec.
> >
> > And I guess if we do this with interrupts disabled, and only poke
> > at the current CPU's rq, we know first entity
> > won't go away so we don't need locks?
>
> Nope, not true. Current isn't actually in the tree, and any other task
> is subject to being moved at any time.
> Even if current was in the tree, there is no guarantee it is
> ->rb_leftmost; imagine a task being migrated in that has a smaller
> vruntime.
>
> So this really cannot work without locks :/
>
> I've not thought about the actual problem you're trying to solve; but I
> figured I'd let you know this before you continue down this path.
Hmm. This is in fact called in the context of a given task,
so maybe I should just change the API and use
&task->se
instead of
cfs_rq->current
:
static bool expected_to_run_fair(struct cfs_rq *cfs_rq, struct task_struct
*task, s64 t)
{
struct sched_entity *left;
struct sched_entity *curr = &task->se;
if (!curr || !curr->on_rq)
return false;
left = __pick_first_entity(cfs_rq);
if (!left)
return true;
return (s64)(curr->vruntime + calc_delta_fair(t, curr) -
left->vruntime) < 0;
}
This way it is caller's respinsibility to make sure task
is not going away.
Left here is on tree so it's not going away, right?
--
MST
