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

[ebiederm@xmission.com (Eric W. Biederman)]

Oleg Nesterov <oleg@tv-sign.ru> 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