Re: [PATCH 01/16] sched: add sched_class->task_dead.

[Juri Lelli]

On 04/08/2012 07:49 PM, Oleg Nesterov wrote:
> On 04/06, Juri Lelli wrote:
> > --- a/kernel/sched.c
> > +++ b/kernel/sched.c
> > @@ -3219,6 +3219,9 @@ static void finish_task_switch(struct rq *rq, struct task_struct *prev)
> >   	if (mm)
> >   		mmdrop_delayed(mm);
> >   	if (unlikely(prev_state == TASK_DEAD)) {
> > +		if (prev->sched_class->task_dead)
> > +			prev->sched_class->task_dead(prev);
> > +
>
> And 5/16 adds
>
> 	+static void task_dead_dl(struct task_struct *p)
> 	+{
> 	+       struct hrtimer *timer =&p->dl.dl_timer;
> 	+
> 	+	if (hrtimer_active(timer))
> 	+               hrtimer_try_to_cancel(timer);
> 	+}
>
> This looks suspicious. finish_task_switch() does put_task_struct()
> after that, it is quite possible this actually frees the memory.
>
> What if hrtimer_try_to_cancel() fails because the timer is running?
> In this case __run_hrtimer() can play with the freed timer. Say, to
> clear HRTIMER_STATE_CALLBACK. Not to mention dl_task_timer() itself.
>
> Oleg.

Right, hrtimer_cancel(timer) looks way better.

Thanks!

- Juri
--
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/