[PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Stanislav Kinsbursky]

This is a bug fix for https://bugzilla.redhat.com/show_bug.cgi?id=830862.

The problem is that with NFSv4 mount in container (with separated mount
namesapce) and active lock on it, dying child reaped of this container will
try to umount NFS and doing this will try to create RPC client to send
unmonitor request to statd.
But creation of RCP client requires valid current->nsproxy (for operation with
utsname()) and during umount on child reaper exit it's equal to zero.

Proposed solution is to introduce refrence-counter per-net NSM client, which
is created on fist monitor call and destroyed after the lst monitor call.

The following series implements...

---

Stanislav Kinsbursky (3):
      lockd: use rpc client's cl_nodename for id encoding
      lockd: per-net NSM client creation and destruction helpers introduced
      lockd: create and use per-net NSM RPC clients on MON/UNMON requests


 fs/lockd/mon.c   |   91 +++++++++++++++++++++++++++++++++++++++++++-----------
 fs/lockd/netns.h |    4 ++
 fs/lockd/svc.c   |    1 +
 3 files changed, 77 insertions(+), 19 deletions(-)

[PATCH 1/3] lockd: use rpc client's cl_nodename for id encoding

[Stanislav Kinsbursky]

Taking hostname from uts namespace if not safe, because this cuold be
performind during umount operation on child reaper death. And in this case
current->nsproxy is NULL already.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: <stable@vger.kernel.org>
---
 fs/lockd/mon.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index 7ef14b3..c6186fb 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -426,11 +426,12 @@ static void encode_mon_name(struct xdr_stream *xdr, const struct nsm_args *argp)
  * (via the NLMPROC_SM_NOTIFY call) that the state of host "mon_name"
  * has changed.
  */
-static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp)
+static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp,
+			 char *nodename)
 {
 	__be32 *p;
 
-	encode_nsm_string(xdr, utsname()->nodename);
+	encode_nsm_string(xdr, nodename);
 	p = xdr_reserve_space(xdr, 4 + 4 + 4);
 	*p++ = cpu_to_be32(argp->prog);
 	*p++ = cpu_to_be32(argp->vers);
@@ -441,10 +442,11 @@ static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp)
  * The "mon_id" argument specifies the non-private arguments
  * of an NSMPROC_MON or NSMPROC_UNMON call.
  */
