From 90d5b18061656993410dfd57ddb88aa5a3f34563 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:18:30 -0400 Subject: NLM: LOCKD fails to load if CONFIG_SYSCTL is not set Bruce Fields says: "By the way, we've got another config-related nit here: http://bugzilla.linux-nfs.org/show_bug.cgi?id=156 You can build lockd without CONFIG_SYSCTL set, but then the module will fail to load." For now, disable the sysctl registration calls in lockd if CONFIG_SYSCTL is not enabled. This allows the kernel to build properly if PROC_FS or SYSCTL is not enabled, but an NFS client is desired. In the long run, we would like to be able to build the kernel with an NFS client but without lockd. This makes sense, for example, if you want an NFSv4-only NFS client, as NFSv4 doesn't use NLM at all. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/svc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'fs/lockd') diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 1ed8bd4de94..38c2f0b1dd7 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -74,7 +74,9 @@ static const unsigned long nlm_timeout_min = 3; static const unsigned long nlm_timeout_max = 20; static const int nlm_port_min = 0, nlm_port_max = 65535; +#ifdef CONFIG_SYSCTL static struct ctl_table_header * nlm_sysctl_table; +#endif static unsigned long get_lockd_grace_period(void) { @@ -359,6 +361,8 @@ out: } EXPORT_SYMBOL(lockd_down); +#ifdef CONFIG_SYSCTL + /* * Sysctl parameters (same as module parameters, different interface). */ @@ -443,6 +447,8 @@ static ctl_table nlm_sysctl_root[] = { { .ctl_name = 0 } }; +#endif /* CONFIG_SYSCTL */ + /* * Module (and sysfs) parameters. */ @@ -516,15 +522,21 @@ module_param(nsm_use_hostnames, bool, 0644); static int __init init_nlm(void) { +#ifdef CONFIG_SYSCTL nlm_sysctl_table = register_sysctl_table(nlm_sysctl_root); return nlm_sysctl_table ? 0 : -ENOMEM; +#else + return 0; +#endif } static void __exit exit_nlm(void) { /* FIXME: delete all NLM clients */ nlm_shutdown_hosts(); +#ifdef CONFIG_SYSCTL unregister_sysctl_table(nlm_sysctl_table); +#endif } module_init(init_nlm); -- cgit v1.2.3 From eb18860e1385bfc7f08fcb7ba362e4a5156c8324 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:18:37 -0400 Subject: NLM: NLM protocol version numbers are u32 Clean up: RPC protocol version numbers are u32. Make sure we use an appropriate type for NLM version numbers when calling nlm_lookup_host(). Eliminates a harmless mixed sign comparison in nlm_host_lookup(). Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/host.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/host.c b/fs/lockd/host.c index f1ef49fff11..f23750db165 100644 --- a/fs/lockd/host.c +++ b/fs/lockd/host.c @@ -42,11 +42,12 @@ static struct nsm_handle * nsm_find(const struct sockaddr_in *sin, /* * Common host lookup routine for server & client */ -static struct nlm_host * -nlm_lookup_host(int server, const struct sockaddr_in *sin, - int proto, int version, const char *hostname, - unsigned int hostname_len, - const struct sockaddr_in *ssin) +static struct nlm_host *nlm_lookup_host(int server, + const struct sockaddr_in *sin, + int proto, u32 version, + const char *hostname, + unsigned int hostname_len, + const struct sockaddr_in *ssin) { struct hlist_head *chain; struct hlist_node *pos; @@ -55,7 +56,7 @@ nlm_lookup_host(int server, const struct sockaddr_in *sin, int hash; dprintk("lockd: nlm_lookup_host("NIPQUAD_FMT"->"NIPQUAD_FMT - ", p=%d, v=%d, my role=%s, name=%.*s)\n", + ", p=%d, v=%u, my role=%s, name=%.*s)\n", NIPQUAD(ssin->sin_addr.s_addr), NIPQUAD(sin->sin_addr.s_addr), proto, version, server? "server" : "client", @@ -175,9 +176,10 @@ nlm_destroy_host(struct nlm_host *host) /* * Find an NLM server handle in the cache. If there is none, create it. */ -struct nlm_host * -nlmclnt_lookup_host(const struct sockaddr_in *sin, int proto, int version, - const char *hostname, unsigned int hostname_len) +struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin, + int proto, u32 version, + const char *hostname, + unsigned int hostname_len) { struct sockaddr_in ssin = {0}; -- cgit v1.2.3 From 099bd05f27ff24a3041d54e7ed42d2eb681484b9 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:25:32 -0400 Subject: lockd: Ensure NSM strings aren't longer than protocol allows Introduce a special helper function to check the length of NSM strings before they are placed on the wire. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/mon.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'fs/lockd') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 908b23fadd0..84fd84cb67b 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -149,6 +149,15 @@ nsm_create(void) * XDR functions for NSM. */ +static __be32 *xdr_encode_nsm_string(__be32 *p, char *string) +{ + size_t len = strlen(string); + + if (len > SM_MAXSTRLEN) + len = SM_MAXSTRLEN; + return xdr_encode_opaque(p, string, len); +} + static __be32 * xdr_encode_common(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { -- cgit v1.2.3 From 496951743100b2d2ccd8b268257e79425e489851 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:25:39 -0400 Subject: lockd: refactor SM_MON mon_name argument encoder Clean up: introduce a new XDR encoder specifically for the mon_name argument of SM_MON requests. This will be updated later to support IPv6 addresses in addition to IPv4 addresses. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/mon.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'fs/lockd') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 84fd84cb67b..87bd4e4d65d 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -18,6 +18,8 @@ #define NLMDBG_FACILITY NLMDBG_MONITOR +#define XDR_ADDRBUF_LEN (20) + static struct rpc_clnt * nsm_create(void); static struct rpc_program nsm_program; @@ -158,6 +160,29 @@ static __be32 *xdr_encode_nsm_string(__be32 *p, char *string) return xdr_encode_opaque(p, string, len); } +/* + * "mon_name" specifies the host to be monitored. + * + * Linux uses a text version of the IP address of the remote + * host as the host identifier (the "mon_name" argument). + * + * Linux statd always looks up the canonical hostname first for + * whatever remote hostname it receives, so this works alright. + */ +static __be32 *xdr_encode_mon_name(__be32 *p, struct nsm_args *argp) +{ + char buffer[XDR_ADDRBUF_LEN + 1]; + char *name = argp->mon_name; + + if (!nsm_use_hostnames) { + snprintf(buffer, XDR_ADDRBUF_LEN, + NIPQUAD_FMT, NIPQUAD(argp->addr)); + name = buffer; + } + + return xdr_encode_nsm_string(p, name); +} + static __be32 * xdr_encode_common(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { -- cgit v1.2.3 From 850c95fd07b63704d06909c9a081c51272d32db1 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:25:46 -0400 Subject: lockd: refactor SM_MON my_id argument encoder Clean up: introduce a new XDR encoder specifically for the my_id argument of SM_MON requests. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/mon.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'fs/lockd') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 87bd4e4d65d..06216d6715f 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -183,6 +183,25 @@ static __be32 *xdr_encode_mon_name(__be32 *p, struct nsm_args *argp) return xdr_encode_nsm_string(p, name); } +/* + * The "my_id" argument specifies the hostname and RPC procedure + * to be called when the status manager receives notification + * (via the SM_NOTIFY call) that the state of host "mon_name" + * has changed. + */ +static __be32 *xdr_encode_my_id(__be32 *p, struct nsm_args *argp) +{ + p = xdr_encode_nsm_string(p, utsname()->nodename); + if (!p) + return ERR_PTR(-EIO); + + *p++ = htonl(argp->prog); + *p++ = htonl(argp->vers); + *p++ = htonl(argp->proc); + + return p; +} + static __be32 * xdr_encode_common(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { -- cgit v1.2.3 From ea72a7f170e686baf00ceee57b6197bef686889c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:25:53 -0400 Subject: lockd: document use of mon_id argument in SM_MON requests Clean up: document the argument type that xdr_encode_common() is marshalling by introducing a new function. The new function will replace xdr_encode_common() in just a sec. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/mon.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'fs/lockd') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 06216d6715f..3935d7b604f 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -202,6 +202,19 @@ static __be32 *xdr_encode_my_id(__be32 *p, struct nsm_args *argp) return p; } +/* + * The "mon_id" argument specifies the non-private arguments + * of an SM_MON or SM_UNMON call. + */ +static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) +{ + p = xdr_encode_mon_name(p, argp); + if (!p) + return ERR_PTR(-EIO); + + return xdr_encode_my_id(p, argp); +} + static __be32 * xdr_encode_common(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { -- cgit v1.2.3 From 2ca7754d4c96d68e1475690422a202ba5f0443d8 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:26:01 -0400 Subject: lockd: Fix up incorrect RPC buffer size calculations. Switch to using the new mon_id encoder function. Now that we've refactored the encoding of SM_MON requests, we've discovered that the pre-computed buffer length maximums are incorrect! Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/mon.c | 39 ++++++++------------------------------- 1 file changed, 8 insertions(+), 31 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 3935d7b604f..f2507fec7c6 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -149,6 +149,9 @@ nsm_create(void) /* * XDR functions for NSM. + * + * See http://www.opengroup.org/ for details on the Network + * Status Monitor wire protocol. */ static __be32 *xdr_encode_nsm_string(__be32 *p, char *string) @@ -215,37 +218,10 @@ static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) return xdr_encode_my_id(p, argp); } -static __be32 * -xdr_encode_common(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) -{ - char buffer[20], *name; - - /* - * Use the dotted-quad IP address of the remote host as - * identifier. Linux statd always looks up the canonical - * hostname first for whatever remote hostname it receives, - * so this works alright. - */ - if (nsm_use_hostnames) { - name = argp->mon_name; - } else { - sprintf(buffer, "%u.%u.%u.%u", NIPQUAD(argp->addr)); - name = buffer; - } - if (!(p = xdr_encode_string(p, name)) - || !(p = xdr_encode_string(p, utsname()->nodename))) - return ERR_PTR(-EIO); - *p++ = htonl(argp->prog); - *p++ = htonl(argp->vers); - *p++ = htonl(argp->proc); - - return p; -} - static int xdr_encode_mon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { - p = xdr_encode_common(rqstp, p, argp); + p = xdr_encode_mon_id(p, argp); if (IS_ERR(p)) return PTR_ERR(p); @@ -261,7 +237,7 @@ xdr_encode_mon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) static int xdr_encode_unmon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { - p = xdr_encode_common(rqstp, p, argp); + p = xdr_encode_mon_id(p, argp); if (IS_ERR(p)) return PTR_ERR(p); rqstp->rq_slen = xdr_adjust_iovec(rqstp->rq_svec, p); @@ -286,8 +262,9 @@ xdr_decode_stat(struct rpc_rqst *rqstp, __be32 *p, struct nsm_res *resp) } #define SM_my_name_sz (1+XDR_QUADLEN(SM_MAXSTRLEN)) -#define SM_my_id_sz (3+1+SM_my_name_sz) -#define SM_mon_id_sz (1+XDR_QUADLEN(20)+SM_my_id_sz) +#define SM_my_id_sz (SM_my_name_sz+3) +#define SM_mon_name_sz (1+XDR_QUADLEN(SM_MAXSTRLEN)) +#define SM_mon_id_sz (SM_mon_name_sz+SM_my_id_sz) #define SM_mon_sz (SM_mon_id_sz+4) #define SM_monres_sz 2 #define SM_unmonres_sz 1 -- cgit v1.2.3 From 0490a54a00c14212f22c5948c8c13a4553d745bd Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 14 Mar 2008 14:26:08 -0400 Subject: lockd: introduce new function to encode private argument in SM_MON requests Clean up: refactor the encoding of the opaque 16-byte private argument in xdr_encode_mon(). This will be updated later to support IPv6 addresses. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/lockd/mon.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index f2507fec7c6..e4d563543b1 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -218,6 +218,24 @@ static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) return xdr_encode_my_id(p, argp); } +/* + * The "priv" argument may contain private information required + * by the SM_MON call. This information will be supplied in the + * SM_NOTIFY call. + * + * Linux provides the raw IP address of the monitored host, + * left in network byte order. + */ +static __be32 *xdr_encode_priv(__be32 *p, struct nsm_args *argp) +{ + *p++ = argp->addr; + *p++ = 0; + *p++ = 0; + *p++ = 0; + + return p; +} + static int xdr_encode_mon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) { @@ -225,11 +243,10 @@ xdr_encode_mon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) if (IS_ERR(p)) return PTR_ERR(p); - /* Surprise - there may even be room for an IPv6 address now */ - *p++ = argp->addr; - *p++ = 0; - *p++ = 0; - *p++ = 0; + p = xdr_encode_priv(p, argp); + if (IS_ERR(p)) + return PTR_ERR(p); + rqstp->rq_slen = xdr_adjust_iovec(rqstp->rq_svec, p); return 0; } @@ -265,7 +282,8 @@ xdr_decode_stat(struct rpc_rqst *rqstp, __be32 *p, struct nsm_res *resp) #define SM_my_id_sz (SM_my_name_sz+3) #define SM_mon_name_sz (1+XDR_QUADLEN(SM_MAXSTRLEN)) #define SM_mon_id_sz (SM_mon_name_sz+SM_my_id_sz) -#define SM_mon_sz (SM_mon_id_sz+4) +#define SM_priv_sz (XDR_QUADLEN(SM_PRIV_SIZE)) +#define SM_mon_sz (SM_mon_id_sz+SM_priv_sz) #define SM_monres_sz 2 #define SM_unmonres_sz 1 -- cgit v1.2.3 From 4a9af59fee0701d9db99bc148d87b8852d6d6dd8 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 1 Apr 2008 18:57:06 -0400 Subject: NLM/lockd: Ensure we don't corrupt fl->fl_flags in nlmclnt_unlock() Also fix up nlmclnt_lock() so that it doesn't pass modified versions of fl->fl_flags to nlmclnt_cancel() and other helpers. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index b6b74a60e1e..4e1c0123b45 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -493,6 +493,7 @@ nlmclnt_lock(struct nlm_rqst *req, struct file_lock *fl) } fl->fl_flags |= FL_ACCESS; status = do_vfs_lock(fl); + fl->fl_flags = fl_flags; if (status < 0) goto out; @@ -530,10 +531,11 @@ again: goto again; } /* Ensure the resulting lock will get added to granted list */ - fl->fl_flags = fl_flags | FL_SLEEP; + fl->fl_flags |= FL_SLEEP; if (do_vfs_lock(fl) < 0) printk(KERN_WARNING "%s: VFS is out of sync with lock manager!\n", __FUNCTION__); up_read(&host->h_rwsem); + fl->fl_flags = fl_flags; } status = nlm_stat_to_errno(resp->status); out_unblock: @@ -543,7 +545,6 @@ out_unblock: nlmclnt_cancel(host, req->a_args.block, fl); out: nlm_release_call(req); - fl->fl_flags = fl_flags; return status; } @@ -598,7 +599,8 @@ nlmclnt_unlock(struct nlm_rqst *req, struct file_lock *fl) { struct nlm_host *host = req->a_host; struct nlm_res *resp = &req->a_res; - int status = 0; + int status; + unsigned char fl_flags = fl->fl_flags; /* * Note: the server is supposed to either grant us the unlock @@ -607,11 +609,13 @@ nlmclnt_unlock(struct nlm_rqst *req, struct file_lock *fl) */ fl->fl_flags |= FL_EXISTS; down_read(&host->h_rwsem); - if (do_vfs_lock(fl) == -ENOENT) { - up_read(&host->h_rwsem); + status = do_vfs_lock(fl); + up_read(&host->h_rwsem); + fl->fl_flags = fl_flags; + if (status == -ENOENT) { + status = 0; goto out; } - up_read(&host->h_rwsem); if (req->a_flags & RPC_TASK_ASYNC) return nlm_async_call(req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); -- cgit v1.2.3 From 5e7f37a76fa5b604949020b7317962262812b2dd Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 1 Apr 2008 18:58:49 -0400 Subject: NLM/lockd: Add a reference counter to struct nlm_rqst When we replace the existing synchronous RPC calls with asynchronous calls, the reference count will be needed in order to allow us to examine the result of the RPC call. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index 4e1c0123b45..749eb5328cb 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -221,6 +221,7 @@ struct nlm_rqst *nlm_alloc_call(struct nlm_host *host) for(;;) { call = kzalloc(sizeof(*call), GFP_KERNEL); if (call != NULL) { + atomic_set(&call->a_count, 1); locks_init_lock(&call->a_args.lock.fl); locks_init_lock(&call->a_res.lock.fl); call->a_host = host; @@ -237,6 +238,8 @@ struct nlm_rqst *nlm_alloc_call(struct nlm_host *host) void nlm_release_call(struct nlm_rqst *call) { + if (!atomic_dec_and_test(&call->a_count)) + return; nlm_release_host(call->a_host); nlmclnt_release_lockargs(call); kfree(call); -- cgit v1.2.3 From dc9d8d048168ff61c458bec06b28996cb90b182a Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 28 Mar 2008 16:04:36 -0400 Subject: NLM/lockd: convert __nlm_async_call to use rpc_run_task() Peter Staubach comments: > In the course of investigating testing failures in the locking phase of > the Connectathon testsuite, I discovered a couple of things. One was > that one of the tests in the locking tests was racy when it didn't seem > to need to be and two, that the NFS client asynchronously releases locks > when a process is exiting. ... > The Single UNIX Specification Version 3 specifies that: "All locks > associated with a file for a given process shall be removed when a file > descriptor for that file is closed by that process or the process holding > that file descriptor terminates.". > > This does not specify whether those locks must be released prior to the > completion of the exit processing for the process or not. However, > general assumptions seem to be that those locks will be released. This > leads to more deterministic behavior under normal circumstances. The following patch converts the NFSv2/v3 locking code to use the same mechanism as NFSv4 for sending asynchronous RPC calls and then waiting for them to complete. This ensures that the UNLOCK and CANCEL RPC calls will complete even if the user interrupts the call, yet satisfies the above request for synchronous behaviour on process exit. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 64 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 10 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index 749eb5328cb..a34b709006a 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -346,10 +346,16 @@ in_grace_period: /* * Generic NLM call, async version. */ -static int __nlm_async_call(struct nlm_rqst *req, u32 proc, struct rpc_message *msg, const struct rpc_call_ops *tk_ops) +static struct rpc_task *__nlm_async_call(struct nlm_rqst *req, u32 proc, struct rpc_message *msg, const struct rpc_call_ops *tk_ops) { struct nlm_host *host = req->a_host; struct rpc_clnt *clnt; + struct rpc_task_setup task_setup_data = { + .rpc_message = msg, + .callback_ops = tk_ops, + .callback_data = req, + .flags = RPC_TASK_ASYNC, + }; dprintk("lockd: call procedure %d on %s (async)\n", (int)proc, host->h_name); @@ -359,21 +365,36 @@ static int __nlm_async_call(struct nlm_rqst *req, u32 proc, struct rpc_message * if (clnt == NULL) goto out_err; msg->rpc_proc = &clnt->cl_procinfo[proc]; + task_setup_data.rpc_client = clnt; /* bootstrap and kick off the async RPC call */ - return rpc_call_async(clnt, msg, RPC_TASK_ASYNC, tk_ops, req); + return rpc_run_task(&task_setup_data); out_err: tk_ops->rpc_release(req); - return -ENOLCK; + return ERR_PTR(-ENOLCK); +} + +static int nlm_do_async_call(struct nlm_rqst *req, u32 proc, struct rpc_message *msg, const struct rpc_call_ops *tk_ops) +{ + struct rpc_task *task; + + task = __nlm_async_call(req, proc, msg, tk_ops); + if (IS_ERR(task)) + return PTR_ERR(task); + rpc_put_task(task); + return 0; } +/* + * NLM asynchronous call. + */ int nlm_async_call(struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *tk_ops) { struct rpc_message msg = { .rpc_argp = &req->a_args, .rpc_resp = &req->a_res, }; - return __nlm_async_call(req, proc, &msg, tk_ops); + return nlm_do_async_call(req, proc, &msg, tk_ops); } int nlm_async_reply(struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *tk_ops) @@ -381,7 +402,32 @@ int nlm_async_reply(struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *t struct rpc_message msg = { .rpc_argp = &req->a_res, }; - return __nlm_async_call(req, proc, &msg, tk_ops); + return nlm_do_async_call(req, proc, &msg, tk_ops); +} + +/* + * NLM client asynchronous call. + * + * Note that although the calls are asynchronous, and are therefore + * guaranteed to complete, we still always attempt to wait for + * completion in order to be able to correctly track the lock + * state. + */ +static int nlmclnt_async_call(struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *tk_ops) +{ + struct rpc_message msg = { + .rpc_argp = &req->a_args, + .rpc_resp = &req->a_res, + }; + struct rpc_task *task; + int err; + + task = __nlm_async_call(req, proc, &msg, tk_ops); + if (IS_ERR(task)) + return PTR_ERR(task); + err = rpc_wait_for_completion_task(task); + rpc_put_task(task); + return err; } /* @@ -620,10 +666,8 @@ nlmclnt_unlock(struct nlm_rqst *req, struct file_lock *fl) goto out; } - if (req->a_flags & RPC_TASK_ASYNC) - return nlm_async_call(req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); - - status = nlmclnt_call(req, NLMPROC_UNLOCK); + atomic_inc(&req->a_count); + status = nlmclnt_async_call(req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); if (status < 0) goto out; @@ -697,7 +741,7 @@ static int nlmclnt_cancel(struct nlm_host *host, int block, struct file_lock *fl nlmclnt_setlockargs(req, fl); req->a_args.block = block; - status = nlm_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); + status = nlmclnt_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); spin_lock_irqsave(¤t->sighand->siglock, flags); current->blocked = oldset; -- cgit v1.2.3 From 8ec7ff74448f65ac963e330795d771ab14ec8408 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 28 Mar 2008 16:04:51 -0400 Subject: NLM: Remove the signal masking in nlmclnt_proc/nlmclnt_cancel The signal masks have been rendered obsolete by the preceding patch. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 42 +----------------------------------------- 1 file changed, 1 insertion(+), 41 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index a34b709006a..5f13e0363b2 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -155,8 +155,6 @@ static void nlmclnt_release_lockargs(struct nlm_rqst *req) int nlmclnt_proc(struct nlm_host *host, int cmd, struct file_lock *fl) { struct nlm_rqst *call; - sigset_t oldset; - unsigned long flags; int status; nlm_get_host(host); @@ -168,22 +166,6 @@ int nlmclnt_proc(struct nlm_host *host, int cmd, struct file_lock *fl) /* Set up the argument struct */ nlmclnt_setlockargs(call, fl); - /* Keep the old signal mask */ - spin_lock_irqsave(¤t->sighand->siglock, flags); - oldset = current->blocked; - - /* If we're cleaning up locks because the process is exiting, - * perform the RPC call asynchronously. */ - if ((IS_SETLK(cmd) || IS_SETLKW(cmd)) - && fl->fl_type == F_UNLCK - && (current->flags & PF_EXITING)) { - sigfillset(¤t->blocked); /* Mask all signals */ - recalc_sigpending(); - - call->a_flags = RPC_TASK_ASYNC; - } - spin_unlock_irqrestore(¤t->sighand->siglock, flags); - if (IS_SETLK(cmd) || IS_SETLKW(cmd)) { if (fl->fl_type != F_UNLCK) { call->a_args.block = IS_SETLKW(cmd) ? 1 : 0; @@ -198,11 +180,6 @@ int nlmclnt_proc(struct nlm_host *host, int cmd, struct file_lock *fl) fl->fl_ops->fl_release_private(fl); fl->fl_ops = NULL; - spin_lock_irqsave(¤t->sighand->siglock, flags); - current->blocked = oldset; - recalc_sigpending(); - spin_unlock_irqrestore(¤t->sighand->siglock, flags); - dprintk("lockd: clnt proc returns %d\n", status); return status; } @@ -722,16 +699,6 @@ static const struct rpc_call_ops nlmclnt_unlock_ops = { static int nlmclnt_cancel(struct nlm_host *host, int block, struct file_lock *fl) { struct nlm_rqst *req; - unsigned long flags; - sigset_t oldset; - int status; - - /* Block all signals while setting up call */ - spin_lock_irqsave(¤t->sighand->siglock, flags); - oldset = current->blocked; - sigfillset(¤t->blocked); - recalc_sigpending(); - spin_unlock_irqrestore(¤t->sighand->siglock, flags); req = nlm_alloc_call(nlm_get_host(host)); if (!req) @@ -741,14 +708,7 @@ static int nlmclnt_cancel(struct nlm_host *host, int block, struct file_lock *fl nlmclnt_setlockargs(req, fl); req->a_args.block = block; - status = nlmclnt_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); - - spin_lock_irqsave(¤t->sighand->siglock, flags); - current->blocked = oldset; - recalc_sigpending(); - spin_unlock_irqrestore(¤t->sighand->siglock, flags); - - return status; + return nlmclnt_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); } static void nlmclnt_cancel_callback(struct rpc_task *task, void *data) -- cgit v1.2.3 From 6b4b3a752b3464f2fd9fe2837fb19270c23c1d6b Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 2 Apr 2008 14:40:53 -0400 Subject: NLM/lockd: Ensure that nlmclnt_cancel() returns results of the CANCEL call Currently, it returns success as long as the RPC call was sent. We'd like to know if the CANCEL operation succeeded on the server. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index 5f13e0363b2..ea1a6940af2 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -699,6 +699,10 @@ static const struct rpc_call_ops nlmclnt_unlock_ops = { static int nlmclnt_cancel(struct nlm_host *host, int block, struct file_lock *fl) { struct nlm_rqst *req; + int status; + + dprintk("lockd: blocking lock attempt was interrupted by a signal.\n" + " Attempting to cancel lock.\n"); req = nlm_alloc_call(nlm_get_host(host)); if (!req) @@ -708,7 +712,12 @@ static int nlmclnt_cancel(struct nlm_host *host, int block, struct file_lock *fl nlmclnt_setlockargs(req, fl); req->a_args.block = block; - return nlmclnt_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); + atomic_inc(&req->a_count); + status = nlmclnt_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); + if (status == 0 && req->a_res.status == nlm_lck_denied) + status = -ENOLCK; + nlm_release_call(req); + return status; } static void nlmclnt_cancel_callback(struct rpc_task *task, void *data) -- cgit v1.2.3 From 5f50c0c6d644d6c8180d9079c13c5d9de3adeb34 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 1 Apr 2008 20:26:22 -0400 Subject: NLM/lockd: Fix a race when cancelling a blocking lock We shouldn't remove the lock from the list of blocked locks until the CANCEL call has completed since we may be racing with a GRANTED callback. Also ensure that we send an UNLOCK if the CANCEL request failed. Normally that should only happen if the process gets hit with a fatal signal. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index ea1a6940af2..37d1aa20a60 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -510,6 +510,7 @@ nlmclnt_lock(struct nlm_rqst *req, struct file_lock *fl) struct nlm_res *resp = &req->a_res; struct nlm_wait *block = NULL; unsigned char fl_flags = fl->fl_flags; + unsigned char fl_type; int status = -ENOLCK; if (nsm_monitor(host) < 0) { @@ -525,13 +526,16 @@ nlmclnt_lock(struct nlm_rqst *req, struct file_lock *fl) block = nlmclnt_prepare_block(host, fl); again: + /* + * Initialise resp->status to a valid non-zero value, + * since 0 == nlm_lck_granted + */ + resp->status = nlm_lck_blocked; for(;;) { /* Reboot protection */ fl->fl_u.nfs_fl.state = host->h_state; status = nlmclnt_call(req, NLMPROC_LOCK); if (status < 0) - goto out_unblock; - if (!req->a_args.block) break; /* Did a reclaimer thread notify us of a server reboot? */ if (resp->status == nlm_lck_denied_grace_period) @@ -540,15 +544,22 @@ again: break; /* Wait on an NLM blocking lock */ status = nlmclnt_block(block, req, NLMCLNT_POLL_TIMEOUT); - /* if we were interrupted. Send a CANCEL request to the server - * and exit - */ if (status < 0) - goto out_unblock; + break; if (resp->status != nlm_lck_blocked) break; } + /* if we were interrupted while blocking, then cancel the lock request + * and exit + */ + if (resp->status == nlm_lck_blocked) { + if (!req->a_args.block) + goto out_unlock; + if (nlmclnt_cancel(host, req->a_args.block, fl) == 0) + goto out_unblock; + } + if (resp->status == nlm_granted) { down_read(&host->h_rwsem); /* Check whether or not the server has rebooted */ @@ -562,16 +573,30 @@ again: printk(KERN_WARNING "%s: VFS is out of sync with lock manager!\n", __FUNCTION__); up_read(&host->h_rwsem); fl->fl_flags = fl_flags; + status = 0; } + if (status < 0) + goto out_unlock; status = nlm_stat_to_errno(resp->status); out_unblock: nlmclnt_finish_block(block); - /* Cancel the blocked request if it is still pending */ - if (resp->status == nlm_lck_blocked) - nlmclnt_cancel(host, req->a_args.block, fl); out: nlm_release_call(req); return status; +out_unlock: + /* Fatal error: ensure that we remove the lock altogether */ + dprintk("lockd: lock attempt ended in fatal error.\n" + " Attempting to unlock.\n"); + nlmclnt_finish_block(block); + fl_type = fl->fl_type; + fl->fl_type = F_UNLCK; + down_read(&host->h_rwsem); + do_vfs_lock(fl); + up_read(&host->h_rwsem); + fl->fl_type = fl_type; + fl->fl_flags = fl_flags; + nlmclnt_async_call(req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); + return status; } /* -- cgit v1.2.3 From d11d10cc05c94a32632d6928d15a1034200dd9a5 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 2 Apr 2008 14:44:05 -0400 Subject: NLM/lockd: Ensure client locking calls use correct credentials Now that we've added the 'generic' credentials (that are independent of the rpc_client) to the nfs_open_context, we can use those in the NLM client to ensure that the lock/unlock requests are authenticated to whoever originally opened the file. Signed-off-by: Trond Myklebust --- fs/lockd/clntproc.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'fs/lockd') diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index 37d1aa20a60..40b16f23e49 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -247,7 +247,7 @@ static int nlm_wait_on_grace(wait_queue_head_t *queue) * Generic NLM call */ static int -nlmclnt_call(struct nlm_rqst *req, u32 proc) +nlmclnt_call(struct rpc_cred *cred, struct nlm_rqst *req, u32 proc) { struct nlm_host *host = req->a_host; struct rpc_clnt *clnt; @@ -256,6 +256,7 @@ nlmclnt_call(struct nlm_rqst *req, u32 proc) struct rpc_message msg = { .rpc_argp = argp, .rpc_resp = resp, + .rpc_cred = cred, }; int status; @@ -390,11 +391,12 @@ int nlm_async_reply(struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *t * completion in order to be able to correctly track the lock * state. */ -static int nlmclnt_async_call(struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *tk_ops) +static int nlmclnt_async_call(struct rpc_cred *cred, struct nlm_rqst *req, u32 proc, const struct rpc_call_ops *tk_ops) { struct rpc_message msg = { .rpc_argp = &req->a_args, .rpc_resp = &req->a_res, + .rpc_cred = cred, }; struct rpc_task *task; int err; @@ -415,7 +417,7 @@ nlmclnt_test(struct nlm_rqst *req, struct file_lock *fl) { int status; - status = nlmclnt_call(req, NLMPROC_TEST); + status = nlmclnt_call(nfs_file_cred(fl->fl_file), req, NLMPROC_TEST); if (status < 0) goto out; @@ -506,6 +508,7 @@ static int do_vfs_lock(struct file_lock *fl) static int nlmclnt_lock(struct nlm_rqst *req, struct file_lock *fl) { + struct rpc_cred *cred = nfs_file_cred(fl->fl_file); struct nlm_host *host = req->a_host; struct nlm_res *resp = &req->a_res; struct nlm_wait *block = NULL; @@ -534,7 +537,7 @@ again: for(;;) { /* Reboot protection */ fl->fl_u.nfs_fl.state = host->h_state; - status = nlmclnt_call(req, NLMPROC_LOCK); + status = nlmclnt_call(cred, req, NLMPROC_LOCK); if (status < 0) break; /* Did a reclaimer thread notify us of a server reboot? */ @@ -595,7 +598,7 @@ out_unlock: up_read(&host->h_rwsem); fl->fl_type = fl_type; fl->fl_flags = fl_flags; - nlmclnt_async_call(req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); + nlmclnt_async_call(cred, req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); return status; } @@ -619,8 +622,8 @@ nlmclnt_reclaim(struct nlm_host *host, struct file_lock *fl) nlmclnt_setlockargs(req, fl); req->a_args.reclaim = 1; - if ((status = nlmclnt_call(req, NLMPROC_LOCK)) >= 0 - && req->a_res.status == nlm_granted) + status = nlmclnt_call(nfs_file_cred(fl->fl_file), req, NLMPROC_LOCK); + if (status >= 0 && req->a_res.status == nlm_granted) return 0; printk(KERN_WARNING "lockd: failed to reclaim lock for pid %d " @@ -669,7 +672,8 @@ nlmclnt_unlock(struct nlm_rqst *req, struct file_lock *fl) } atomic_inc(&req->a_count); - status = nlmclnt_async_call(req, NLMPROC_UNLOCK, &nlmclnt_unlock_ops); + status = nlmclnt_async_call(nfs_file_cred(fl->fl_file), req, + NLMPROC_UNLOCK, &nlmclnt_unlock_ops); if (status < 0) goto out; @@ -738,7 +742,8 @@ static int nlmclnt_cancel(struct nlm_host *host, int block, struct file_lock *fl req->a_args.block = block; atomic_inc(&req->a_count); - status = nlmclnt_async_call(req, NLMPROC_CANCEL, &nlmclnt_cancel_ops); + status = nlmclnt_async_call(nfs_file_cred(fl->fl_file), req, + NLMPROC_CANCEL, &nlmclnt_cancel_ops); if (status == 0 && req->a_res.status == nlm_lck_denied) status = -ENOLCK; nlm_release_call(req); -- cgit v1.2.3