On Nov 5, 2008, at 3:06 PM, J. Bruce Fields wrote:
The sm_sticky logic seems racy;
Can you describe what races you suspect?
instead just unmonitor before destruction of handle.
XXX: how to test?
Would this allow the nsm_handle cache to grow large on long-running
hosts?
Instead of sm_sticky, maybe we should put a "don't unmonitor" flag in
the nlm_host structure instead.
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
---
fs/lockd/host.c | 24 ++++++++++++++++++++----
fs/lockd/mon.c | 15 +--------------
fs/lockd/svcsubs.c | 6 ------
include/linux/lockd/lockd.h | 3 +--
4 files changed, 22 insertions(+), 26 deletions(-)
diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index e9069c2..1a1adb9 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -683,6 +683,10 @@ found:
return nsm;
}
+
+/* XXX: */
+int nsm_mon_unmon(struct nsm_handle *, u32, struct nsm_res *);
+
/*
* Release an NSM handle
*/
@@ -691,9 +695,21 @@ nsm_release(struct nsm_handle *nsm)
{
if (!nsm)
return;
- if (atomic_dec_and_lock(&nsm->sm_count, &nsm_lock)) {
- list_del(&nsm->sm_link);
- spin_unlock(&nsm_lock);
- kfree(nsm);
+ if (!atomic_dec_and_lock(&nsm->sm_count, &nsm_lock))
+ return;
+ list_del(&nsm->sm_link);
+ spin_unlock(&nsm_lock);
+ if (nsm->sm_monitored) {
+ struct nsm_res res;
+ int status;
+
+ dprintk("lockd: nsm_unmonitor(%s)\n", nsm->sm_name);
+ status = nsm_mon_unmon(nsm, SM_UNMON, &res);
+ if (status < 0)
+ printk(KERN_NOTICE "lockd: cannot unmonitor %s\n",
+ nsm->sm_name);
+ else
+ nsm->sm_monitored = 0;
}
+ kfree(nsm);
}
diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index 480f197..8cc0e97 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -32,7 +32,7 @@ int nsm_local_state;
/*
* Common procedure for SM_MON/SM_UNMON calls
*/
-static int
+int
nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res)
{
struct rpc_clnt *clnt;
@@ -100,24 +100,11 @@ nsm_monitor(struct nlm_host *host)
void nsm_unmonitor(struct nlm_host *host)
{
struct nsm_handle *nsm = host->h_nsmhandle;
- struct nsm_res res;
- int status;
if (nsm == NULL)
return;
host->h_nsmhandle = NULL;
- if (atomic_read(&nsm->sm_count) == 1
- && nsm->sm_monitored && !nsm->sm_sticky) {
- dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name);
-
- status = nsm_mon_unmon(nsm, SM_UNMON, &res);
- if (status < 0)
- printk(KERN_NOTICE "lockd: cannot unmonitor %s\n",
- host->h_name);
- else
- nsm->sm_monitored = 0;
- }
nsm_release(nsm);
}
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index 9c186f0..0296904 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -336,12 +336,6 @@ nlmsvc_is_client(void *data, struct nlm_host
*dummy)
struct nlm_host *host = data;
BUG_ON(!host->h_server);
- /* we are destroying locks even though the client
- * hasn't asked us too, so don't unmonitor the
- * client
- */
- if (host->h_nsmhandle)
- host->h_nsmhandle->sm_sticky = 1;
return 1;
}
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index b56d5aa..e1c2690 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -75,8 +75,7 @@ struct nsm_handle {
char * sm_name;
struct sockaddr_storage sm_addr;
size_t sm_addrlen;
- unsigned int sm_monitored : 1,
- sm_sticky : 1; /* don't unmonitor */
+ unsigned int sm_monitored : 1;
char sm_addrbuf[48]; /* address eyecatcher */
};
--
1.5.5.rc1
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