-static void encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp)
+static void encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp,
+			  char *nodename)
 {
 	encode_mon_name(xdr, argp);
-	encode_my_id(xdr, argp);
+	encode_my_id(xdr, argp, nodename);
 }
 
 /*
@@ -463,14 +465,14 @@ static void encode_priv(struct xdr_stream *xdr, const struct nsm_args *argp)
 static void nsm_xdr_enc_mon(struct rpc_rqst *req, struct xdr_stream *xdr,
 			    const struct nsm_args *argp)
 {
-	encode_mon_id(xdr, argp);
+	encode_mon_id(xdr, argp, req->rq_task->tk_client->cl_nodename);
 	encode_priv(xdr, argp);
 }
 
 static void nsm_xdr_enc_unmon(struct rpc_rqst *req, struct xdr_stream *xdr,
 			      const struct nsm_args *argp)
 {
-	encode_mon_id(xdr, argp);
+	encode_mon_id(xdr, argp, req->rq_task->tk_client->cl_nodename);
 }
 
 static int nsm_xdr_dec_stat_res(struct rpc_rqst *rqstp,

Re: [PATCH 1/3] lockd: use rpc client's cl_nodename for id encoding

[Chuck Lever]

Hi-

On Sep 14, 2012, at 10:25 AM, Stanislav Kinsbursky wrote:

> Taking hostname from uts namespace if not safe, because this cuold be
> performind during umount operation on child reaper death. And in this case
> current->nsproxy is NULL already.
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> Cc: <stable@vger.kernel.org>
> ---
> fs/lockd/mon.c |   14 ++++++++------
> 1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
> index 7ef14b3..c6186fb 100644
> --- a/fs/lockd/mon.c
> +++ b/fs/lockd/mon.c
> @@ -426,11 +426,12 @@ static void encode_mon_name(struct xdr_stream *xdr, const struct nsm_args *argp)
>  * (via the NLMPROC_SM_NOTIFY call) that the state of host "mon_name"
>  * has changed.
>  */
> -static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp)
> +static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp,
> +			 char *nodename)
> {
> 	__be32 *p;
> 
> -	encode_nsm_string(xdr, utsname()->nodename);
> +	encode_nsm_string(xdr, nodename);
> 	p = xdr_reserve_space(xdr, 4 + 4 + 4);
> 	*p++ = cpu_to_be32(argp->prog);
> 	*p++ = cpu_to_be32(argp->vers);
> @@ -441,10 +442,11 @@ static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp)
>  * The "mon_id" argument specifies the non-private arguments
>  * of an NSMPROC_MON or NSMPROC_UNMON call.
>  */
> -static void encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp)
> +static void encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp,
> +			  char *nodename)
> {
> 	encode_mon_name(xdr, argp);
> -	encode_my_id(xdr, argp);
> +	encode_my_id(xdr, argp, nodename);
> }
> 
> /*
> @@ -463,14 +465,14 @@ static void encode_priv(struct xdr_stream *xdr, const struct nsm_args *argp)
> static void nsm_xdr_enc_mon(struct rpc_rqst *req, struct xdr_stream *xdr,
> 			    const struct nsm_args *argp)
> {
> -	encode_mon_id(xdr, argp);
> +	encode_mon_id(xdr, argp, req->rq_task->tk_client->cl_nodename);

IMO you should get the cl_nodename in nsm_mon_unmon() from clnt->cl_nodename, and pass it in as part of *argp .  The choice of which nodename to use is clearly a decision for an "upper layer" not a choice for the XDR functions.

Long ago I had patches which fixed this layering violation for a very similar purpose as yours, but they were never applied.

> 	encode_priv(xdr, argp);
> }
> 
> static void nsm_xdr_enc_unmon(struct rpc_rqst *req, struct xdr_stream *xdr,
> 			      const struct nsm_args *argp)
> {
> -	encode_mon_id(xdr, argp);
> +	encode_mon_id(xdr, argp, req->rq_task->tk_client->cl_nodename);

Ditto.

> }
> 
> static int nsm_xdr_dec_stat_res(struct rpc_rqst *rqstp,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/3] lockd: use rpc client's cl_nodename for id encoding

[Stanislav Kinsbursky]

14.09.2012 23:08, Chuck Lever пишет:
> Hi-
>
> On Sep 14, 2012, at 10:25 AM, Stanislav Kinsbursky wrote:
>
>> Taking hostname from uts namespace if not safe, because this cuold be
>> performind during umount operation on child reaper death. And in this case
>> current->nsproxy is NULL already.
>>
>> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
>> Cc: <stable@vger.kernel.org>
>> ---
>> fs/lockd/mon.c |   14 ++++++++------
>> 1 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
>> index 7ef14b3..c6186fb 100644
>> --- a/fs/lockd/mon.c
>> +++ b/fs/lockd/mon.c
>> @@ -426,11 +426,12 @@ static void encode_mon_name(struct xdr_stream *xdr, const struct nsm_args *argp)
>>   * (via the NLMPROC_SM_NOTIFY call) that the state of host "mon_name"
>>   * has changed.
>>   */
>> -static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp)
>> +static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp,
>> +			 char *nodename)
>> {
>> 	__be32 *p;
>>
>> -	encode_nsm_string(xdr, utsname()->nodename);
>> +	encode_nsm_string(xdr, nodename);
>> 	p = xdr_reserve_space(xdr, 4 + 4 + 4);
>> 	*p++ = cpu_to_be32(argp->prog);
>> 	*p++ = cpu_to_be32(argp->vers);
>> @@ -441,10 +442,11 @@ static void encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp)
>>   * The "mon_id" argument specifies the non-private arguments
>>   * of an NSMPROC_MON or NSMPROC_UNMON call.
>>   */
>> -static void encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp)
>> +static void encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp,
>> +			  char *nodename)
>> {
>> 	encode_mon_name(xdr, argp);
>> -	encode_my_id(xdr, argp);
>> +	encode_my_id(xdr, argp, nodename);
>> }
>>
>> /*
>> @@ -463,14 +465,14 @@ static void encode_priv(struct xdr_stream *xdr, const struct nsm_args *argp)
>> static void nsm_xdr_enc_mon(struct rpc_rqst *req, struct xdr_stream *xdr,
>> 			    const struct nsm_args *argp)
>> {
>> -	encode_mon_id(xdr, argp);
>> +	encode_mon_id(xdr, argp, req->rq_task->tk_client->cl_nodename);
>
> IMO you should get the cl_nodename in nsm_mon_unmon() from clnt->cl_nodename, and pass it in as part of *argp .  The choice of which nodename to use is clearly a decision for an "upper layer" not a choice for the XDR functions.
>
> Long ago I had patches which fixed this layering violation for a very similar purpose as yours, but they were never applied.
>

I like it. Thanks.

-- 
Best regards,
Stanislav Kinsbursky

[PATCH 2/3] lockd: per-net NSM client creation and destruction helpers introduced

[Stanislav Kinsbursky]

NSM RPC client can be required on NFSv3 umount, when child reaper is dying (and
destroying it's mount namespace). It means, that current nsproxy is set to
NULL already, but creation of RPC client requires UTS namespace for gaining
hostname string.
This patch introduces reference counted NFS RPC clients creation and
destruction helpers (similar to RPCBIND RPC clients).

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: <stable@vger.kernel.org>
---
 fs/lockd/mon.c   |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/lockd/netns.h |    4 ++++
 fs/lockd/svc.c   |    1 +
 3 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index c6186fb..77e07fe 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -19,6 +19,8 @@
 
 #include <asm/unaligned.h>
 
+#include "netns.h"
+
 #define NLMDBG_FACILITY		NLMDBG_MONITOR
 #define NSM_PROGRAM		100024
 #define NSM_VERSION		1
@@ -83,6 +85,51 @@ static struct rpc_clnt *nsm_create(struct net *net)
 	return rpc_create(&args);
 }
 
+__maybe_unused static struct rpc_clnt *nsm_client_get(struct net *net)
+{
+	static DEFINE_MUTEX(nsm_create_mutex);
+	struct rpc_clnt	*clnt;
+	struct lockd_net *ln = net_generic(net, lockd_net_id);
+
+	spin_lock(&ln->nsm_clnt_lock);
+	if (ln->nsm_users) {
+		ln->nsm_users++;
+		clnt = ln->nsm_clnt;
+		spin_unlock(&ln->nsm_clnt_lock);
+		goto out;
+	}
+	spin_unlock(&ln->nsm_clnt_lock);
+
+	mutex_lock(&nsm_create_mutex);
+	clnt = nsm_create(net);
+	if (!IS_ERR(clnt)) {
+		ln->nsm_clnt = clnt;
+		smp_wmb();
+		ln->nsm_users = 1;
+	}
+	mutex_unlock(&nsm_create_mutex);
+out:
+	return clnt;
+}
+
+__maybe_unused static void nsm_client_put(struct net *net)
+{
+	struct lockd_net *ln = net_generic(net, lockd_net_id);
+	struct rpc_clnt	*clnt = ln->nsm_clnt;
+	int shutdown = 0;
+
+	spin_lock(&ln->nsm_clnt_lock);
+	if (ln->nsm_users) {
+		if (--ln->nsm_users)
+			ln->nsm_clnt = NULL;
+		shutdown = !ln->nsm_users;
+	}
+	spin_unlock(&ln->nsm_clnt_lock);
+
+	if (shutdown)
+		rpc_shutdown_client(clnt);
+}
+
 static int nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res,
 			 struct net *net)
 {
diff --git a/fs/lockd/netns.h b/fs/lockd/netns.h
index 4eee248..5010b55 100644
--- a/fs/lockd/netns.h
+++ b/fs/lockd/netns.h
@@ -12,6 +12,10 @@ struct lockd_net {
 	struct delayed_work grace_period_end;
 	struct lock_manager lockd_manager;
 	struct list_head grace_list;
+
+	spinlock_t nsm_clnt_lock;
+	unsigned int nsm_users;
+	struct rpc_clnt *nsm_clnt;
 };
 
 extern int lockd_net_id;
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 31a63f8..7e35587 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -596,6 +596,7 @@ static int lockd_init_net(struct net *net)
 
 	INIT_DELAYED_WORK(&ln->grace_period_end, grace_ender);
 	INIT_LIST_HEAD(&ln->grace_list);
+	spin_lock_init(&ln->nsm_clnt_lock);
 	return 0;
 }
 

[PATCH 3/3] lockd: create and use per-net NSM RPC clients on MON/UNMON requests

[Stanislav Kinsbursky]

NSM RPC client can be required on NFSv3 umount, when child reaper is dying
(and destroying it's mount namespace). It means, that current nsproxy is set
to NULL already, but creation of RPC client requires UTS namespace for gaining
hostname string.

This patch creates reference-counted per-net NSM client on first monitor
request and destroys it after last unmonitor request.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: <stable@vger.kernel.org>
---
 fs/lockd/mon.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index 77e07fe..c233ed5 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -85,7 +85,7 @@ static struct rpc_clnt *nsm_create(struct net *net)
 	return rpc_create(&args);
 }
 
-__maybe_unused static struct rpc_clnt *nsm_client_get(struct net *net)
+static struct rpc_clnt *nsm_client_get(struct net *net)
 {
 	static DEFINE_MUTEX(nsm_create_mutex);
 	struct rpc_clnt	*clnt;
@@ -112,7 +112,7 @@ out:
 	return clnt;
 }
 
-__maybe_unused static void nsm_client_put(struct net *net)
+static void nsm_client_put(struct net *net)
 {
 	struct lockd_net *ln = net_generic(net, lockd_net_id);
 	struct rpc_clnt	*clnt = ln->nsm_clnt;
@@ -131,9 +131,8 @@ __maybe_unused static void nsm_client_put(struct net *net)
 }
 
 static int nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res,
-			 struct net *net)
+			 struct rpc_clnt *clnt)
 {
-	struct rpc_clnt	*clnt;
 	int		status;
 	struct nsm_args args = {
 		.priv		= &nsm->sm_priv,
@@ -147,13 +146,7 @@ static int nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res,
 		.rpc_resp	= res,
 	};
 
-	clnt = nsm_create(net);
-	if (IS_ERR(clnt)) {
-		status = PTR_ERR(clnt);
-		dprintk("lockd: failed to create NSM upcall transport, "
-				"status=%d\n", status);
-		goto out;
-	}
+	BUG_ON(clnt == NULL);
 
 	memset(res, 0, sizeof(*res));
 
@@ -164,8 +157,6 @@ static int nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res,
 				status);
 	else
 		status = 0;
-	rpc_shutdown_client(clnt);
- out:
 	return status;
 }
 
@@ -185,6 +176,7 @@ int nsm_monitor(const struct nlm_host *host)
 	struct nsm_handle *nsm = host->h_nsmhandle;
 	struct nsm_res	res;
 	int		status;
+	struct rpc_clnt *clnt;
 
 	dprintk("lockd: nsm_monitor(%s)\n", nsm->sm_name);
 
@@ -197,7 +189,15 @@ int nsm_monitor(const struct nlm_host *host)
 	 */
 	nsm->sm_mon_name = nsm_use_hostnames ? nsm->sm_name : nsm->sm_addrbuf;
 
