_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Thu, 2007-04-26 at 01:33 -0300, Fabio Olive Leite wrote:
> This leads to several problematic situations in long-lived NFS mounts.
> We can see old attributes not being updated, negative dentries that do
> not revalidate even after touching their parent dir, etc.
>
This is quite an interesting issue and I'm still having trouble
understanding it so let me try and explain my understanding of part of
it.
Lets consider the d_fsdata field of a negative dentry and the
nfs_neg_need_reval() function. Assuming no in progress io and the
request is not an exclusive open then nfs_neg_need_reval() will return
false when d_fsdata is after the modification timestamp of the
directory. This leads to the dentry being considered valid and so no
(VFS) lookup is performed. This is the only problem case I can see here
because otherwise the dentry is unhashed by nfs_lookup_revalidate()
forcing a new (VFS) lookup to be done (assuming the reference count of
the negative dentry is really 0).
So the problem you describe actually happens when the directory inode
cache_change_attribute warps and the dentry d_fstada timestamp is later
than inode cache_change_attribute. So updating the
cache_change_attribute field, as in the patch that Neil referred to,
will often not fix it.
Have I got this right yet?
Ian
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Wed, 2007-05-02 at 19:52 +0800, Ian Kent wrote:
> On Thu, 2007-04-26 at 01:33 -0300, Fabio Olive Leite wrote:
> > This leads to several problematic situations in long-lived NFS mounts.
> > We can see old attributes not being updated, negative dentries that do
> > not revalidate even after touching their parent dir, etc.
> >
>
> This is quite an interesting issue and I'm still having trouble
> understanding it so let me try and explain my understanding of part of
> it.
>
snip ...
> So the problem you describe actually happens when the directory inode
> cache_change_attribute warps and the dentry d_fstada timestamp is later
> than inode cache_change_attribute. So updating the
> cache_change_attribute field, as in the patch that Neil referred to,
> will often not fix it.
I believe the patch that Neil refers to will ensure that the inode
cache_change_attribute is updated to a reasonably recent time.
>
> Have I got this right yet?
No response so I guess so!
Then how about this patch.
It doesn't step on other dentry fields.
I haven't tested this except that it compiles because I'm after comments
as to whether it is a sensible approach.
---
--- linux-2.6.21/fs/nfs/dir.c.use-timespec-for-verifier 2007-05-08 09:37:14.000000000 +0800
+++ linux-2.6.21/fs/nfs/dir.c 2007-05-08 10:33:31.000000000 +0800
@@ -633,28 +633,49 @@
}
/*
+ * Allocate verifier timespec once during lookup and free
+ * it during d_release.
+ */
+static inline int nfs_alloc_verifier(struct dentry *dentry)
+{
+ dentry->d_fsdata = kmalloc(sizeof(struct timespec), GFP_KERNEL);
+ if (!dentry->d_fsdata)
+ return 0;
+ return 1;
+}
+
+/*
* A check for whether or not the parent directory has changed.
* In the case it has, we assume that the dentries are untrustworthy
* and may need to be looked up again.
*/
static int nfs_check_verifier(struct inode *dir, struct dentry *dentry)
{
+ struct timespec *chattr = dentry->d_fsdata;
+ struct timespec cache_chattr;
+
if (IS_ROOT(dentry))
return 1;
if ((NFS_I(dir)->cache_validity & NFS_INO_INVALID_ATTR) != 0
|| nfs_attribute_timeout(dir))
return 0;
- return nfs_verify_change_attribute(dir, (unsigned long)dentry->d_fsdata);
+ jiffies_to_timespec(NFS_I(dir)->cache_change_attribute, &cache_chattr);
+ return !nfs_caches_unstable(dir)
+ && (timespec_compare(chattr, &cache_chattr) >= 0);
}
static inline void nfs_set_verifier(struct dentry * dentry, unsigned long verf)
{
- dentry->d_fsdata = (void *)verf;
+ jiffies_to_timespec(verf, (struct timespec *) &dentry->d_fsdata);
}
static void nfs_refresh_verifier(struct dentry * dentry, unsigned long verf)
{
- if (time_after(verf, (unsigned long)dentry->d_fsdata))
+ struct timespec *last = dentry->d_fsdata;
+ struct timespec verifier;
+
+ jiffies_to_timespec(verf, &verifier);
+ if (timespec_compare(&verifier, last) > 0)
nfs_set_verifier(dentry, verf);
}
@@ -860,10 +881,17 @@
iput(inode);
}
+static void nfs_dentry_release(struct dentry *dentry)
+{
+ if (dentry->d_fsdata)
+ kfree(dentry->d_fsdata);
+}
+
struct dentry_operations nfs_dentry_operations = {
.d_revalidate = nfs_lookup_revalidate,
.d_delete = nfs_dentry_delete,
.d_iput = nfs_dentry_iput,
+ .d_release = nfs_dentry_release,
};
/*
@@ -909,6 +937,8 @@
res = ERR_PTR(-ENOMEM);
dentry->d_op = NFS_PROTO(dir)->dentry_ops;
+ if (!nfs_alloc_verifier(dentry))
+ goto out;
lock_kernel();
@@ -967,6 +997,7 @@
.d_revalidate = nfs_open_revalidate,
.d_delete = nfs_dentry_delete,
.d_iput = nfs_dentry_iput,
+ .d_release = nfs_dentry_release,
};
/*
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Tue, 2007-05-08 at 10:37 +0800, Ian Kent wrote:
> On Wed, 2007-05-02 at 19:52 +0800, Ian Kent wrote:
> > On Thu, 2007-04-26 at 01:33 -0300, Fabio Olive Leite wrote:
> > > This leads to several problematic situations in long-lived NFS mounts.
> > > We can see old attributes not being updated, negative dentries that do
> > > not revalidate even after touching their parent dir, etc.
> > >
> >
> > This is quite an interesting issue and I'm still having trouble
> > understanding it so let me try and explain my understanding of part of
> > it.
> >
>
> snip ...
>
> > So the problem you describe actually happens when the directory inode
> > cache_change_attribute warps and the dentry d_fstada timestamp is later
> > than inode cache_change_attribute. So updating the
> > cache_change_attribute field, as in the patch that Neil referred to,
> > will often not fix it.
>
> I believe the patch that Neil refers to will ensure that the inode
> cache_change_attribute is updated to a reasonably recent time.
>
> >
> > Have I got this right yet?
>
> No response so I guess so!
>
> Then how about this patch.
> It doesn't step on other dentry fields.
> I haven't tested this except that it compiles because I'm after comments
> as to whether it is a sensible approach.
>
> ---
> --- linux-2.6.21/fs/nfs/dir.c.use-timespec-for-verifier 2007-05-08 09:37:14.000000000 +0800
> +++ linux-2.6.21/fs/nfs/dir.c 2007-05-08 10:33:31.000000000 +0800
> @@ -633,28 +633,49 @@
> }
>
> /*
> + * Allocate verifier timespec once during lookup and free
> + * it during d_release.
> + */
> +static inline int nfs_alloc_verifier(struct dentry *dentry)
> +{
> + dentry->d_fsdata = kmalloc(sizeof(struct timespec), GFP_KERNEL);
> + if (!dentry->d_fsdata)
> + return 0;
> + return 1;
> +}
> +
> +/*
> * A check for whether or not the parent directory has changed.
> * In the case it has, we assume that the dentries are untrustworthy
> * and may need to be looked up again.
> */
> static int nfs_check_verifier(struct inode *dir, struct dentry *dentry)
> {
> + struct timespec *chattr = dentry->d_fsdata;
> + struct timespec cache_chattr;
> +
> if (IS_ROOT(dentry))
> return 1;
> if ((NFS_I(dir)->cache_validity & NFS_INO_INVALID_ATTR) != 0
> || nfs_attribute_timeout(dir))
> return 0;
> - return nfs_verify_change_attribute(dir, (unsigned long)dentry->d_fsdata);
> + jiffies_to_timespec(NFS_I(dir)->cache_change_attribute, &cache_chattr);
> + return !nfs_caches_unstable(dir)
> + && (timespec_compare(chattr, &cache_chattr) >= 0);
> }
>
> static inline void nfs_set_verifier(struct dentry * dentry, unsigned long verf)
> {
> - dentry->d_fsdata = (void *)verf;
> + jiffies_to_timespec(verf, (struct timespec *) &dentry->d_fsdata);
Oops, that's wrong.
The & needs to be removed but you get the idea.
> }
>
> static void nfs_refresh_verifier(struct dentry * dentry, unsigned long verf)
> {
> - if (time_after(verf, (unsigned long)dentry->d_fsdata))
> + struct timespec *last = dentry->d_fsdata;
> + struct timespec verifier;
> +
> + jiffies_to_timespec(verf, &verifier);
> + if (timespec_compare(&verifier, last) > 0)
> nfs_set_verifier(dentry, verf);
> }
>
> @@ -860,10 +881,17 @@
> iput(inode);
> }
>
> +static void nfs_dentry_release(struct dentry *dentry)
> +{
> + if (dentry->d_fsdata)
> + kfree(dentry->d_fsdata);
> +}
> +
> struct dentry_operations nfs_dentry_operations = {
> .d_revalidate = nfs_lookup_revalidate,
> .d_delete = nfs_dentry_delete,
> .d_iput = nfs_dentry_iput,
> + .d_release = nfs_dentry_release,
> };
>
> /*
> @@ -909,6 +937,8 @@
>
> res = ERR_PTR(-ENOMEM);
> dentry->d_op = NFS_PROTO(dir)->dentry_ops;
> + if (!nfs_alloc_verifier(dentry))
> + goto out;
>
> lock_kernel();
>
> @@ -967,6 +997,7 @@
> .d_revalidate = nfs_open_revalidate,
> .d_delete = nfs_dentry_delete,
> .d_iput = nfs_dentry_iput,
> + .d_release = nfs_dentry_release,
> };
>
> /*
>
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by DB2 Express
> Download DB2 Express C - the FREE version of DB2 express and take
> control of your XML. No limits. Just data. Click to get it now.
> http://sourceforge.net/powerbar/db2/
> _______________________________________________
> NFS maillist - [email protected]
> https://lists.sourceforge.net/lists/listinfo/nfs
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs