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 <[email protected]>
> ---
>
> 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.
On Fri, 2007-12-14 at 15:52 -0500, J. Bruce Fields wrote:
> 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 <[email protected]>
> > ---
> >
> > 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.
>
It's not.
> --b.