NLM failover - nlm_unlock

By writing exported filesytem id into /proc/fs/nfsd/nlm_unlock, this patch walks thru lockd's global nlm_files list to release all the locks associated with the particular id. It is used to enable NFS lock failover with active-active clustered servers.
[...]
static int

(quoted diff) +3 -0

@@ -334,6 +348,32 @@ static ssize_t write_filehandle(struct f
 	return mesg - buf;	
 }
 
+static ssize_t do_nlm_fo_unlock(struct file *file, char *buf, size_t size)
+{
+	char *mesg = buf;

G'day Wendy,

Your fsid-based approach does seem to make dropping locks less fiddly than using the virtual server address. Some minor nits...

Quoted 6 lines

By writing exported filesytem id into /proc/fs/nfsd/nlm_unlock, this patch walks thru lockd's global nlm_files list to release all the locks associated with the particular id. It is used to enable NFS lock failover with active-active clustered servers.
[...]
static int

Review

Quoted 10 lines

(quoted diff) +3 -2

-nlm_traverse_files(struct nlm_host *host, int action)
+nlm_traverse_files(struct nlm_host *host, int *fsid_p, int action)
[...]
 {
 	struct nlm_file	*file, **fp;
-	int		i;
+	int		i, rc, fsid, act=action;
 
 	mutex_lock(&nlm_file_mutex);
+	if (fsid_p) fsid = *fsid_p;

I don't see any point initialising fsid like this, as you throw away that value before ever using it. Initialising to zero should be enough to stop the compiler complaining.

Review

Quoted 2 lines

(quoted diff) +2 -0

+				dprintk("lockd: drop lock file=0x%x fsid=%d\n",
+					(int) file, fsid);

`file' is a pointer, and will not be the same size as an int on 64bit machines. This is why %p exists, see examples in the existing NLM code.

Review nlm_traverse_files(struct nlm_host *host · line 311

Quoted 6 lines

(quoted diff) +2 -0

@@ -253,6 +307,8 @@ nlm_traverse_files(struct nlm_host *host
 			/* No more references to this file. Let go of it. */
 			if (!file->f_blocks && !file->f_locks
 			 && !file->f_shares && !file->f_count) {
+				dprintk("lockd: fo_unlock close file=0x%x\n", 
+					(int) file);

Ditto.

Review static ssize_t (*write_op[])(struct file · line 109

Quoted 8 lines

(quoted diff) +1 -0

@@ -90,6 +103,7 @@ static ssize_t (*write_op[])(struct file
 	[NFSD_Getfd] = write_getfd,
 	[NFSD_Getfs] = write_getfs,
 	[NFSD_Fh] = write_filehandle,
+	[NFSD_Nlm_unlock] = do_nlm_fo_unlock,
 	[NFSD_Threads] = write_threads,
 	[NFSD_Versions] = write_versions,
 #ifdef CONFIG_NFSD_V4

All the other entries in write_op[] have a consistent naming scheme, being called write_foo().

Quoted 7 lines

(quoted diff) +3 -0

@@ -334,6 +348,32 @@ static ssize_t write_filehandle(struct f
 	return mesg - buf;	
 }
 
+static ssize_t do_nlm_fo_unlock(struct file *file, char *buf, size_t size)
+{
+	char *mesg = buf;

Review static ssize_t write_filehandle(struct f · line 363

Quoted 10 lines

(quoted diff) +10 -0

+	int fsid, rc;
+
+	if (size <= 0) return -EINVAL;
+
+	/* convert string into a valid fsid */
+	rc = get_int(&mesg, &fsid);
+	if (rc) {
+		dprintk("nfsd: do_nlm_ip_unlock invalid ip(%s)\n", buf);
+		return rc;
+	}

This dprintk seems to reflect the function's previous name and semantics.

Review static ssize_t write_filehandle(struct f · line 366

Quoted 3 lines

(quoted diff) +3 -0

+
+	/* call nlm to release the locks */
+	rc = nlmsvc_fo_unlock(&fsid);

I don't understand why you pass this parameter by reference? It's not large and none of the functions called by nlmsvc_fo_unlock() need to write to it.

include/linux/nfsd/debug.h line 38

Quoted 11 lines

(quoted diff) +0 -0

 #ifdef CONFIG_NFSD_V4

include/linux/nfsd/debug.h +1 -0

@@ -32,6 +32,7 @@
 #define NFSDDBG_REPCACHE	0x0080
 #define NFSDDBG_XDR		0x0100
 #define NFSDDBG_LOCKD		0x0200
+#define NFSDDBG_CLUSTER 	0x0400
 #define NFSDDBG_ALL		0x7FFF
 #define NFSDDBG_NOCHANGE	0xFFFF
 

You don't seem to use this anywhere.

Greg.