Re: [PATCH 01/23] tref: Implement task references.

From: Eric W. Biederman
Date: Tue Mar 07 2006 - 17:59:34 EST


Oleg Nesterov <oleg@xxxxxxxxxx> writes:

> "Eric W. Biederman" wrote:
>>
>> struct pid
>> {
>> + atomic_t count;
>> /* Try to keep pid_chain in the same cacheline as nr for find_pid */
>> int nr;
>> struct hlist_node pid_chain;
>> /* list of pids with the same nr, only one of them is in the hash */
>> - struct list_head pid_list;
>> - /* Does a weak reference of this type exist to the task struct? */
>> - struct task_ref *tref;
>> + struct list_head tasks[PIDTYPE_MAX];
>> + struct rcu_head rcu;
>> };
>>
>> ...
>>
>> +static void rcu_put_pid(struct rcu_head *rhp)
>> +{
>> + struct pid *pid = container_of(rhp, struct pid, rcu);
>> + put_pid(pid);
>> +}
>
> I hope we can do it without pid->rcu and rcu_put_pid(). Hopefuly
> we can use SLAB_DESTROY_BY_RCU. To do so we need some changes in
> find_task_by_pid_type().

I am fairly certain that SLAB_DESTROY_BY_RCU is a entire slab
operation, not something that applies on an individual slab
entry basis.

Delaying the decrement of the struct pid with call_rcu does have the
advantage that we can safely do an atomic_inc(&pid->count) after
looking up the pid.

> I'll try to look closer at this patch tomorrow.

Thanks. One semi painful thing that has occurred to me is that
we may want to make the definition.

struct pid
{
atomic_t count;
/* Try to keep pid_chain in the same cacheline as nr for find_pid */
int nr;
struct hlist_node pid_chain;
/* list of pids with the same nr, only one of them is in the hash */
struct hlist_head tasks[PIDTYPE_MAX];
struct rcu_head rcu;
};

Using a hlist_head for tasks cuts the size of the structure almost in
half. Which for a long lived structure is a desirable property.
Unfortunately hlist_for_each_entry_rcu takes an additional argument,
compared to list_for_each_entry_rcu.

Eric

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/