-	status = nsm_mon_unmon(nsm, NSMPROC_MON, &res, host->net);
+	clnt = nsm_client_get(host->net);
+	if (IS_ERR(clnt)) {
+		status = PTR_ERR(clnt);
+		dprintk("lockd: failed to create NSM upcall transport, "
+				"status=%d, net=%p\n", status, host->net);
+		return status;
+	}
+
+	status = nsm_mon_unmon(nsm, NSMPROC_MON, &res, clnt);
 	if (unlikely(res.status != 0))
 		status = -EIO;
 	if (unlikely(status < 0)) {
@@ -229,9 +229,11 @@ void nsm_unmonitor(const struct nlm_host *host)
 
 	if (atomic_read(&nsm->sm_count) == 1
 	 && nsm->sm_monitored && !nsm->sm_sticky) {
+		struct lockd_net *ln = net_generic(host->net, lockd_net_id);
+
 		dprintk("lockd: nsm_unmonitor(%s)\n", nsm->sm_name);
 
-		status = nsm_mon_unmon(nsm, NSMPROC_UNMON, &res, host->net);
+		status = nsm_mon_unmon(nsm, NSMPROC_UNMON, &res, ln->nsm_clnt);
 		if (res.status != 0)
 			status = -EIO;
 		if (status < 0)
@@ -239,6 +241,8 @@ void nsm_unmonitor(const struct nlm_host *host)
 					nsm->sm_name);
 		else
 			nsm->sm_monitored = 0;
+
+		nsm_client_put(host->net);
 	}
 }
 

