On Tue, 2009-12-08 at 16:04 +0100, Oleg Nesterov wrote: > > > > > + /* > > > > > + * In theory spin_lock() doesn't imply rcu_read_lock(). > > > > > + * Once we clear ->utrace_flags this task_struct can go away > > > > > + * because tracehook_prepare_release_task() path does not take > > > > > + * utrace->lock when ->utrace_flags == 0. > > > > > + */ > > > > > + rcu_read_lock(); > > > > > + task->utrace_flags = flags; > > > > > + spin_unlock(&utrace->lock); > > > > > + rcu_read_unlock(); > > > > > > > > yuck! > > > > > > > > why not simply keep a task reference over the utrace_reset call? > > > > > > Yes, we could use get_task_struct() instead. Not sure this would > > > be more clean, though. > > > > For one it would allow getting rid of that insane assymetric locking. > > Well, this is subjective, but I don't agree that > > get_task_struct(task); > task->utrace_flags = flags; > spin_unlock(&utrace->lock); > put_task_struct(task); > > looks better.
No, what I mean by assymetric locking is that utrace_reset() and utrace_reap() drop the utrace->lock where their caller acquired it, resulting in non-obvious like: utrace_control() { ... spin_lock(&utrace->lock); ... if (reset) utrace_reset(utrace); else spin_unlock(&utrace->lock); } If you take a task ref you can write the much saner: utrace_control() { ... spin_lock(&utrace->lock); ... if (reset) utrace_reset(utrace); spin_unlock(&utrace->lock); }