Patch For Making Readdir_plus configurable

From: Rishi Agrawal <Rishi_Agrawal@symantec.com>
To: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Cc: Rajesh Ghanekar <Rajesh_Ghanekar@symantec.com>,
	Ram Pandiri <ram_pandiri@symantec.com>,
	Sreeharsha Sarabu <Sreeharsha_Sarabu@symantec.com>,
	Abhijit Dey <Abhijit_Dey@symantec.com>,
	Tushar Shinde <Tushar_Shinde@symantec.com>,
	"bfields@redhat.com" <bfields@redhat.com>,
	"steved@redhat.com" <steved@redhat.com>
Subject: Patch For Making Readdir_plus configurable
Date: Fri, 25 Jul 2014 09:19:25 -0700	[thread overview]
Message-ID: <20AEB6A025F81A4288597093171D1B5719CF5813D2@APJ1XCHEVSPIN35.SYMC.SYMANTEC.COM> (raw)

[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]

Hi,
   One of our customer's application only needs file names not
file attributes. With directories having 10K+ inodes (assuming buffer
cache has directory blocks cached having file names, but inode
cache is limited and hence need eviction of older cached inodes),
older inodes are evicted periodically. So if they keep on doing
readdir(2) from NSF client on multiple directories, some directory's
files are periodically removed from inode cache and hence new
readdir(2) on same directory requires disk access to bring back
inodes again to inode cache.

As READDIRPLUS request fetches attributes also, doing getattr on
each file on server, it causes unnecessary disk accesses. If
READDIRPLUS on NFS client is returned with -ENOTSUPP, NFS
client uses READDIR request which just gets the names of the files
in a directory, not attributes, hence avoiding disk accesses on server.

Can this be per-export tunable so that different application can
use it in different ways, some supporting readdirplus and others
not supporting readdirplus?

I have worked on a patch against 3.15.6 for this. Also I am
adding a patch against latest nfs-utils to have an export option
for this. Can someone let me know if these patches are adequate
or if any other changes are required. Patches attached, although
with minimal testing. I would like to know if something like this
can be added and if the patch can be reworked if I am missing
something for NFSv4.


Regards,
Rishi Agrawal



[-- Attachment #2: patch_linux-kernel-3-15-6 --]
[-- Type: application/octet-stream, Size: 2982 bytes --]

diff -r -u linux-3.15.6/fs/nfsd/export.c patched_linux-3.15.6/fs/nfsd/export.c

--- linux-3.15.6/fs/nfsd/export.c	2014-07-18 04:53:31.000000000 +0530
+++ patched_linux-3.15.6/fs/nfsd/export.c	2014-07-24 20:11:34.576599619 +0530
@@ -1123,6 +1123,7 @@
 	{ NFSEXP_ALLSQUASH, {"all_squash", ""}},
 	{ NFSEXP_ASYNC, {"async", "sync"}},
 	{ NFSEXP_GATHERED_WRITES, {"wdelay", "no_wdelay"}},
+	{ NFSEXP_NOREADDIRPLUS, {"nordirplus", ""}},
 	{ NFSEXP_NOHIDE, {"nohide", ""}},
 	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
 	{ NFSEXP_NOSUBTREECHECK, {"no_subtree_check", ""}},
diff -r -u linux-3.15.6/fs/nfsd/nfs3proc.c patched_linux-3.15.6/fs/nfsd/nfs3proc.c
--- linux-3.15.6/fs/nfsd/nfs3proc.c	2014-07-18 04:53:31.000000000 +0530
+++ patched_linux-3.15.6/fs/nfsd/nfs3proc.c	2014-07-24 20:20:06.650599709 +0530
@@ -446,6 +446,26 @@
 	RETURN_STATUS(nfserr);
 }
 
