[PATCH] nfsd + scalability

[Mark Hemment <markhe@veritas.com>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 682 bytes --]

Hi Neil, all,

  The nfs daemons run holding the global kernel lock.  They still hold
this lock over calls to file_op's read and write.

  The file system kernel interface (FSKI) doesn't require the kernel lock
to be held over these read/write calls.  The nfs daemons do not require 
that the reads or writes do not block (would be v silly if they did), so
they have no guarantee the lock isn't dropped and retaken during
blocking.  ie. they aren't using it as a guard across the calls.

  Dropping the kernel lock around read and write in fs/nfsd/vfs.c is a
_big_ SMP scalability win!

  Attached patch is against 2.4.1-ac18, but should apply to most recent
kernel versions.

Mark

[-- Attachment #2: nfsd.patch --]
[-- Type: TEXT/PLAIN, Size: 2247 bytes --]

--- vanilla-2.4.1-ac18/fs/nfsd/vfs.c	Sun Feb 18 15:06:27 2001
+++ markhe-2.4.1-ac18/fs/nfsd/vfs.c	Sun Feb 18 19:32:18 2001
@@ -30,6 +30,7 @@
 #include <linux/net.h>
 #include <linux/unistd.h>
 #include <linux/slab.h>
+#include <linux/smp_lock.h>
 #include <linux/in.h>
 #define __NO_VERSION__
 #include <linux/module.h>
@@ -602,12 +603,28 @@
 		file.f_ralen = ra->p_ralen;
 		file.f_rawin = ra->p_rawin;
 	}
+
+	/*
+	 * The nfs daemons run holding the global kernel lock, but
+	 * f_op->read() doesn't need the lock to be held.
+	 * Drop it here to help scalability.
+	 *
+	 * The "kernel_locked()" test isn't perfect (someone else could be
+	 * holding the lock when we're not), but it will eventually catch
+	 * any cases of entering here without the lock held.
+	 */
+	if (!kernel_locked())
+		BUG();
+	unlock_kernel();
+
 	file.f_pos = offset;
 
 	oldfs = get_fs(); set_fs(KERNEL_DS);
 	err = file.f_op->read(&file, buf, *count, &file.f_pos);
 	set_fs(oldfs);
 
+	lock_kernel();
+
 	/* Write back readahead params */
 	if (ra != NULL) {
 		dprintk("nfsd: raparms %ld %ld %ld %ld %ld\n",
@@ -664,6 +681,22 @@
 		goto out_close;
 #endif
 
+	/*
+	 * The nfs daemons run holding the global kernel lock, but
+	 * f_op->write() doesn't need the lock to be held.
+	 * Also, as the struct file is private, the export is read-locked,
+	 * and the inode attached to the dentry cannot change under us, the
+	 * lock can be dropped ahead of the call to write() for even better
+	 * scalability.
+	 *
+	 * The "kernel_locked()" test isn't perfect (someone else could be
+	 * holding the lock when we're not), but it will eventually catch
+	 * any cases of entering here without the lock held.
+	 */
+	if (!kernel_locked())
+		BUG();
+	unlock_kernel();
+
 	dentry = file.f_dentry;
 	inode = dentry->d_inode;
 	exp   = fhp->fh_export;
@@ -692,9 +725,12 @@
 	/* Write the data. */
 	oldfs = get_fs(); set_fs(KERNEL_DS);
 	err = file.f_op->write(&file, buf, cnt, &file.f_pos);
+	set_fs(oldfs);
+
+	lock_kernel();
+
 	if (err >= 0)
 		nfsdstats.io_write += cnt;
-	set_fs(oldfs);
 
 	/* clear setuid/setgid flag after write */
 	if (err >= 0 && (inode->i_mode & (S_ISUID | S_ISGID))) {