2009-06-04 18:16:30

by Labiaga, Ricardo

[permalink] [raw]
Subject: RE: [pnfs] [RFC 04/39] nfs41: minorversion support for nfs4_{init, destroy}_callback

> -----Original Message-----
> From: Myklebust, Trond
> Sent: Wednesday, June 03, 2009 7:40 PM
> To: Benny Halevy
> Cc: [email protected]; [email protected]
> Subject: Re: [pnfs] [RFC 04/39] nfs41: minorversion support for
> nfs4_{init, destroy}_callback
>
> On Fri, 2009-05-01 at 02:19 +0300, Benny Halevy wrote:
> > move nfs4_init_callback into nfs4_init_client_minor_version
> > and nfs4_destroy_callback into nfs4_clear_client_minor_version
> >
> > as these need to happen also when auto-negotiating the minorversion
> > once the callback service for nfs41 becomes different than for
nfs4.0
> >
> > Signed-off-by: Benny Halevy <[email protected]>
> > [nfs41: Fix checkpatch warning]
> > Signed-off-by: Ricardo Labiaga <[email protected]>
> > Signed-off-by: Benny Halevy <[email protected]>
> > ---
> > fs/nfs/callback.c | 67
++++++++++++++++++++++++++++++++++++++--------
> -------
> > fs/nfs/callback.h | 2 +-
> > fs/nfs/client.c | 21 +++++-----------
> > 3 files changed, 56 insertions(+), 34 deletions(-)
> >
> > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> > index a886e69..3926046 100644
> > --- a/fs/nfs/callback.c
> > +++ b/fs/nfs/callback.c
> > @@ -59,7 +59,7 @@ module_param_call(callback_tcpport,
param_set_port,
> param_get_int,
> > * This is the callback kernel thread.
> > */
> > static int
> > -nfs_callback_svc(void *vrqstp)
> > +nfs4_callback_svc(void *vrqstp)
> > {
> > int err, preverr = 0;
> > struct svc_rqst *rqstp = vrqstp;
> > @@ -97,20 +97,12 @@ nfs_callback_svc(void *vrqstp)
> > }
> >
> > /*
> > - * Bring up the callback thread if it is not already up.
> > + * Prepare to bring up the NFSv4 callback service
> > */
> > -int nfs_callback_up(void)
> > +struct svc_rqst *
> > +nfs4_callback_up(struct svc_serv *serv)
> > {
> > - struct svc_serv *serv = NULL;
> > - int ret = 0;
> > -
> > - mutex_lock(&nfs_callback_mutex);
> > - if (nfs_callback_info.users++ || nfs_callback_info.task != NULL)
> > - goto out;
> > - serv = svc_create(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE,
> NULL);
> > - ret = -ENOMEM;
> > - if (!serv)
> > - goto out_err;
> > + int ret;
> >
> > ret = svc_create_xprt(serv, "tcp", PF_INET,
> > nfs_callback_set_tcpport,
SVC_SOCK_ANONYMOUS);
> > @@ -131,18 +123,55 @@ int nfs_callback_up(void)
> > goto out_err;
> > #endif /* defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
*/
> >
> > - nfs_callback_info.rqst = svc_prepare_thread(serv, &serv-
> >sv_pools[0]);
> > - if (IS_ERR(nfs_callback_info.rqst)) {
> > - ret = PTR_ERR(nfs_callback_info.rqst);
> > - nfs_callback_info.rqst = NULL;
> > + return svc_prepare_thread(serv, &serv->sv_pools[0]);
> > +
> > +out_err:
> > + if (ret == 0)
> > + ret = -ENOMEM;
> > + return ERR_PTR(ret);
> > +}
> > +
> > +/*
> > + * Bring up the callback thread if it is not already up.
> > + */
> > +int nfs_callback_up(u32 minorversion, void *args)
> ^^^^^^^^^^
> There is no reason not to type check the arguments here. Please fix...

Will do.

- ricardo

> > +{
> > + struct svc_serv *serv = NULL;
> > + struct svc_rqst *rqstp;
> > + int (*callback_svc)(void *vrqstp);
> > + char svc_name[12];
> > + int ret = 0;
> > +
> > + mutex_lock(&nfs_callback_mutex);
> > + if (nfs_callback_info.users++ || nfs_callback_info.task != NULL)
> > + goto out;
> > + serv = svc_create(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE,
> NULL);
> > + if (!serv) {
> > + ret = -ENOMEM;
> > + goto out_err;
> > + }
> > +
> > + /* FIXME: either 4.0 or 4.1 callback service can be up at a time
> > + * need to monitor and control them both */
> > + if (!minorversion) {
> > + rqstp = nfs4_callback_up(serv);
> > + callback_svc = nfs4_callback_svc;
> > + } else {
> > + BUG(); /* for now */
> > + }
> > +
> > + if (IS_ERR(rqstp)) {
> > + ret = PTR_ERR(rqstp);
> > goto out_err;
> > }
> >
> > svc_sock_update_bufs(serv);
> >
> > - nfs_callback_info.task = kthread_run(nfs_callback_svc,
> > + sprintf(svc_name, "nfsv4.%u-svc", minorversion);
> > + nfs_callback_info.rqst = rqstp;
> > + nfs_callback_info.task = kthread_run(callback_svc,
> > nfs_callback_info.rqst,
> > - "nfsv4-svc");
> > + svc_name);
> > if (IS_ERR(nfs_callback_info.task)) {
> > ret = PTR_ERR(nfs_callback_info.task);
> > svc_exit_thread(nfs_callback_info.rqst);
> > diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
> > index e110e28..7f12e65 100644
> > --- a/fs/nfs/callback.h
> > +++ b/fs/nfs/callback.h
> > @@ -63,7 +63,7 @@ extern __be32 nfs4_callback_getattr(struct
> cb_getattrargs *args, struct cb_getat
> > extern __be32 nfs4_callback_recall(struct cb_recallargs *args, void
> *dummy);
> >
> > #ifdef CONFIG_NFS_V4
> > -extern int nfs_callback_up(void);
> > +extern int nfs_callback_up(u32 minorversion, void *args);
> > extern void nfs_callback_down(void);
> > #else
> > #define nfs_callback_up() (0)
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index d9657d4..df2b40d 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -47,9 +47,6 @@
> > #include "internal.h"
> > #include "fscache.h"
> >
> > -static int nfs4_init_callback(struct nfs_client *);
> > -static void nfs4_destroy_callback(struct nfs_client *);
> > -
> > #define NFSDBG_FACILITY NFSDBG_CLIENT
> >
> > static DEFINE_SPINLOCK(nfs_client_lock);
> > @@ -124,9 +121,6 @@ static struct nfs_client *nfs_alloc_client(const
> struct nfs_client_initdata *cl_
> >
> > clp->rpc_ops = cl_init->rpc_ops;
> >
> > - if (nfs4_init_callback(clp) < 0)
> > - goto error_2;
> > -
> > atomic_set(&clp->cl_count, 1);
> > clp->cl_cons_state = NFS_CS_INITING;
> >
> > @@ -136,7 +130,7 @@ static struct nfs_client *nfs_alloc_client(const
> struct nfs_client_initdata *cl_
> > if (cl_init->hostname) {
> > clp->cl_hostname = kstrdup(cl_init->hostname,
GFP_KERNEL);
> > if (!clp->cl_hostname)
> > - goto error_3;
> > + goto error_cleanup;
> > }
> >
> > INIT_LIST_HEAD(&clp->cl_superblocks);
> > @@ -161,9 +155,7 @@ static struct nfs_client *nfs_alloc_client(const
> struct nfs_client_initdata *cl_
> >
> > return clp;
> >
> > -error_3:
> > - nfs4_destroy_callback(clp);
> > -error_2:
> > +error_cleanup:
> > kfree(clp);
> > error_0:
> > return NULL;
> > @@ -207,6 +199,8 @@ static void
nfs4_clear_client_minor_version(struct
> nfs_client *clp)
> >
> > clp->cl_call_sync = _nfs4_call_sync;
> > #endif /* CONFIG_NFS_V4_1 */
> > +
> > + nfs4_destroy_callback(clp);
> > }
> >
> > /*
> > @@ -225,8 +219,6 @@ static void nfs_free_client(struct nfs_client
*clp)
> > if (!IS_ERR(clp->cl_rpcclient))
> > rpc_shutdown_client(clp->cl_rpcclient);
> >
> > - nfs4_destroy_callback(clp);
> > -
> > if (clp->cl_machine_cred != NULL)
> > put_rpccred(clp->cl_machine_cred);
> >
> > @@ -1104,7 +1096,8 @@ static int nfs4_init_callback(struct
nfs_client
> *clp)
> > int error;
> >
> > if (clp->rpc_ops->version == 4) {
> > - error = nfs_callback_up();
> > + error = nfs_callback_up(clp->cl_minorversion,
> > + clp->cl_rpcclient->cl_xprt);
> > if (error < 0) {
> > dprintk("%s: failed to start callback. Error =
%d\n",
> > __func__, error);
> > @@ -1139,7 +1132,7 @@ static int
nfs4_init_client_minor_version(struct
> nfs_client *clp)
> > }
> > #endif /* CONFIG_NFS_V4_1 */
> >
> > - return 0;
> > + return nfs4_init_callback(clp);
> > }
> >
> > /*
>
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
> _______________________________________________
> pNFS mailing list
> [email protected]
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs