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

[Serge E. Hallyn]

Quoting Dan Smith (danms at us.ibm.com):
> 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 at us.ibm.com>


> detection phase to adjust SOCK_DEAD sockets' reference counts down by
> one to match reality.
> 
> Signed-off-by: Dan Smith <danms at us.ibm.com>
> ---
>  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 at lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers