Messages in this thread | | | Date | Sun, 08 Apr 2012 20:09:33 +0200 | From | Juri Lelli <> | Subject | Re: [PATCH 01/16] sched: add sched_class->task_dead. |
| |
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
| |