+static int
+nfsd3_is_readdirplus_supported(struct svc_rqst *rqstp, struct svc_fh *fhp)
+{
+	struct svc_export *exp;
+	int supported = 1; /* fall back to readdirplus supported in case of errors.*/
+	int err;
+
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_READ);
+	if (err) {
+		goto out;
+	}
+
+	exp = fhp->fh_export;
+	if (exp->ex_flags & NFSEXP_NOREADDIRPLUS) {
+		supported = 0;
+	}
+out:
+	return supported;
+}
+
 /*
  * Read a portion of a directory, including file handles and attrs.
  * For now, we choose to ignore the dircount parameter.
@@ -476,10 +496,16 @@
 	resp->buflen = resp->count;
 	resp->rqstp = rqstp;
 	offset = argp->cookie;
-	nfserr = nfsd_readdir(rqstp, &resp->fh,
-				     &offset,
-				     &resp->common,
-				     nfs3svc_encode_entry_plus);
+
+	if (nfsd3_is_readdirplus_supported(rqstp, &resp->fh)) {
+		nfserr = nfsd_readdir(rqstp, &resp->fh,
+				&offset,
+				&resp->common,
+				nfs3svc_encode_entry_plus);
+	} else {
+		nfserr = nfserrno(-EOPNOTSUPP);
+	}
+
 	memcpy(resp->verf, argp->verf, 8);
 	for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) {
 		page_addr = page_address(*p);
diff -r -u linux-3.15.6/include/uapi/linux/nfsd/export.h patched_linux-3.15.6/include/uapi/linux/nfsd/export.h
--- linux-3.15.6/include/uapi/linux/nfsd/export.h	2014-07-18 04:53:31.000000000 +0530
+++ patched_linux-3.15.6/include/uapi/linux/nfsd/export.h	2014-07-24 20:17:57.845599993 +0530
@@ -28,7 +28,8 @@
 #define NFSEXP_ALLSQUASH	0x0008
 #define NFSEXP_ASYNC		0x0010
 #define NFSEXP_GATHERED_WRITES	0x0020
-/* 40 80 100 currently unused */
+#define NFSEXP_NOREADDIRPLUS    0x0040
+/* 80 100 currently unused */
 #define NFSEXP_NOHIDE		0x0200
 #define NFSEXP_NOSUBTREECHECK	0x0400
 #define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests - just trust */
@@ -47,7 +48,7 @@
  */
 #define	NFSEXP_V4ROOT		0x10000
 /* All flags that we claim to support.  (Note we don't support NOACL.) */
-#define NFSEXP_ALLFLAGS		0x17E3F
+#define NFSEXP_ALLFLAGS		0x1FE7F
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \

[-- Attachment #3: patch_nfs-utils-1.3 --]
[-- Type: application/octet-stream, Size: 1446 bytes --]

diff -r -u nfs-utils-1.3.0/support/include/nfs/export.h patched_nfs-utils-1.3.0/support/include/nfs/export.h
--- nfs-utils-1.3.0/support/include/nfs/export.h	2014-03-25 20:42:07.000000000 +0530
+++ patched_nfs-utils-1.3.0/support/include/nfs/export.h	2014-07-22 17:41:26.023782985 +0530
@@ -17,7 +17,8 @@
 #define NFSEXP_ALLSQUASH	0x0008
 #define NFSEXP_ASYNC		0x0010
 #define NFSEXP_GATHERED_WRITES	0x0020
-/* 40, 80, 100 unused */
+#define NFSEXP_NOREADDIRPLUS	0x0040
+/* 80, 100 unused */
 #define NFSEXP_NOHIDE		0x0200
 #define NFSEXP_NOSUBTREECHECK	0x0400
 #define NFSEXP_NOAUTHNLM	0x0800
diff -r -u nfs-utils-1.3.0/support/nfs/exports.c patched_nfs-utils-1.3.0/support/nfs/exports.c
--- nfs-utils-1.3.0/support/nfs/exports.c	2014-03-25 20:42:07.000000000 +0530
+++ patched_nfs-utils-1.3.0/support/nfs/exports.c	2014-07-22 17:44:21.532782882 +0530
@@ -273,6 +273,8 @@
 		"in" : "");
 	fprintf(fp, "%sacl,", (ep->e_flags & NFSEXP_NOACL)?
 		"no_" : "");
+	if (ep->e_flags & NFSEXP_NOREADDIRPLUS)
+		fprintf(fp, "nordirplus,");
 	if (ep->e_flags & NFSEXP_FSID) {
 		fprintf(fp, "fsid=%d,", ep->e_fsid);
 	}
@@ -539,6 +541,8 @@
 			clearflags(NFSEXP_ASYNC, active, ep);
 		else if (!strcmp(opt, "async"))
 			setflags(NFSEXP_ASYNC, active, ep);
+		else if (!strcmp(opt, "nordirplus"))
+			setflags(NFSEXP_NOREADDIRPLUS, active, ep);
 		else if (!strcmp(opt, "nohide"))
 			setflags(NFSEXP_NOHIDE, active, ep);
 		else if (!strcmp(opt, "hide"))
