[lkml]   [2008]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: fork_idle && pid problems ?
    Oleg Nesterov wrote:
    > On 04/17, Ingo Molnar wrote:
    >> * Oleg Nesterov <> wrote:
    >>>> 1331 if (likely(p->pid)) {
    >>>> 1351 }
    >>>> Event leaked_storage: Returned without freeing storage "pid" Also
    >>>> see events: [alloc_fn][var_assign][pass_arg]
    >>> this looks like a false alarm.
    >>> p->pid == pid->numbers[0].nr. If "struct pid *pid" was allocated, its
    >>> .nr can't be 0.
    >>> IOW, !p->pid means that pid == init_struct_pid, it wasn't allocated
    >>> but was passed from the caller.
    >> should we perhaps codify this rule via adding something like this to the
    >> else branch:
    >> WARN_ON_ONCE(task_pid(p) != &init_struct_pid);
    >> ?
    > Perhaps yes, I don't know...
    > But please note that we heavily rely on the fact that nobody except idle
    > threads can have pid_nr == 0, and more importantly, each "struct pid" must
    > have the unique .nr withing the same namespace (init_pid_ns in this case).
    > I'd suggest to just add a small comment.
    > But wait... What _is_ the task_pid() after fork_idle() ???

    It is NULL, but every code getting one can handle such case :)

    > fork_idle() doesn't really attach the new thread to the init_struct_pid,
    > so ->pids[PIDTYPE_PID].pid just points the parent's pid, no?
    > As for x86, the parent is /sbin/init (kernel_init->smp_prepare_cpus),
    > not so bad, it can't exit.
    > But what about HOTPLUG_CPU? Suppose we add CPU, use some non-idle
    > kernel thread (workqueue) to fork the idle thread. CPU goes down,
    > parent exits and frees the pid. Now, if this CPU goes up again, the
    > idle thread runs with its ->pid pointing to the freed memory, not
    > good.

    Nope - it will be NULL.

    > Not serious perhaps, afaics we only need this ->pid to ensure that
    > swapper can safely fork /sbin/init, but still.
    > Pavel, Eric, Sukadev? Please say I missed something! ;)
    > Otherwise, we can change init_idle() to do attach_pid(init_struct_pid),
    > afaics we can do this lockless. In that case we should also change
    > INIT_STRUCT_PID() and remove the initialization of .tasks.

    Well, these was some request to make tasks always have pid link
    point to not NULL (from Matt?) so we'll need this :)

    > Oleg.


     \ /
      Last update: 2008-04-17 18:21    [W:0.044 / U:14.260 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site