Re: [PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Chuck Lever]

What happens if statd is restarted?

Sent from my iPhone

On Sep 14, 2012, at 10:25 AM, Stanislav Kinsbursky <skinsbursky@parallels.com> wrote:

> This is a bug fix for https://bugzilla.redhat.com/show_bug.cgi?id=830862.
> 
> The problem is that with NFSv4 mount in container (with separated mount
> namesapce) and active lock on it, dying child reaped of this container will
> try to umount NFS and doing this will try to create RPC client to send
> unmonitor request to statd.
> But creation of RCP client requires valid current->nsproxy (for operation with
> utsname()) and during umount on child reaper exit it's equal to zero.
> 
> Proposed solution is to introduce refrence-counter per-net NSM client, which
> is created on fist monitor call and destroyed after the lst monitor call.
> 
> The following series implements...
> 
> ---
> 
> Stanislav Kinsbursky (3):
>      lockd: use rpc client's cl_nodename for id encoding
>      lockd: per-net NSM client creation and destruction helpers introduced
>      lockd: create and use per-net NSM RPC clients on MON/UNMON requests
> 
> 
> fs/lockd/mon.c   |   91 +++++++++++++++++++++++++++++++++++++++++++-----------
> fs/lockd/netns.h |    4 ++
> fs/lockd/svc.c   |    1 +
> 3 files changed, 77 insertions(+), 19 deletions(-)
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Myklebust, Trond]

