2012-02-28 17:11:49

by Oleg Nesterov

Subject: Re: + procfs-mark-thread-stack-correctly-in-proc-pid-maps.patch added to -mm tree

> +int vm_is_stack(struct task_struct *task,
> + struct vm_area_struct *vma, int in_group)
> +{
> + if (vm_is_stack_for_task(task, vma))
> + return 1;
> +
> + if (in_group) {
> + struct task_struct *t = task;
> + while_each_thread(task, t) {
> + if (vm_is_stack_for_task(t, vma))
> + return 1;
> + }
> + }
> +
> + return 0;
> +}

This is obviously wrong, while_each_thread() is not safe without
tasklist or siglock or rcu.

Oleg.

2012-02-28 17:18:23

by Siddhesh Poyarekar

Subject: Re: + procfs-mark-thread-stack-correctly-in-proc-pid-maps.patch added to -mm tree

On Tue, Feb 28, 2012 at 10:34 PM, Oleg Nesterov <[email protected]> wrote:
>> +int vm_is_stack(struct task_struct *task,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? struct vm_area_struct *vma, int in_group)
>> +{
>> + ? ? if (vm_is_stack_for_task(task, vma))
>> + ? ? ? ? ? ? return 1;
>> +
>> + ? ? if (in_group) {
>> + ? ? ? ? ? ? struct task_struct *t = task;
>> + ? ? ? ? ? ? while_each_thread(task, t) {
>> + ? ? ? ? ? ? ? ? ? ? if (vm_is_stack_for_task(t, vma))
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? return 1;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>
> This is obviously wrong, while_each_thread() is not safe without
> tasklist or siglock or rcu.

I have fixed this in my git stash. I'll submit once I get to work on
Mike Frysinger's idea of marking stacks with their tids and see if it
breaks anything.

--
Siddhesh Poyarekar
http://siddhesh.in

2012-02-28 17:47:53

by Oleg Nesterov

Subject: Re: + procfs-mark-thread-stack-correctly-in-proc-pid-maps.patch added to -mm tree

On 02/28, Siddhesh Poyarekar wrote:
>
> On Tue, Feb 28, 2012 at 10:34 PM, Oleg Nesterov <[email protected]> wrote:
> >> +int vm_is_stack(struct task_struct *task,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? struct vm_area_struct *vma, int in_group)
> >> +{
> >> + ? ? if (vm_is_stack_for_task(task, vma))
> >> + ? ? ? ? ? ? return 1;
> >> +
> >> + ? ? if (in_group) {
> >> + ? ? ? ? ? ? struct task_struct *t = task;
> >> + ? ? ? ? ? ? while_each_thread(task, t) {
> >> + ? ? ? ? ? ? ? ? ? ? if (vm_is_stack_for_task(t, vma))
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? return 1;
> >> + ? ? ? ? ? ? }
> >> + ? ? }
> >> +
> >> + ? ? return 0;
> >> +}
> >
> > This is obviously wrong, while_each_thread() is not safe without
> > tasklist or siglock or rcu.
>
> I have fixed this in my git stash. I'll submit

OK, please CC me ;)

Oleg.