Mailing List Archive: Comment about proc-dont-lock-task

Comment about proc-dont-lock-task_structs-indefinitely.patch

Apr 10, 2006, 12:42 PM

Post #1 of 3 (133 views)

Hi,

In reply to http://marc.theaimsgroup.com/?l=linux-kernel&m=114119848908725&q=raw
I was not following and just noticed it. The bug is introduced in the patch
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.16/2.6.16-mm1/broken-out/proc-dont-lock-task_structs-indefinitely.patch

The task decrement problem is fixed, but I think we have two more
problems in the following patch segment.

The priv->tail_vma should not be set NULL; In old code, the local
variable tail vma was overloaded for two more purposes as return value
and also in version calculation, in addition to beging initialised
from gate vma. It we set the priv->tail_vma as NULL as the following
patch does, and if we seek back, we will not be able to see the gate
vma anymore from m_next.

@@ -337,35 +349,37 @@ static void *m_start(struct seq_file *m,
}

if (l != mm->map_count)
- tail_vma = NULL; /* After gate vma */
+ priv->tail_vma = NULL; /* After gate vma */

out:
if (vma)
return vma;

/* End of vmas has been reached */
- m->version = (tail_vma != NULL)? 0: -1UL;
+ m->version = (priv->tail_vma != NULL)? 0: -1UL;
up_read(&mm->mmap_sem);
mmput(mm);
- return tail_vma;
+ return priv->tail_vma;

Thanks,
Prasanna.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: Comment about proc-dont-lock-task_structs-indefinitely.patch [ In reply to ]

mlp at google

Apr 11, 2006, 3:48 AM

Post #2 of 3 (127 views)

Permalink

On 4/11/06, Prasanna Meda <mlp@google.com> wrote:

>
> The task decrement problem is fixed, but I think we have two more
> problems in the following patch segment.
>

I think you agreed with the first problem. And the second problem is,
show_map_internal is still treating m->private as task_struct instead
of proc_maps_private.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Re: Comment about proc-dont-lock-task_structs-indefinitely.patch [ In reply to ]

ebiederm at xmission

Apr 11, 2006, 4:28 AM

Post #3 of 3 (128 views)

Permalink

"Prasanna Meda" <mlp@google.com> writes:

> On 4/11/06, Prasanna Meda <mlp@google.com> wrote:
>
>>
>> The task decrement problem is fixed, but I think we have two more
>> problems in the following patch segment.
>>
>
> I think you agreed with the first problem. And the second problem is,
> show_map_internal is still treating m->private as task_struct instead
> of proc_maps_private.

Sorry my brain has been off thinking about a subtle
bug accidentally introduced in 2.6.17-rc1.

You are absolutely right. Somehow I missed the
fact that show_map_internal was using m->private.
Because get_gate_vma doesn't actually use it's argument
no bad behavior will result but that could change.

As for the seek case you may be right.
I have a cold that is beating on me, and I need to take a nap.

I remember looking at that closely and not seeing a problem,
but I have made mistakes before, and I'm not certain I recall
the seek case.

Eric

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/