Re: [RFC PATCH 5 3/7] nfsd - use service thread if not executing in init namespace

[J. Bruce Fields]

On Wed, Apr 01, 2015 at 08:22:58AM +0800, Ian Kent wrote:
> On Tue, 2015-03-31 at 09:14 -0400, J. Bruce Fields wrote:
> > On Tue, Mar 31, 2015 at 11:14:58AM +0800, Ian Kent wrote:
> > > From: Ian Kent <ikent@xxxxxxxxxx>
> > > 
> > > If nfsd is running within a container the client tracking operations
> > > should run within their originating container also. To do that get a
> > > token to a service thread created within the container environment
> > > for usermode helper calls.
> > > 
> > > Signed-off-by: Ian Kent <ikent@xxxxxxxxxx>
> > > Cc: Benjamin Coddington <bcodding@xxxxxxxxxx>
> > > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> > > Cc: J. Bruce Fields <bfields@xxxxxxxxxxxx>
> > > Cc: David Howells <dhowells@xxxxxxxxxx>
> > > Cc: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > > Cc: Oleg Nesterov <onestero@xxxxxxxxxx>
> > > Cc: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
> > > Cc: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
> > > ---
> > >  fs/nfsd/netns.h       |    3 +++
> > >  fs/nfsd/nfs4recover.c |   48 +++++++++++++++++++++++++++++++-----------------
> > >  fs/nfsd/nfsctl.c      |    6 ++++++
> > >  3 files changed, 40 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> > > index ea6749a..099a3c5 100644
> > > --- a/fs/nfsd/netns.h
> > > +++ b/fs/nfsd/netns.h
> > > @@ -112,6 +112,9 @@ struct nfsd_net {
> > >  	u32 clientid_counter;
> > >  
> > >  	struct svc_serv *nfsd_serv;
> > > +
> > > +	/* Namespace token */
> > > +	int umh_token;
> > >  };
> > >  
> > >  /* Simple check to find out if a given net was properly initialized */
> > > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > > index 1c307f0..2547edb 100644
> > > --- a/fs/nfsd/nfs4recover.c
> > > +++ b/fs/nfsd/nfs4recover.c
> > > @@ -1184,7 +1184,8 @@ nfsd4_cltrack_grace_start(time_t grace_start)
> > >  }
> > >  
> > >  static int
> > > -nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1)
> > > +nfsd4_umh_cltrack_upcall(char *cmd, char *arg,
> > > +			 char *env0, char *env1, int token)
> > >  {
> > >  	char *envp[3];
> > >  	char *argv[4];
> > > @@ -1209,7 +1210,11 @@ nfsd4_umh_cltrack_upcall(char *cmd, char *arg, char *env0, char *env1)
> > >  	argv[2] = arg;
> > >  	argv[3] = NULL;
> > >  
> > > -	ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
> > > +	if (token > 0)
> > > +		ret = call_usermodehelper_service(argv[0], argv, envp,
> > > +						  token, UMH_WAIT_PROC);
> > > +	else
> > > +		ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
> > 
> > Do we really need to handle the init_net case specially, or could we
> > just let it create a workqueue in that case as well?
> 
> That's pretty much up to you but there's still the need to get a token
> to create the work queue (and put it to terminate it) or just pass 0 to
> call_usermodehelper_service().

Creating a new workqueue in the init_net case would mean inheriting
stuff from the rpc.nfsd environment that we didn't used to.  (But what
stuff exactly?)  So for backwards compatibility reasons perhaps it's
safer to do what you've done even if the result seems a little
inconsistent.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/