On Tuesday 19 August 2014 03:12 AM, Abhijit Dey wrote: > Bruce > > Your change looks appropriate. Please go ahead. > > I'd make that: > > "This option will disable READDIRPLUS request handling. When > set, READDIRPLUS requests from NFS clients return > NFS3ERR_NOTSUPP, and clients fall back on READDIR. This option > affects only NFSv3 clients." > > Regards > Abhijit > > > -----Original Message----- > From: J. Bruce Fields [mailto:bfields@xxxxxxxxxxxx] > Sent: Monday, August 18, 2014 2:19 PM > To: Rajesh Ghanekar > Cc: Steve Dickson; Rishi Agrawal; linux-nfs@xxxxxxxxxxxxxxx; Ram Pandiri; Sreeharsha Sarabu; Abhijit Dey; Tushar Shinde; bfields@xxxxxxxxxx > Subject: Re: [PATCH] nfsd: allow turning off nfsv3 readdir_plus > > On Mon, Aug 18, 2014 at 11:06:03AM -0700, Rajesh Ghanekar wrote: >> diff -uprN nfs-utils-1.3.0.old/utils/exportfs/exports.man nfs-utils-1.3.0/utils/exportfs/exports.man >> --- nfs-utils-1.3.0.old/utils/exportfs/exports.man 2014-03-25 20:42:07.000000000 +0530 >> +++ nfs-utils-1.3.0/utils/exportfs/exports.man 2014-08-18 22:27:23.360261358 +0530 >> @@ -360,6 +360,13 @@ supported so the same configuration can >> kernels alike. >> >> .TP >> +.IR nordirplus >> +This option will allow disabling READDIRPLUS request handling. >> +When enabled, READDIRPLUS requests from NFS client will be returned >> +with "not supported" reply. Most of the NFS client implementations >> +starts to use READDIR request if READDIRPLUS is returned with >> +"not supported" reply. This option is only applicable for NFSv3. > I'd make that: > > "This option will disable READDIRPLUS request handling. When > set, READDIRPLUS requests from NFS clients return > NFS3ERR_NOTSUPP, and clients fall back on READDIR. This option > affects only NFSv3 clients." > > --b. Okay. From: Rajesh Ghanekar <Rajesh_Ghanekar@xxxxxxxxxxxx> 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. There's already a corresponding client-side mount option, but an export option reduces the need for configuration across multiple clients. This flag affects NFSv3 only. If it turns out it's needed for NFSv4 as well then we may have to figure out how to extend the behavior to NFSv4, but it's not currently obvious how to do that. ----- Signed-off-by: Rajesh Ghanekar <rajesh_ghanekar@xxxxxxxxxxxx> diff -uprN nfs-utils-1.3.0.old/support/include/nfs/export.h nfs-utils-1.3.0/support/include/nfs/export.h --- nfs-utils-1.3.0.old/support/include/nfs/export.h 2014-03-25 20:42:07.000000000 +0530 +++ nfs-utils-1.3.0/support/include/nfs/export.h 2014-08-18 22:28:24.420262810 +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 -uprN nfs-utils-1.3.0.old/support/nfs/exports.c nfs-utils-1.3.0/support/nfs/exports.c --- nfs-utils-1.3.0.old/support/nfs/exports.c 2014-03-25 20:42:07.000000000 +0530 +++ nfs-utils-1.3.0/support/nfs/exports.c 2014-08-18 22:28:24.600262814 +0530 @@ -273,6 +273,8 @@ putexportent(struct exportent *ep) "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 @@ parseopts(char *cp, struct exportent *ep 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")) diff -uprN nfs-utils-1.3.0.old/utils/exportfs/exports.man nfs-utils-1.3.0/utils/exportfs/exports.man --- nfs-utils-1.3.0.old/utils/exportfs/exports.man 2014-03-25 20:42:07.000000000 +0530 +++ nfs-utils-1.3.0/utils/exportfs/exports.man 2014-08-19 12:32:30.498780854 +0530 @@ -360,6 +360,11 @@ supported so the same configuration can kernels alike. .TP +.IR nordirplus +This option will disable READDIRPLUS request handling. When set, +READDIRPLUS requests from NFS clients return NFS3ERR_NOTSUPP, and +clients fall back on READDIR. This option affects only NFSv3 clients. +.TP .IR refer= path@host[+host][:path@host[+host]] A client referencing the export point will be directed to choose from the given list an alternative location for the filesystem. ----- Thanks, Rajesh > >> +.TP >> .IR refer= path@host[+host][:path@host[+host]] >> A client referencing the export point will be directed to choose from >> the given list an alternative location for the filesystem. >> >> ----- >> >> Thanks, >> Rajesh >> >> >> -----Original Message----- >> From: linux-nfs-owner@xxxxxxxxxxxxxxx [mailto:linux-nfs-owner@xxxxxxxxxxxxxxx] On Behalf Of Rajesh Ghanekar >> Sent: Monday, August 18, 2014 11:17 PM >> To: J. Bruce Fields; Steve Dickson >> Cc: Rishi Agrawal; linux-nfs@xxxxxxxxxxxxxxx; Ram Pandiri; Sreeharsha Sarabu; Abhijit Dey; Tushar Shinde; bfields@xxxxxxxxxx >> Subject: RE: [PATCH] nfsd: allow turning off nfsv3 readdir_plus >> >> Hi Bruce, Steve, >> Here is the nfs-utils patch reworked. Sorry for top posting, though. >> We had to wait for internal legal approval to complete, and hence got delayed. Same "signed-off" by signature can go to nfsd kernel patch which you have reworked. Please let me know if I need to resend (copy from your mail with signed-off added) nfsd kernel patch. >> >> Signed-off-by: Rajesh Ghanekar <rajesh_ghanekar@xxxxxxxxxxxx> >> >> >> diff -uprN nfs-utils-1.3.0.old/support/include/nfs/export.h nfs-utils-1.3.0/support/include/nfs/export.h >> --- nfs-utils-1.3.0.old/support/include/nfs/export.h 2014-03-25 20:42:07.000000000 +0530 >> +++ nfs-utils-1.3.0/support/include/nfs/export.h 2014-08-18 22:28:24.420262810 +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 -uprN nfs-utils-1.3.0.old/support/nfs/exports.c nfs-utils-1.3.0/support/nfs/exports.c >> --- nfs-utils-1.3.0.old/support/nfs/exports.c 2014-03-25 20:42:07.000000000 +0530 >> +++ nfs-utils-1.3.0/support/nfs/exports.c 2014-08-18 22:28:24.600262814 +0530 >> @@ -273,6 +273,8 @@ putexportent(struct exportent *ep) >> "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 @@ parseopts(char *cp, struct exportent *ep >> 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")) >> diff -uprN nfs-utils-1.3.0.old/utils/exportfs/exports.man nfs-utils-1.3.0/utils/exportfs/exports.man >> --- nfs-utils-1.3.0.old/utils/exportfs/exports.man 2014-03-25 20:42:07.000000000 +0530 >> +++ nfs-utils-1.3.0/utils/exportfs/exports.man 2014-08-18 22:27:23.360261358 +0530 >> @@ -360,6 +360,13 @@ supported so the same configuration can kernels alike. >> >> .TP >> +.IR nordirplus >> +This option will allow disabling READDIRPLUS request handling. >> +When enabled, READDIRPLUS requests from NFS client will be returned >> +with "not supported" reply. Most of the NFS client implementations >> +starts to use READDIR request if READDIRPLUS is returned with "not >> +supported" reply. This option is only applicable for NFSv3. >> +.TP >> .IR refer= path@host[+host][:path@host[+host]] >> A client referencing the export point will be directed to choose from the given list an alternative location for the filesystem. >> >> >> Thanks, >> Rajesh >> >> -----Original Message----- >> From: J. Bruce Fields [mailto:bfields@xxxxxxxxxxxx] >> Sent: Tuesday, August 05, 2014 11:52 PM >> To: Steve Dickson >> Cc: Rishi Agrawal; linux-nfs@xxxxxxxxxxxxxxx; Rajesh Ghanekar; Ram Pandiri; Sreeharsha Sarabu; Abhijit Dey; Tushar Shinde; bfields@xxxxxxxxxx >> Subject: Re: [PATCH] nfsd: allow turning off nfsv3 readdir_plus >> >> On Mon, Aug 04, 2014 at 05:46:47PM -0400, J. Bruce Fields wrote: >>> On Mon, Aug 04, 2014 at 11:24:11AM -0400, bfields wrote: >>>> +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; >>>> + } >>> Actually, this isn't right: errors from fh_verify should be returned >>> to the client or weird things could happen (e.g. what should have been >>> a transient DELAY error could result in the client turning off >>> readdirplus). >> Apologies, I misread: as the comment above notes, it falls back on allowing readdirplus when this fails, so I don't think there's a real bug here. >> >>> And MAY_READ is more than nfsd_readdir actually asks for, I think, >>> probably should just be MAY_NOP here. >>> >>> I'll fix that up.--b. >> But it's probably still better to return the fh_verify error on failure, as follows. >> >> --b. >> >> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c index 72ffd7cce3c3..30a739d896ff 100644 >> --- a/fs/nfsd/export.c >> +++ b/fs/nfsd/export.c >> @@ -1145,6 +1145,7 @@ static struct flags { >> { 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 --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index fa2525b2e9d7..247b06fb400d 100644 >> --- a/fs/nfsd/nfs3proc.c >> +++ b/fs/nfsd/nfs3proc.c >> @@ -471,6 +471,14 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp, struct nfsd3_readdirargs *argp, >> resp->buflen = resp->count; >> resp->rqstp = rqstp; >> offset = argp->cookie; >> + >> + nfserr = fh_verify(rqstp, &resp->fh, S_IFDIR, NFSD_MAY_NOP); >> + if (nfserr) >> + RETURN_STATUS(nfserr); >> + >> + if (resp->fh.fh_export->ex_flags & NFSEXP_NOREADDIRPLUS) >> + RETURN_STATUS(nfserr_notsupp); >> + >> nfserr = nfsd_readdir(rqstp, &resp->fh, >> &offset, >> &resp->common, >> diff --git a/include/uapi/linux/nfsd/export.h b/include/uapi/linux/nfsd/export.h >> index cf47c313794e..584b6ef3a5e8 100644 >> --- a/include/uapi/linux/nfsd/export.h >> +++ b/include/uapi/linux/nfsd/export.h >> @@ -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 \ >> -- >> 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 -- 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
