Messages in this thread | | | Date | Thu, 1 Mar 2012 17:51:53 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/2] Take rcu read lock when iterating through thread group |
| |
On 03/01, Siddhesh Poyarekar wrote: > > Protect the iteration through thread group with rcu_read_lock when > looking for tasks in the group that use the current vma as > stack. Thanks KOSAKI Motohiro (kosaki.motohiro@gmail.com) for pointing > it out. > > Signed-off-by: Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com> > --- > mm/memory.c | 15 +++++++++++---- > 1 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index 601a920..a88b764 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3915,20 +3915,27 @@ void print_vma_addr(char *prefix, unsigned long ip) > * just check in the current task. > */ > int vm_is_stack(struct task_struct *task, > - struct vm_area_struct *vma, int in_group) > + struct vm_area_struct *vma, int in_group) > { > + int ret = 0; > + > if (vm_is_stack_for_task(task, vma)) > return 1; > > if (in_group) { > struct task_struct *t = task; > + rcu_read_lock(); > while_each_thread(task, t) {
This is the commont mistake. rcu_read_lock() can not help unless you verify that ->thread_group.next still points to the rcu-protected memory.
Just suppose that this task exits, then next_thread() exits too. Now you take rcu_read_lock() but it is too late, ->next points to nowhere.
Also. In fact while_each_thread() is not safe under rcu. We are going to fix this, but only for the case when while_each_thread() starts at the thread group leader.
Oleg.
| |