Re: [PATCH 1/2] Adjust socket reference count during leak detection phase

[Oren Laadan <orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>]

Serge E. Hallyn wrote:
> Quoting Dan Smith (danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
>> Sockets that have been disconnected from their struct file have
>> a reference count one less than normal sockets (potentially zero if the
>> socket is only in the system because of an outstanding skb).  Since the
>> objhash assumes a minimum reference count of 2, skb's in such a situation
>> will always be counted as a leak.
>>
>> This (probably contentious) patch adds a fixup function during the leak
> 
> This is IMO the right place for it though.  If we have to play
> games with refcounts to pacify leak detection, then all such
> games should be played together in ckpt_obj_contained.
> 
> Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

I concur, it's a leak-detection specific code.
Pulled for v18.

Oren.

> 
> 
>> detection phase to adjust SOCK_DEAD sockets' reference counts down by
>> one to match reality.
>>
>> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>> ---
>>  checkpoint/objhash.c |   24 ++++++++++++++++++++++++
>>  1 files changed, 24 insertions(+), 0 deletions(-)
>>
>> diff --git a/checkpoint/objhash.c b/checkpoint/objhash.c
>> index b85aa77..ee9dbfd 100644
>> --- a/checkpoint/objhash.c
>> +++ b/checkpoint/objhash.c
>> @@ -816,6 +816,26 @@ static void ckpt_obj_users_inc(struct ckpt_ctx *ctx, void *ptr, int increment)
>>   */
>>
>>  /**
>> + * obj_sock_adjust_users - remove implicit reference on DEAD sockets
>> + * @obj: CKPT_OBJ_SOCK object to adjust
>> + *
>> + * Sockets that have been disconnected from their struct file have
>> + * a reference count one less than normal sockets.  The objhash's
>> + * assumption of such a reference is therefore incorrect, so we correct
>> + * it here.
>> + */
>> +static inline void obj_sock_adjust_users(struct ckpt_obj *obj)
>> +{
>> +	struct sock *sk = (struct sock *)obj->ptr;
>> +
>> +	if (sock_flag(sk, SOCK_DEAD)) {
>> +		obj->users--;
>> +		ckpt_debug("Adjusting SOCK %i count to %i\n",
>> +			   obj->objref, obj->users);
>> +	}
>> +}
>> +
>> +/**
>>   * ckpt_obj_contained - test if shared objects are contained in checkpoint
>>   * @ctx: checkpoint context
>>   *
>> @@ -838,6 +858,10 @@ int ckpt_obj_contained(struct ckpt_ctx *ctx)
>>  	hlist_for_each_entry(obj, node, &ctx->obj_hash->list, next) {
>>  		if (!obj->ops->ref_users)
>>  			continue;
>> +
>> +		if (obj->ops->obj_type == CKPT_OBJ_SOCK)
>> +			obj_sock_adjust_users(obj);
>> +
>>  		if (obj->ops->ref_users(obj->ptr) != obj->users) {
>>  			ckpt_debug("usage leak: %s\n", obj->ops->obj_name);
>>  			ckpt_write_err(ctx, "OP", "leak: usage (%d != %d (%s)",
>> -- 
>> 1.6.2.5
>>
>> _______________________________________________
>> Containers mailing list
>> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>> https://lists.linux-foundation.org/mailman/listinfo/containers
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers