Re: [PATCH 14/38] svc: Change sk_inuse to a kref

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, Dec 11, 2007 at 05:32:25PM -0600, Tom Tucker wrote:
> 
> Change the atomic_t reference count to a kref and move it to the 
> transport indepenent svc_xprt structure. Change the reference count
> wrapper names to be generic.
> 
> Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
> ---
> 
>  include/linux/sunrpc/svc_xprt.h |    8 ++++++
>  include/linux/sunrpc/svcsock.h  |    1 -
>  net/sunrpc/svc_xprt.c           |   17 ++++++++++++
>  net/sunrpc/svcsock.c            |   54 +++++++++++++++------------------------
>  4 files changed, 46 insertions(+), 34 deletions(-)
> 
> diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
> index 3f4a1df..eb801ad 100644
> --- a/include/linux/sunrpc/svc_xprt.h
> +++ b/include/linux/sunrpc/svc_xprt.h
> @@ -8,6 +8,7 @@
>  #define SUNRPC_SVC_XPRT_H
>  
>  #include <linux/sunrpc/svc.h>
> +#include <linux/module.h>
>  
>  struct svc_xprt_ops {
>  	struct svc_xprt	*(*xpo_create)(struct svc_serv *,
> @@ -34,11 +35,18 @@ struct svc_xprt_class {
>  struct svc_xprt {
>  	struct svc_xprt_class	*xpt_class;
>  	struct svc_xprt_ops	*xpt_ops;
> +	struct kref		xpt_ref;
>  };
>  
>  int	svc_reg_xprt_class(struct svc_xprt_class *);
>  int	svc_unreg_xprt_class(struct svc_xprt_class *);
>  void	svc_xprt_init(struct svc_xprt_class *, struct svc_xprt *);
>  int	svc_create_xprt(struct svc_serv *, char *, unsigned short, int);
> +void	svc_xprt_put(struct svc_xprt *xprt);
> +
> +static inline void svc_xprt_get(struct svc_xprt *xprt)
> +{
> +	kref_get(&xprt->xpt_ref);
> +}
>  
>  #endif /* SUNRPC_SVC_XPRT_H */
> diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
> index 3181d9d..ba07d50 100644
> --- a/include/linux/sunrpc/svcsock.h
> +++ b/include/linux/sunrpc/svcsock.h
> @@ -24,7 +24,6 @@ struct svc_sock {
>  
>  	struct svc_pool *	sk_pool;	/* current pool iff queued */
>  	struct svc_serv *	sk_server;	/* service for this socket */
> -	atomic_t		sk_inuse;	/* use count */
>  	unsigned long		sk_flags;
>  #define	SK_BUSY		0			/* enqueued/receiving */
>  #define	SK_CONN		1			/* conn pending */
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 9136da4..43418cf 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -82,6 +82,22 @@ int svc_unreg_xprt_class(struct svc_xprt_class *xcl)
>  }
>  EXPORT_SYMBOL_GPL(svc_unreg_xprt_class);
>  
> +static void svc_xprt_free(struct kref *kref)
> +{
> +	struct svc_xprt *xprt =
> +		container_of(kref, struct svc_xprt, xpt_ref);
> +	struct module *owner = xprt->xpt_class->xcl_owner;
> +	BUG_ON(atomic_read(&kref->refcount));

All this BUG_ON() is doing is checking for what looks like a bizarrely
unlikely bug in kref_put().  The BUG_ON from the original code is:

	BUG_ON(!test_bit(SK_DEAD, &svsk->sk_flags));

But I don't have any strong feelings as to whether that's necessary.

--b.