Re: [PATCH 04/12] lockd: unmonitor only when last host goes away

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux