From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754559AbbCaNOd (ORCPT ); Tue, 31 Mar 2015 09:14:33 -0400 Received: from fieldses.org ([173.255.197.46]:49452 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752535AbbCaNOb (ORCPT ); Tue, 31 Mar 2015 09:14:31 -0400 Date: Tue, 31 Mar 2015 09:14:29 -0400 From: "J. Bruce Fields" To: Ian Kent Cc: Kernel Mailing List , David Howells , Oleg Nesterov , Trond Myklebust , Benjamin Coddington , Al Viro , Jeff Layton , "Eric W. Biederman" Subject: Re: [RFC PATCH 5 3/7] nfsd - use service thread if not executing in init namespace Message-ID: <20150331131429.GH6901@fieldses.org> References: <20150331030340.10464.30272.stgit@pluto.fritz.box> <20150331031457.10464.98317.stgit@pluto.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150331031457.10464.98317.stgit@pluto.fritz.box> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 31, 2015 at 11:14:58AM +0800, Ian Kent wrote: > From: Ian Kent > > 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 > Cc: Benjamin Coddington > Cc: Al Viro > Cc: J. Bruce Fields > Cc: David Howells > Cc: Trond Myklebust > Cc: Oleg Nesterov > Cc: Eric W. Biederman > Cc: Jeff Layton > --- > 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? --b. > /* > * Disable the upcall mechanism if we're getting an ENOENT or EACCES > * error. The admin can re-enable it on the fly by using sysfs > @@ -1252,14 +1257,8 @@ nfsd4_umh_cltrack_init(struct net *net) > struct nfsd_net *nn = net_generic(net, nfsd_net_id); > char *grace_start = nfsd4_cltrack_grace_start(nn->boot_time); > > - /* XXX: The usermode helper s not working in container yet. */ > - if (net != &init_net) { > - WARN(1, KERN_ERR "NFSD: attempt to initialize umh client " > - "tracking in a container!\n"); > - return -EINVAL; > - } > - > - ret = nfsd4_umh_cltrack_upcall("init", NULL, grace_start, NULL); > + ret = nfsd4_umh_cltrack_upcall("init", NULL, > + grace_start, NULL, nn->umh_token); > kfree(grace_start); > return ret; > } > @@ -1285,6 +1284,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) > { > char *hexid, *has_session, *grace_start; > struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); > + int ret; > > /* > * With v4.0 clients, there's little difference in outcome between a > @@ -1312,7 +1312,10 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) > grace_start = nfsd4_cltrack_grace_start(nn->boot_time); > > nfsd4_cltrack_upcall_lock(clp); > - if (!nfsd4_umh_cltrack_upcall("create", hexid, has_session, grace_start)) > + ret = nfsd4_umh_cltrack_upcall("create", > + hexid, has_session, grace_start, > + nn->umh_token); > + if (!ret) > set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); > nfsd4_cltrack_upcall_unlock(clp); > > @@ -1324,7 +1327,9 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp) > static void > nfsd4_umh_cltrack_remove(struct nfs4_client *clp) > { > + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); > char *hexid; > + int ret; > > if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) > return; > @@ -1336,9 +1341,13 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) > } > > nfsd4_cltrack_upcall_lock(clp); > - if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) && > - nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0) > - clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); > + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) { > + ret = nfsd4_umh_cltrack_upcall("remove", > + hexid, NULL, NULL, > + nn->umh_token); > + if (ret == 0) > + clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); > + } > nfsd4_cltrack_upcall_unlock(clp); > > kfree(hexid); > @@ -1347,8 +1356,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp) > static int > nfsd4_umh_cltrack_check(struct nfs4_client *clp) > { > - int ret; > + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); > char *hexid, *has_session, *legacy; > + int ret; > > if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) > return 0; > @@ -1366,7 +1376,9 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp) > if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) { > ret = 0; > } else { > - ret = nfsd4_umh_cltrack_upcall("check", hexid, has_session, legacy); > + ret = nfsd4_umh_cltrack_upcall("check", hexid, > + has_session, legacy, > + nn->umh_token); > if (ret == 0) > set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags); > } > @@ -1386,7 +1398,9 @@ nfsd4_umh_cltrack_grace_done(struct nfsd_net *nn) > > sprintf(timestr, "%ld", nn->boot_time); > legacy = nfsd4_cltrack_legacy_topdir(); > - nfsd4_umh_cltrack_upcall("gracedone", timestr, legacy, NULL); > + nfsd4_umh_cltrack_upcall("gracedone", > + timestr, legacy, NULL, > + nn->umh_token); > kfree(legacy); > } > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > index aa47d75..f5ae092 100644 > --- a/fs/nfsd/nfsctl.c > +++ b/fs/nfsd/nfsctl.c > @@ -1224,6 +1224,8 @@ static __net_init int nfsd_init_net(struct net *net) > goto out_idmap_error; > nn->nfsd4_lease = 90; /* default lease time */ > nn->nfsd4_grace = 90; > + if (net != &init_net) > + nn->umh_token = umh_wq_get_token(nn->umh_token, "knfsd"); > return 0; > > out_idmap_error: > @@ -1234,6 +1236,10 @@ out_export_error: > > static __net_exit void nfsd_exit_net(struct net *net) > { > + struct nfsd_net *nn = net_generic(net, nfsd_net_id); > + > + umh_wq_put_token(nn->umh_token); > + nn->umh_token = 0; > nfsd_idmap_shutdown(net); > nfsd_export_shutdown(net); > }