On Sun, 16 Feb 2003, Martin J. Bligh wrote:
>
> Right, that's what I expected, and that may well explain the hang, but I
> don't see how that explains the oops still happening. What exactly is
> tasklist_lock protecting here anyway?
>
> read_lock(&tasklist_lock);
> buffer += sprintf(buffer,
> "State:\t%s\n"
> "Tgid:\t%d\n"
> "Pid:\t%d\n"
> "PPid:\t%d\n"
> "TracerPid:\t%d\n"
> "Uid:\t%d\t%d\t%d\t%d\n"
> "Gid:\t%d\t%d\t%d\t%d\n",
> get_task_state(p), p->tgid,
> p->pid, p->pid ? p->real_parent->pid : 0,
> p->pid && p->ptrace ? p->parent->pid : 0,
> p->uid, p->euid, p->suid, p->fsuid,
> p->gid, p->egid, p->sgid, p->fsgid);
> read_unlock(&tasklist_lock);
>
> Is it these two accesses:
>
> p->real_parent->pid ?
> p->parent->pid ?
Yup.
> Don't see what I can do for this apart from to invert the ordering and take
> tasklist_lock around the whole function, and nest task_lock inside that, or
> I suppose I could take the task_lock for each of the parents? I seem to
> recall Linus reminding people recently that it was only the lock
> acquisition order that was important, not release ... does something like
> the following look OK?
This patch looks like it should certainly fix the problem, but that is
still some god-awful ugly overkill in locking.
I'd rather make the rule be that you have to take the task lock before
modifying things like the parent pointers (and all the other fundamntal
pointers), since that's already the rule for most of it anyway.
And then the tasklist lock would go away _entirely_ from /proc (except for
task lookup in ->readdir/->lookup, of course, where it is fundamentally
needed and proper - and will probably some day be replaced by RCU, I
suspect).
Linus
-
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/
This archive was generated by hypermail 2b29 : Sun Feb 23 2003 - 22:00:15 EST