T24gRnJpLCAyMDEyLTA5LTE0IGF0IDEzOjAxIC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
V2hhdCBoYXBwZW5zIGlmIHN0YXRkIGlzIHJlc3RhcnRlZD8NCg0KTm90aGluZyB1bnVzdWFsLiBX
aHk/DQoNCj4gU2VudCBmcm9tIG15IGlQaG9uZQ0KPiANCj4gT24gU2VwIDE0LCAyMDEyLCBhdCAx
MDoyNSBBTSwgU3RhbmlzbGF2IEtpbnNidXJza3kgPHNraW5zYnVyc2t5QHBhcmFsbGVscy5jb20+
IHdyb3RlOg0KPiANCj4gPiBUaGlzIGlzIGEgYnVnIGZpeCBmb3IgaHR0cHM6Ly9idWd6aWxsYS5y
ZWRoYXQuY29tL3Nob3dfYnVnLmNnaT9pZD04MzA4NjIuDQo+ID4gDQo+ID4gVGhlIHByb2JsZW0g
aXMgdGhhdCB3aXRoIE5GU3Y0IG1vdW50IGluIGNvbnRhaW5lciAod2l0aCBzZXBhcmF0ZWQgbW91
bnQNCj4gPiBuYW1lc2FwY2UpIGFuZCBhY3RpdmUgbG9jayBvbiBpdCwgZHlpbmcgY2hpbGQgcmVh
cGVkIG9mIHRoaXMgY29udGFpbmVyIHdpbGwNCj4gPiB0cnkgdG8gdW1vdW50IE5GUyBhbmQgZG9p
bmcgdGhpcyB3aWxsIHRyeSB0byBjcmVhdGUgUlBDIGNsaWVudCB0byBzZW5kDQo+ID4gdW5tb25p
dG9yIHJlcXVlc3QgdG8gc3RhdGQuDQo+ID4gQnV0IGNyZWF0aW9uIG9mIFJDUCBjbGllbnQgcmVx
dWlyZXMgdmFsaWQgY3VycmVudC0+bnNwcm94eSAoZm9yIG9wZXJhdGlvbiB3aXRoDQo+ID4gdXRz
bmFtZSgpKSBhbmQgZHVyaW5nIHVtb3VudCBvbiBjaGlsZCByZWFwZXIgZXhpdCBpdCdzIGVxdWFs
IHRvIHplcm8uDQo+ID4gDQo+ID4gUHJvcG9zZWQgc29sdXRpb24gaXMgdG8gaW50cm9kdWNlIHJl
ZnJlbmNlLWNvdW50ZXIgcGVyLW5ldCBOU00gY2xpZW50LCB3aGljaA0KPiA+IGlzIGNyZWF0ZWQg
b24gZmlzdCBtb25pdG9yIGNhbGwgYW5kIGRlc3Ryb3llZCBhZnRlciB0aGUgbHN0IG1vbml0b3Ig
Y2FsbC4NCj4gPiANCj4gPiBUaGUgZm9sbG93aW5nIHNlcmllcyBpbXBsZW1lbnRzLi4uDQo+ID4g
DQo+ID4gLS0tDQo+ID4gDQo+ID4gU3RhbmlzbGF2IEtpbnNidXJza3kgKDMpOg0KPiA+ICAgICAg
bG9ja2Q6IHVzZSBycGMgY2xpZW50J3MgY2xfbm9kZW5hbWUgZm9yIGlkIGVuY29kaW5nDQo+ID4g
ICAgICBsb2NrZDogcGVyLW5ldCBOU00gY2xpZW50IGNyZWF0aW9uIGFuZCBkZXN0cnVjdGlvbiBo
ZWxwZXJzIGludHJvZHVjZWQNCj4gPiAgICAgIGxvY2tkOiBjcmVhdGUgYW5kIHVzZSBwZXItbmV0
IE5TTSBSUEMgY2xpZW50cyBvbiBNT04vVU5NT04gcmVxdWVzdHMNCj4gPiANCj4gPiANCj4gPiBm
cy9sb2NrZC9tb24uYyAgIHwgICA5MSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrLS0tLS0tLS0tLS0NCj4gPiBmcy9sb2NrZC9uZXRucy5oIHwgICAgNCArKw0KPiA+
IGZzL2xvY2tkL3N2Yy5jICAgfCAgICAxICsNCj4gPiAzIGZpbGVzIGNoYW5nZWQsIDc3IGluc2Vy
dGlvbnMoKyksIDE5IGRlbGV0aW9ucygtKQ0KPiA+IA0KPiA+IA0KPiA+IC0tDQo+ID4gVG8gdW5z
dWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4
LW5mcyIgaW4NCj4gPiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2Vy
bmVsLm9yZw0KPiA+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5v
cmcvbWFqb3Jkb21vLWluZm8uaHRtbA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZT
IGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20N
Cnd3dy5uZXRhcHAuY29tDQo=

