Messages in this thread Patch in this message | | | Date | Mon, 10 Nov 2008 14:04:04 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH] account_group_exec_runtime: fix the racy usage of ->signal |
| |
On 11/08, Ingo Molnar wrote: > > * Oleg Nesterov <oleg@redhat.com> wrote: > > > On 11/07, Ingo Molnar wrote: > > > > > > the signal lock must not nest inside the rq > > > lock, and these accounting functions are called from within the > > > scheduler. > > > > Why? we seem to never do task_rq_lock() under ->siglock ? > > signal_wake_up() ?
I'd wish very much I could say I have already realized this, but I didn't. Thanks Ingo!
I don't see the good solution for this problem. I'll send the new patch in a minute, but it is ugly. Basically it is
--- a/kernel/exit.c +++ b/kernel/exit.c @@ -141,6 +141,8 @@ static void __exit_signal(struct task_st if (sig) { flush_sigqueue(&sig->shared_pending); taskstats_tgid_free(sig); + smp_mb(); + spin_unlock_wait(&task_rq(tsk)->lock); __cleanup_signal(sig); } }
except this needs a helper in sched.c. You can nack it right now ;) Of course we can protect ->signal with rcu, but this is even worse imho.
Anybody sees a bettter fix?
Perhaps we can change sched.c to do update_curr() only when the task is not running (except ->task_tick), iow perhaps we can check sleep/wakeup == T before calling update_cur(). But this is not easy even if really possible.
Oleg.
| |