On 11/17, Eric W. Biederman wrote:
>
> Oleg Nesterov <[email protected]> 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,
struct pair {
union {
long val1;
void *ptr1;
};
union {
long val2;
void *ptr2;
};
};
#define PAIR(x1, x2) (struct pair){{ . x1 }, { . x2 }}
Now, next_tgid() can do
return PAIR(ptr1 = task, val2 = tgid);
With -freg-struct-return the generated code is nice.
Of course, another option is to rewrite the kernle in perl, in that case
proc_pid_readdir() can just do
(task, tgid) = next_tgid();
</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.
Oleg.
--- kernel/pid.c 2007-10-25 16:22:12.000000000 +0400
+++ - 2007-11-18 16:56:30.682555454 +0300
@@ -323,7 +323,7 @@ int fastcall attach_pid(struct task_stru
struct pid_link *link;
link = &task->pids[type];
- link->pid = pid;
+ link->pid = get_pid(pid);
hlist_add_head_rcu(&link->node, &pid->tasks[type]);
return 0;
@@ -339,7 +339,6 @@ void fastcall detach_pid(struct task_str
pid = link->pid;
hlist_del_rcu(&link->node);
- link->pid = NULL;
for (tmp = PIDTYPE_MAX; --tmp >= 0; )
if (!hlist_empty(&pid->tasks[tmp]))
@@ -348,6 +347,14 @@ void fastcall detach_pid(struct task_str
free_pid(pid);
}
+void task_put_pids(struct pid_link *pids)
+{
+ int type = PIDTYPE_MAX;
+
+ while (type--)
+ put_pid(pids[type].pid);
+}
+
/* transfer_pid is an optimization of attach_pid(new), detach_pid(old) */
void fastcall transfer_pid(struct task_struct *old, struct task_struct *new,
enum pid_type type)
--- kernel/fork.c 2007-11-09 12:57:31.000000000 +0300
+++ - 2007-11-18 16:57:34.037105563 +0300
@@ -121,6 +121,7 @@ void __put_task_struct(struct task_struc
WARN_ON(atomic_read(&tsk->usage));
WARN_ON(tsk == current);
+ task_put_pids(tsk->pids);
security_task_free(tsk);
free_uid(tsk->user);
put_group_info(tsk->group_info);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