Re: [PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Chuck Lever]

On Sep 14, 2012, at 1:38 PM, Myklebust, Trond wrote:

> On Fri, 2012-09-14 at 13:01 -0400, Chuck Lever wrote:
>> What happens if statd is restarted?
> 
> Nothing unusual. Why?

The NSM upcall transport is a potential application for TCP + softconn, now that a persistent rpc_clnt is used.  It just depends on what failure mode we'd like to optimize for.

> 
>> Sent from my iPhone
>> 
>> On Sep 14, 2012, at 10:25 AM, Stanislav Kinsbursky <skinsbursky@parallels.com> wrote:
>> 
>>> This is a bug fix for https://bugzilla.redhat.com/show_bug.cgi?id=830862.
>>> 
>>> The problem is that with NFSv4 mount in container (with separated mount
>>> namesapce) and active lock on it, dying child reaped of this container will
>>> try to umount NFS and doing this will try to create RPC client to send
>>> unmonitor request to statd.
>>> But creation of RCP client requires valid current->nsproxy (for operation with
>>> utsname()) and during umount on child reaper exit it's equal to zero.
>>> 
>>> Proposed solution is to introduce refrence-counter per-net NSM client, which
>>> is created on fist monitor call and destroyed after the lst monitor call.
>>> 
>>> The following series implements...
>>> 
>>> ---
>>> 
>>> Stanislav Kinsbursky (3):
>>>     lockd: use rpc client's cl_nodename for id encoding
>>>     lockd: per-net NSM client creation and destruction helpers introduced
>>>     lockd: create and use per-net NSM RPC clients on MON/UNMON requests
>>> 
>>> 
>>> fs/lockd/mon.c   |   91 +++++++++++++++++++++++++++++++++++++++++++-----------
>>> fs/lockd/netns.h |    4 ++
>>> fs/lockd/svc.c   |    1 +
>>> 3 files changed, 77 insertions(+), 19 deletions(-)
>>> 
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Stanislav Kinsbursky]

14.09.2012 23:10, Chuck Lever пишет:
>
> On Sep 14, 2012, at 1:38 PM, Myklebust, Trond wrote:
>
>> On Fri, 2012-09-14 at 13:01 -0400, Chuck Lever wrote:
>>> What happens if statd is restarted?
>>
>> Nothing unusual. Why?
>
> The NSM upcall transport is a potential application for TCP + softconn, now that a persistent rpc_clnt is used.  It just depends on what failure mode we'd like to optimize for.
>

I don't understand, where the problem is.
Could you be more specific, please?

-- 
Best regards,
Stanislav Kinsbursky

Re: [PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Chuck Lever]

On Sep 17, 2012, at 6:49 AM, Stanislav Kinsbursky wrote:

> 14.09.2012 23:10, Chuck Lever пишет:
>> 
>> On Sep 14, 2012, at 1:38 PM, Myklebust, Trond wrote:
>> 
>>> On Fri, 2012-09-14 at 13:01 -0400, Chuck Lever wrote:
>>>> What happens if statd is restarted?
>>> 
>>> Nothing unusual. Why?
>> 
>> The NSM upcall transport is a potential application for TCP + softconn, now that a persistent rpc_clnt is used.  It just depends on what failure mode we'd like to optimize for.
>> 
> 
> I don't understand, where the problem is.
> Could you be more specific, please?

I'm suggesting an enhancement.

The change is to use TCP for the NSM upcall transport, and set RPC_TASK_SOFTCONN on the individual RPCs.  The advantage of this is that the kernel could discover when statd is not running and fail the upcall immediately, rather than waiting possibly many seconds for each upcall RPC to time out.

The client already has a check in the mount.nfs command to see that statd is running, likely to avoid this lengthly timeout.  Since the client already has long-standing logic to avoid it, I think the benefit would be mostly on the server side.

But this change can be done at some later point.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/3] lockd: use per-net refrence-counted NSM clients

[Stanislav Kinsbursky]

17.09.2012 19:10, Chuck Lever пишет:
>
> On Sep 17, 2012, at 6:49 AM, Stanislav Kinsbursky wrote:
>
>> 14.09.2012 23:10, Chuck Lever пишет:
>>>
>>> On Sep 14, 2012, at 1:38 PM, Myklebust, Trond wrote:
>>>
>>>> On Fri, 2012-09-14 at 13:01 -0400, Chuck Lever wrote:
>>>>> What happens if statd is restarted?
>>>>
>>>> Nothing unusual. Why?
>>>
>>> The NSM upcall transport is a potential application for TCP + softconn, now that a persistent rpc_clnt is used.  It just depends on what failure mode we'd like to optimize for.
>>>
>>
>> I don't understand, where the problem is.
>> Could you be more specific, please?
>
> I'm suggesting an enhancement.
>
> The change is to use TCP for the NSM upcall transport, and set RPC_TASK_SOFTCONN on the individual RPCs.  The advantage of this is that the kernel could discover when statd is not running and fail the upcall immediately, rather than waiting possibly many seconds for each upcall RPC to time out.
>
> The client already has a check in the mount.nfs command to see that statd is running, likely to avoid this lengthly timeout.  Since the client already has long-standing logic to avoid it, I think the benefit would be mostly on the server side.
>
> But this change can be done at some later point.
>

Ok, thanks.
Sounds reasonable to me.
I'll do so.

-- 
Best regards,
Stanislav Kinsbursky