Messages in this thread | | | Subject | Re: [PATCH 01/23] tref: Implement task references. | From | (Eric W. Biederman) | Date | Mon, 06 Mar 2006 18:39:54 -0700 |
| |
Oleg Nesterov <oleg@tv-sign.ru> writes:
> I think I have a really good idea. > > Forget about task ref for a moment. I thinks we can greatly > simplify the pids management. We don't PIDTYPE_MAX hash tables, > we need only one. > > The plan: > > kill PIDTYPE_TGID > (copy_process/unhash_process need a simple fix)
Worth doing. But I think it is an independent problem. I almost wonder if it is possible to let the thread group leader exit, at which point we really would need PIDTYPE_TGID.
Or do you need this to have the thread list embedded in the task_struct?
> kill 'struct pid' > > Now, > > struct task_struct > { > ... > struct list_head pids[PIDTYPE_MAX]; > struct list_head tgrp; > ... > }; > > static inline struct task_struct *next_thread(struct task_struct *p) > { > return list_entry(p->tgrp.next, struct task_struct, tgrp); > } > > struct pid_head > { > pid_t nr; > struct hlist_node chain; > struct list_head tasks[PIDTYPE_MAX]; > };
pid_head is decent but I am very tempted to call this struct pid. Especially if we start getting a lot of pointers to them a simple name that makes sense is useful.
> kernel/pid.c: > > static kmem_cache_t *pid_cachep; > > static struct hlist_head *pid_hash; > #define pid_bucket(nr) (pid_hash + pid_hashfn(nr)) > > // alloc_pidmap() becomes static, > // do_fork() calls this instead > struct pid_head *alloc_pid(void) > { > struct pid_head *pid; > > pid = kmem_cache_alloc(pid_cachep, GFP_KERNEL); > > if (likely(pid)) { > enum pid_type type; > > for (type = 0; type < PIDTYPE_MAX; ++type) > INIT_LIST_HEAD(pid->tasks + type); > > pid->nr = alloc_pidmap(); > hlist_add_head_rcu(&pid->chain, pid_bucket(pid->nr)); > } > > return pid; > }
Hmm. I guess that works. I'm tempted to still return just a pid_t. I guess I can't see how the struct pid_head, will be used.
There may be another problem here as well. I don't think we have a lock at this point that makes us safe to update the hash table.
> static struct pid_head *find_pid(pid_t nr) > { > struct hlist_node *node; > struct pid_head *pid; > > hlist_for_each_entry_rcu(pid, node, pid_bucket(nr), chain) > if (pid->nr == nr) > return pid; > > return NULL; > }
I'm pretty certain there are uses for find_pid, outside of pid.c
> And noe we can inplement pid_ref almost for free, just add ->count > to 'struct pid_head'.
If we want to kill the tasklist_lock we also want to add a lock to struct pid_head. Otherwise I don't see how we can safely bump the count, above zero. But using the tasklist_lock for the first version shouldn't be a problem.
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/
| |