[lkml]   [2007]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] task_pid_nr_ns() breaks proc_pid_readdir()
    Oleg Nesterov <> writes:

    > On 11/17, Eric W. Biederman wrote:
    >> Oleg Nesterov <> writes:
    >> > Make sure that task_pid_nr_ns() returns !0 before updating tgid. Note that
    >> > next_tgid(tgid + 1) can find the same "struct pid" again, but we shouldn't
    >> > go into the endless loop because pid_task(PIDTYPE_PID) must return NULL in
    >> > this case, so next_tgid() can't return the same task.
    >> >
    >> Oleg I think I would rather update next_tgid to return the tgid (which
    >> removes the need to call task_pid_nr_ns). This keeps all of the task
    >> iteration logic together in next_tgid.
    > Yes sure, I think your patch is also correct, please use it.
    > <offtopic>
    > Personally, I hate the functions which use pointers to return another value.
    > (yes, yes, I know, my taste is perverted). Why don't we return structure in
    > this case? We can even make a common helper struct, say,
    > Of course, another option is to rewrite the kernle in perl, in that case
    > proc_pid_readdir() can just do
    > (task, tgid) = next_tgid();

    I wish that last syntax worked in C. That would make returning
    structures so much easier. From a compiler optimization standpoint
    returning structures is ever so much nicer. Seeing through pointers
    or references to optimize things is tricky.

    > </offtopic>
    >> Although looking at this in more detail, I'm half wondering if
    >> proc_pid_make_inode() should take a struct pid instead of a task.
    > Yes, I also thought about this. Needs more changes, and still not perfect.
    > I am starting to think we need a more generic change. How about the patch
    > below? With this change the stable task_struct implies we have the stable
    > pids, this allows us to do a lot of cleanups.

    I don't see the huge pile of opportunities to clean up. But otherwise
    I am in favor of it. In the normal case it should only delay freeing
    of struct pid (and the corresponding namespace) by an rcu interval so it
    isn't a big deal.

    I suspect it will be a help with killing things like the global pids
    in task_struct.

    Regardless I'm going to pass on the keeping struct pid on the task
    struct until we free it and reference counting it there for the
    immediate present as I think we can solve the immediate issues cleanly
    without it, and we are pretty much in bug fixing territory now.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-11-19 19:19    [W:0.024 / U:7.312 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site