From: Bob Bell <[email protected]>
Add NFS_MOUNT_NONEGDE mount flag. When this flag is set on a mounted
NFS filesystem, negative dentries for that filesystem will not be
cached. This can help address cache coherency when the NFS server does
not provide sufficiently fine-grained timestamps to consistently
distinguish when a directory has been modified, though performance when
repeatedly accessing filenames for files that do not exist may be
impacted.
Signed-off-by: Bob Bell <[email protected]>
---
fs/nfs/dir.c | 5 +++++
fs/nfs/nfsroot.c | 10 +++++++++-
fs/nfs/super.c | 9 +++++++++
include/linux/nfs_mount.h | 35 ++++++++++++++++++-----------------
4 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 410449d..9419f55 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -851,6 +851,11 @@ static int nfs_dentry_delete(struct dentry *dentry)
* files will be cleaned up during umount */
return 1;
}
+ if (dentry->d_inode == NULL &&
+ (NFS_SB(dentry->d_sb)->flags & NFS_MOUNT_NONEGDE)) {
+ /* Unhash it, so that negative dentries won't be cached */
+ return 1;
+ }
return 0;
}
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c
index 4b03345..1c8e7ee 100644
--- a/fs/nfs/nfsroot.c
+++ b/fs/nfs/nfsroot.c
@@ -124,7 +124,7 @@ enum {
Opt_soft, Opt_hard, Opt_intr,
Opt_nointr, Opt_posix, Opt_noposix, Opt_cto, Opt_nocto, Opt_ac,
Opt_noac, Opt_lock, Opt_nolock, Opt_v2, Opt_v3, Opt_udp, Opt_tcp,
- Opt_acl, Opt_noacl,
+ Opt_acl, Opt_noacl, Opt_negde, Opt_nonegde,
/* Error token */
Opt_err
};
@@ -161,6 +161,8 @@ static match_table_t __initdata tokens = {
{Opt_tcp, "tcp"},
{Opt_acl, "acl"},
{Opt_noacl, "noacl"},
+ {Opt_negde, "negde"},
+ {Opt_nonegde, "nonegde"},
{Opt_err, NULL}
};
@@ -275,6 +277,12 @@ static int __init root_nfs_parse(char *name, char *buf)
case Opt_noacl:
nfs_data.flags |= NFS_MOUNT_NOACL;
break;
+ case Opt_negde:
+ nfs_data.flags &= ~NFS_MOUNT_NONEGDE;
+ break;
+ case Opt_nonegde:
+ nfs_data.flags |= NFS_MOUNT_NONEGDE;
+ break;
default:
printk(KERN_WARNING "Root-NFS: unknown "
"option: %s\n", p);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 0d1bc61..7043a07 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -75,6 +75,7 @@ enum {
Opt_acl, Opt_noacl,
Opt_rdirplus, Opt_nordirplus,
Opt_sharecache, Opt_nosharecache,
+ Opt_negde, Opt_nonegde,
/* Mount options that take integer arguments */
Opt_port,
@@ -124,6 +125,8 @@ static match_table_t nfs_mount_option_tokens = {
{ Opt_nordirplus, "nordirplus" },
{ Opt_sharecache, "sharecache" },
{ Opt_nosharecache, "nosharecache" },
+ { Opt_negde, "negde" },
+ { Opt_nonegde, "nonegde" },
{ Opt_port, "port=%u" },
{ Opt_rsize, "rsize=%u" },
@@ -455,6 +458,7 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
{ NFS_MOUNT_NOACL, ",noacl", "" },
{ NFS_MOUNT_NORDIRPLUS, ",nordirplus", "" },
{ NFS_MOUNT_UNSHARED, ",nosharecache", ""},
+ { NFS_MOUNT_NONEGDE, ",nonegde", ""},
{ 0, NULL, NULL }
};
const struct proc_nfs_info *nfs_infop;
@@ -779,6 +783,11 @@ static int nfs_parse_mount_options(char *raw,
case Opt_nosharecache:
mnt->flags |= NFS_MOUNT_UNSHARED;
break;
+ case Opt_negde:
+ mnt->flags &= ~NFS_MOUNT_NONEGDE;
+ break;
+ case Opt_nonegde:
+ mnt->flags |= NFS_MOUNT_NONEGDE;
case Opt_port:
if (match_int(args, &option))
diff --git a/include/linux/nfs_mount.h b/include/linux/nfs_mount.h
index a3ade89..44ffb60 100644
--- a/include/linux/nfs_mount.h
+++ b/include/linux/nfs_mount.h
@@ -47,22 +47,23 @@ struct nfs_mount_data {
/* bits in the flags field */
-#define NFS_MOUNT_SOFT 0x0001 /* 1 */
-#define NFS_MOUNT_INTR 0x0002 /* 1 */
-#define NFS_MOUNT_SECURE 0x0004 /* 1 */
-#define NFS_MOUNT_POSIX 0x0008 /* 1 */
-#define NFS_MOUNT_NOCTO 0x0010 /* 1 */
-#define NFS_MOUNT_NOAC 0x0020 /* 1 */
-#define NFS_MOUNT_TCP 0x0040 /* 2 */
-#define NFS_MOUNT_VER3 0x0080 /* 3 */
-#define NFS_MOUNT_KERBEROS 0x0100 /* 3 */
-#define NFS_MOUNT_NONLM 0x0200 /* 3 */
-#define NFS_MOUNT_BROKEN_SUID 0x0400 /* 4 */
-#define NFS_MOUNT_NOACL 0x0800 /* 4 */
-#define NFS_MOUNT_STRICTLOCK 0x1000 /* reserved for NFSv4 */
-#define NFS_MOUNT_SECFLAVOUR 0x2000 /* 5 */
-#define NFS_MOUNT_NORDIRPLUS 0x4000 /* 5 */
-#define NFS_MOUNT_UNSHARED 0x8000 /* 5 */
-#define NFS_MOUNT_FLAGMASK 0xFFFF
+#define NFS_MOUNT_SOFT 0x00001 /* 1 */
+#define NFS_MOUNT_INTR 0x00002 /* 1 */
+#define NFS_MOUNT_SECURE 0x00004 /* 1 */
+#define NFS_MOUNT_POSIX 0x00008 /* 1 */
+#define NFS_MOUNT_NOCTO 0x00010 /* 1 */
+#define NFS_MOUNT_NOAC 0x00020 /* 1 */
+#define NFS_MOUNT_TCP 0x00040 /* 2 */
+#define NFS_MOUNT_VER3 0x00080 /* 3 */
+#define NFS_MOUNT_KERBEROS 0x00100 /* 3 */
+#define NFS_MOUNT_NONLM 0x00200 /* 3 */
+#define NFS_MOUNT_BROKEN_SUID 0x00400 /* 4 */
+#define NFS_MOUNT_NOACL 0x00800 /* 4 */
+#define NFS_MOUNT_STRICTLOCK 0x01000 /* reserved for NFSv4 */
+#define NFS_MOUNT_SECFLAVOUR 0x02000 /* 5 */
+#define NFS_MOUNT_NORDIRPLUS 0x04000 /* 5 */
+#define NFS_MOUNT_UNSHARED 0x08000 /* 5 */
+#define NFS_MOUNT_NONEGDE 0x10000 /* 5 */
+#define NFS_MOUNT_FLAGMASK 0x1FFFF
#endif
On Tue, 2008-01-15 at 11:30 -0500, Bob Bell wrote:
> From: Bob Bell <[email protected]>
>
> Add NFS_MOUNT_NONEGDE mount flag. When this flag is set on a mounted
> NFS filesystem, negative dentries for that filesystem will not be
> cached. This can help address cache coherency when the NFS server does
> not provide sufficiently fine-grained timestamps to consistently
> distinguish when a directory has been modified, though performance when
> repeatedly accessing filenames for files that do not exist may be
> impacted.
>
> Signed-off-by: Bob Bell <[email protected]>
> ---
The patch itself looks OK. My only gripe is the two names 'negde' and
'nonegde'. Can we find something that rolls a bit more smoothly off the
tongue?
How about something like 'cacheneglookup' 'nocacheneglookup'? Better
suggestions, anyone?
Cheers
Trond
On Jan 15, 2008, at 11:52 AM, Trond Myklebust wrote:
> On Tue, 2008-01-15 at 11:30 -0500, Bob Bell wrote:
>> From: Bob Bell <[email protected]>
>>
>> Add NFS_MOUNT_NONEGDE mount flag. When this flag is set on a mounted
>> NFS filesystem, negative dentries for that filesystem will not be
>> cached. This can help address cache coherency when the NFS server
>> does
>> not provide sufficiently fine-grained timestamps to consistently
>> distinguish when a directory has been modified, though performance
>> when
>> repeatedly accessing filenames for files that do not exist may be
>> impacted.
>>
>> Signed-off-by: Bob Bell <[email protected]>
>> ---
>
> The patch itself looks OK. My only gripe is the two names 'negde' and
> 'nonegde'. Can we find something that rolls a bit more smoothly off
> the
> tongue?
Yeah, actually I thought it was spelled "non edge" until I looked
closer.
> How about something like 'cacheneglookup' 'nocacheneglookup'? Better
> suggestions, anyone?
I suspect system administrators have no idea what a "negative dentry"
is, and it would probably be beyond the scope of nfs(5) to explain
it. So I vote for something that describes the behavior as observed
by a "lay person" rather than something that names the kernel
mechanisms involved.
You might consider tying this behavior to "noac" as well. In other
words, make "noac" equivalent to "sync,actimeo=0,nonegde" .
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Tue, 2008-01-15 at 12:04 -0500, Chuck Lever wrote:
>
> I suspect system administrators have no idea what a "negative dentry"
> is, and it would probably be beyond the scope of nfs(5) to explain
> it. So I vote for something that describes the behavior as observed
> by a "lay person" rather than something that names the kernel
> mechanisms involved.
The concept of a 'dentry' is in any case very Linux-specific. The more
generic term would be a 'negative lookup'.
That said, if someone can find a better name, I'd be all for it.
> You might consider tying this behavior to "noac" as well. In other
> words, make "noac" equivalent to "sync,actimeo=0,nonegde" .
I'd be open for that idea, however that should definitely be a separate
patch.
Cheers
Trond
On Tue, Jan 15, 2008 at 12:04:50PM -0500, Chuck Lever wrote:
>On Jan 15, 2008, at 11:52 AM, Trond Myklebust wrote:
>>The patch itself looks OK. My only gripe is the two names 'negde' and
>>'nonegde'. Can we find something that rolls a bit more smoothly off
>>the
>>tongue?
>
>Yeah, actually I thought it was spelled "non edge" until I looked
>closer.
Heh -- I mentioned the same thing at the bottom of my (admittedly
verbose) intro the patch. In fact, I've mistyped it "nonedge" myself
a few times, which I figured was a strong hint it needed renaming.
I was going for the terseness I saw in the rest of the options, and
"nonegde" was the best my limited creativity could come up with.
>>How about something like 'cacheneglookup' 'nocacheneglookup'? Better
>>suggestions, anyone?
As long as no one objects to the length, I'll go with "nocacheneglookup"
(and NFS_MOUNT_NOCACHENEGLOOKUP, I suppose) for round 2.
>You might consider tying this behavior to "noac" as well. In other
>words, make "noac" equivalent to "sync,actimeo=0,nonegde" .
I'll provide this as a separate patch in the patch set, as Trond
indicated he would prefer. This might make sense, as this is probably
the set of options we'll used.
--
Bob Bell
On Tue, 2008-01-15 at 20:22 -0500, Bob Bell wrote:
> >>How about something like 'cacheneglookup' 'nocacheneglookup'? Better
> >>suggestions, anyone?
>
> As long as no one objects to the length, I'll go with "nocacheneglookup"
> (and NFS_MOUNT_NOCACHENEGLOOKUP, I suppose) for round 2.
I'm still not happy with that name. What do people think about
introducing a generic 'cache' option that can be used for fine control
of caching options?
I'm thinking somewhat along the lines of
-ocache=lookup_all /* Cache all dentries */
-ocache=lookup_positive /* Cache positive dentries */
-ocache=lookup_none /* Force all lookups */
...and then possibly adding other 'cache=' options later if we find out
that we need them?
Cheers
Trond
Trond Myklebust wrote:
> On Tue, 2008-01-15 at 20:22 -0500, Bob Bell wrote:
>>>> How about something like 'cacheneglookup' 'nocacheneglookup'? Better
>>>> suggestions, anyone?
>> As long as no one objects to the length, I'll go with "nocacheneglookup"
>> (and NFS_MOUNT_NOCACHENEGLOOKUP, I suppose) for round 2.
>
> I'm still not happy with that name. What do people think about
> introducing a generic 'cache' option that can be used for fine control
> of caching options?
>
> I'm thinking somewhat along the lines of
>
> -ocache=lookup_all /* Cache all dentries */
> -ocache=lookup_positive /* Cache positive dentries */
> -ocache=lookup_none /* Force all lookups */
Maybe we should define the type of cache were are controlling, in case
we want to add controls for other caches. So how about:
-olpcache=all /* Cache all directory entries */
-olpcache=found /* Cache only directory entires that are found */
-olpcache=off /* Turn off the caching of directory entries */
steved.
My $.02
-odircache=on
-odircache=hitsonly (or positive, or hits, or ?)
-odircache=off
-----Original Message-----
From: Steve Dickson [mailto:[email protected]]
Sent: Wednesday, January 16, 2008 4:50 AM
To: Trond Myklebust
Cc: Bob Bell; Chuck Lever; NFS list
Subject: Re: [PATCH 2/3] NFS: Add NFS_MOUNT_NONEGDE flag to avoid
caching negative dentries
Trond Myklebust wrote:
> On Tue, 2008-01-15 at 20:22 -0500, Bob Bell wrote:
>>>> How about something like 'cacheneglookup' 'nocacheneglookup'?
>>>> Better suggestions, anyone?
>> As long as no one objects to the length, I'll go with
"nocacheneglookup"
>> (and NFS_MOUNT_NOCACHENEGLOOKUP, I suppose) for round 2.
>
> I'm still not happy with that name. What do people think about
> introducing a generic 'cache' option that can be used for fine control
> of caching options?
>
> I'm thinking somewhat along the lines of
>
> -ocache=lookup_all /* Cache all dentries */
> -ocache=lookup_positive /* Cache positive dentries */
> -ocache=lookup_none /* Force all lookups */
Maybe we should define the type of cache were are controlling, in case
we want to add controls for other caches. So how about:
-olpcache=all /* Cache all directory entries */
-olpcache=found /* Cache only directory entires that are
found */
-olpcache=off /* Turn off the caching of directory
entries */
steved.
On Wed, 2008-01-16 at 11:43 -0800, Muntz, Daniel wrote:
> My $.02
>
> -odircache=on
> -odircache=hitsonly (or positive, or hits, or ?)
> -odircache=off
I like the 'dircache' bit, but to me 'on/off' suggests a binary switch:
they don't mix well with a third state.
How about
-odircache=aggr[essive] /* Full caching */
-odircache=noneg[ative] /* Positive lookups only */
-odircache=off /* strict lookup revalidation */
?
> -----Original Message-----
> From: Steve Dickson [mailto:[email protected]]
> Sent: Wednesday, January 16, 2008 4:50 AM
> To: Trond Myklebust
> Cc: Bob Bell; Chuck Lever; NFS list
> Subject: Re: [PATCH 2/3] NFS: Add NFS_MOUNT_NONEGDE flag to avoid
> caching negative dentries
>
> Trond Myklebust wrote:
> > On Tue, 2008-01-15 at 20:22 -0500, Bob Bell wrote:
> >>>> How about something like 'cacheneglookup' 'nocacheneglookup'?
> >>>> Better suggestions, anyone?
> >> As long as no one objects to the length, I'll go with
> "nocacheneglookup"
> >> (and NFS_MOUNT_NOCACHENEGLOOKUP, I suppose) for round 2.
> >
> > I'm still not happy with that name. What do people think about
> > introducing a generic 'cache' option that can be used for fine control
>
> > of caching options?
> >
> > I'm thinking somewhat along the lines of
> >
> > -ocache=lookup_all /* Cache all dentries */
> > -ocache=lookup_positive /* Cache positive dentries */
> > -ocache=lookup_none /* Force all lookups */
> Maybe we should define the type of cache were are controlling, in case
> we want to add controls for other caches. So how about:
>
> -olpcache=all /* Cache all directory entries */
> -olpcache=found /* Cache only directory entires that are
> found */
> -olpcache=off /* Turn off the caching of directory
> entries */
>
>
> steved.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected] 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jan 16, 2008, at 3:43 PM, Trond Myklebust wrote:
> On Wed, 2008-01-16 at 11:43 -0800, Muntz, Daniel wrote:
>> My $.02
>>
>> -odircache=on
>> -odircache=hitsonly (or positive, or hits, or ?)
>> -odircache=off
>
>
> I like the 'dircache' bit, but to me 'on/off' suggests a binary
> switch:
> they don't mix well with a third state.
>
> How about
>
> -odircache=aggr[essive] /* Full caching */
> -odircache=noneg[ative] /* Positive lookups only */
> -odircache=off /* strict lookup revalidation */
"-olookupcache=" would be even more specific, if not more verbose.
dircache=off implies that not even readdir results are cached.
-olookupcache=full
-olookupcache=pos[itive]
-olookupcache=strict
>> -----Original Message-----
>> From: Steve Dickson [mailto:[email protected]]
>> Sent: Wednesday, January 16, 2008 4:50 AM
>> To: Trond Myklebust
>> Cc: Bob Bell; Chuck Lever; NFS list
>> Subject: Re: [PATCH 2/3] NFS: Add NFS_MOUNT_NONEGDE flag to avoid
>> caching negative dentries
>>
>> Trond Myklebust wrote:
>>> On Tue, 2008-01-15 at 20:22 -0500, Bob Bell wrote:
>>>>>> How about something like 'cacheneglookup' 'nocacheneglookup'?
>>>>>> Better suggestions, anyone?
>>>> As long as no one objects to the length, I'll go with
>> "nocacheneglookup"
>>>> (and NFS_MOUNT_NOCACHENEGLOOKUP, I suppose) for round 2.
>>>
>>> I'm still not happy with that name. What do people think about
>>> introducing a generic 'cache' option that can be used for fine
>>> control
>>
>>> of caching options?
>>>
>>> I'm thinking somewhat along the lines of
>>>
>>> -ocache=lookup_all /* Cache all dentries */
>>> -ocache=lookup_positive /* Cache positive dentries */
>>> -ocache=lookup_none /* Force all lookups */
>> Maybe we should define the type of cache were are controlling, in
>> case
>> we want to add controls for other caches. So how about:
>>
>> -olpcache=all /* Cache all directory entries */
>> -olpcache=found /* Cache only directory entires that are
>> found */
>> -olpcache=off /* Turn off the caching of directory
>> entries */
>>
>>
>> steved.
>> -
>> To unsubscribe from this list: send the line "unsubscribe linux-
>> nfs" in
>> the body of a message to [email protected] 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
Already decided not to go the distance, with separate controls of
time-to-live of positive and negative lookup cache entries (or is it
not considered good form to overload negttl=0 as means to disable)?
On Wed, 2008-01-16 at 13:12 -0800, Risto Bell wrote:
> Already decided not to go the distance, with separate controls of
> time-to-live of positive and negative lookup cache entries (or is it
> not considered good form to overload negttl=0 as means to disable)?
We don't have a time-to-live scheme for dentries. When deciding whether
we need to revalidate the lookup, we check the parent directory mtime,
and see if the directory contents have changed since the last
revalidation.
Trond
On Jan 16, 2008, at 4:56 PM, Trond Myklebust wrote:
> On Wed, 2008-01-16 at 13:12 -0800, Risto Bell wrote:
>> Already decided not to go the distance, with separate controls of
>> time-to-live of positive and negative lookup cache entries (or is it
>> not considered good form to overload negttl=0 as means to disable)?
>
> We don't have a time-to-live scheme for dentries.
Adding a TTL heuristic to dentry revalidation might help our
directory mtime problem. :-)
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Wed, 2008-01-16 at 17:09 -0500, Chuck Lever wrote:
> On Jan 16, 2008, at 4:56 PM, Trond Myklebust wrote:
> > On Wed, 2008-01-16 at 13:12 -0800, Risto Bell wrote:
> >> Already decided not to go the distance, with separate controls of
> >> time-to-live of positive and negative lookup cache entries (or is it
> >> not considered good form to overload negttl=0 as means to disable)?
> >
> > We don't have a time-to-live scheme for dentries.
>
> Adding a TTL heuristic to dentry revalidation might help our
> directory mtime problem. :-)
I have a serious problem seeing how it might 'solve' anything at all.
If you're worried about correctness, then ttl is a completely useless
scheme: you can defeat it just as easily as you can defeat the mtime
resolution. The only difference is that you will eventually catch onto
the mistake, but you may have been operating on the wrong file for quite
some time before the ttl expires.
IOW: The only useful ttl values are ttl=infinity and ttl=0.
Trond
On Wed, Jan 16, 2008 at 03:49:20PM -0500, Chuck Lever wrote:
>On Jan 16, 2008, at 3:43 PM, Trond Myklebust wrote:
>>How about
>> -odircache=aggr[essive] /* Full caching */
>> -odircache=noneg[ative] /* Positive lookups only */
>> -odircache=off /* strict lookup revalidation */
>
> "-olookupcache=" would be even more specific, if not more verbose.
>dircache=off implies that not even readdir results are cached.
>
> -olookupcache=full
> -olookupcache=pos[itive]
> -olookupcache=strict
I think that "lookupcache" is a little more accurate, and perhaps worth
the verbosity. "dircache" could be misinterpreted to imply that
directory listings are being cached -- which, incidentally, I'm start to
receive complaints is a problem for us, too...
Converstation on this seems to have died down, and I'm ready to revisit
the patch. I'm inclined to go with:
-o lookupcache=full
-o lookupcache=pos[itive]
-o lookupcache=none
If you have a strong (enough) opinion otherwise, please speak up now and
save me the trouble of an extra pass at the patch...
--
Bob Bell