All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH]  Reinstantiating stale inodes
@ 2004-04-23 14:48 Lever, Charles
  2004-04-23 15:00 ` Trond Myklebust
  2004-04-23 15:08 ` Olaf Kirch
  0 siblings, 2 replies; 33+ messages in thread
From: Lever, Charles @ 2004-04-23 14:48 UTC (permalink / raw)
  To: Trond Myklebust, Steve Dickson; +Cc: nfs

> On Fri, 2004-04-23 at 10:15, Steve Dickson wrote:
> > Here is a 2.4 patch that will reinstantiate an inode
> > when a ESTALE error is returned on a getattr. When
> > the error occurs, a lookup is immediately issued
> > to get a new fh.
> >=20
> > The fixes the problem of a server rsync -a directory
> > that a client has mounted. The key being the -a flag
> > since it causes the server not to update the mtime on
> > the directory.
> >=20
> > My initial efforts was to make nfs_lookup_revalidate()
> > a bit smarter with the use of ctimes but turns out
> > that when nfs_lookup_revalidate() does no caching
> > (i.e. an otw lookup is issued on every call) the
> > ESTALEs still occurred.
> >=20
> > Then I turned my attention to __nfs_refresh_inode() and
> > had it used ctime in its calculations of what is
> > and is not valid... This did work, but it cause a significant
> > amount of extra otw traffic (using the cthon test suite) for
> > the non error cases. The one thing good about this patch (imho)
> > is the extra lookups only occur after an error that generally
> > does not happen...
> >=20
> > Comments would be appreciated, especially about how
> > I'm reinstantiating the dentry....
>=20
> There are several problems here, but the main one is that you have no
> guarantees that you are the exclusive user of that dentry.
>=20
> This again means that people who think they have open files=20
> on the "old"
> inode may suddenly find their program Oopsing or corrupting the new
> file. Ditto for all those shrink_dcache_*() which are likely to Oops.

yes, i was wondering about that when i saw the patch.

there appears to be a real problem when restoring from a backup,
or using rsync.  the file size and the mtime stay precisely the
same, but the file handle changes.  i'm not sure anything can be
done about this in NFSv2/3?


-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 33+ messages in thread
* RE: [PATCH]  Reinstantiating stale inodes
@ 2004-04-23 16:16 Lever, Charles
  2004-04-23 16:27 ` Steve Dickson
  0 siblings, 1 reply; 33+ messages in thread
From: Lever, Charles @ 2004-04-23 16:16 UTC (permalink / raw)
  To: Steve Dickson; +Cc: nfs

> >This again means that people who think they have open files=20
> on the "old"
> >inode may suddenly find their program Oopsing or corrupting the new
> >file.=20
> >
> Again, this new fh is only gotten on getattrs any other ops will still
> fail with ESTALE...

so you need to invalidate all the cached pages for this inode
explicitly.  nfs_refresh_inode will not detect any change in
mtime or i_size so it will not do this for you.

but this exposes us to the "unable to invalidate locked pages"
bug again.

and i think this is only a valid thing to do iff the mtime
and i_size are the same as our cached values but the fh has
changed.  otherwise, you can fool yourself into thinking that
an entirely new file is the same as the old one.



-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 33+ messages in thread
* RE: [PATCH]  Reinstantiating stale inodes
@ 2004-04-23 15:17 Lever, Charles
  0 siblings, 0 replies; 33+ messages in thread
From: Lever, Charles @ 2004-04-23 15:17 UTC (permalink / raw)
  To: Olaf Kirch; +Cc: Trond Myklebust, Steve Dickson, nfs

> On Fri, Apr 23, 2004 at 07:48:11AM -0700, Lever, Charles wrote:
> > there appears to be a real problem when restoring from a backup,
> > or using rsync.  the file size and the mtime stay precisely the
> > same, but the file handle changes.  i'm not sure anything can be
> > done about this in NFSv2/3?
>=20
> Well, namei_open could interpret an ESTALE error as "retry the
> lookup and open, ignoring the cache".
> I think this would solve 99% of all problems.

my impression was that viro would wretch at the idea of adding
file-system specific logic to the VFS layer.  %^)


-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 33+ messages in thread
* [PATCH]  Reinstantiating stale inodes
@ 2004-04-23 14:15 Steve Dickson
  2004-04-23 14:33 ` Olaf Kirch
                   ` (4 more replies)
  0 siblings, 5 replies; 33+ messages in thread
From: Steve Dickson @ 2004-04-23 14:15 UTC (permalink / raw)
  To: nfs

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

Here is a 2.4 patch that will reinstantiate an inode
when a ESTALE error is returned on a getattr. When
the error occurs, a lookup is immediately issued
to get a new fh.

The fixes the problem of a server rsync -a directory
that a client has mounted. The key being the -a flag
since it causes the server not to update the mtime on
the directory.

My initial efforts was to make nfs_lookup_revalidate()
a bit smarter with the use of ctimes but turns out
that when nfs_lookup_revalidate() does no caching
(i.e. an otw lookup is issued on every call) the
ESTALEs still occurred.

Then I turned my attention to __nfs_refresh_inode() and
had it used ctime in its calculations of what is
and is not valid... This did work, but it cause a significant
amount of extra otw traffic (using the cthon test suite) for
the non error cases. The one thing good about this patch (imho)
is the extra lookups only occur after an error that generally
does not happen...

Comments would be appreciated, especially about how
I'm reinstantiating the dentry....

SteveD.



[-- Attachment #2: linux-2.4.21-nfs-estale.patch --]
[-- Type: text/plain, Size: 1413 bytes --]

--- linux-2.4.21/fs/nfs/inode.c.org	2004-04-17 18:26:32.000000000 -0400
+++ linux-2.4.21/fs/nfs/inode.c	2004-04-23 03:19:51.000000000 -0400
@@ -953,13 +953,57 @@ nfs_wait_on_inode(struct inode *inode, i
 }
 
 /*
+ * Reinstantiate an inode that has gone stale
+ */
+static int
+nfs_reinstantiate(
+	struct inode *dir, 
+	struct dentry *dentry, 
+	struct nfs_fattr *fattr)
+{
+	int error;
+	struct nfs_fh fhandle;
+	struct inode *inode;
+
+	error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
+	if (!error) {
+		error = -ENOMEM;
+		inode = nfs_fhget(dentry, &fhandle, &fattr);
+		if (inode) {
+			d_drop(dentry);
+			dput(dentry);
+			d_instantiate(dentry, inode);
+			dentry->d_time = jiffies;
+			error = 0;
+		}
+	}
+	return error;
+}
+
+/*
  * Externally visible revalidation function
  */
 int
 nfs_revalidate(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
-	return nfs_revalidate_inode(NFS_SERVER(inode), inode);
+	struct inode *pinode;
+	struct nfs_fattr fattr;
+	int error;
+
+	error = nfs_revalidate_inode(NFS_SERVER(inode), inode);
+	if (!error || error != -ESTALE)
+		return error;
+	/*
+	 * We have a stale fh so ask the server for another one
+	 */
+	pinode = dentry->d_parent->d_inode;	
+	if (nfs_reinstantiate(pinode, dentry, &fattr) == 0) {
+		inode = dentry->d_inode;
+		if (nfs_refresh_inode(inode, &fattr) == 0)
+			error = 0;
+	}
+	return error;
 }
 
 /*

^ permalink raw reply	[flat|nested] 33+ messages in thread
end of thread, other threads:[~2004-05-06 17:38 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-04-23 14:48 [PATCH] Reinstantiating stale inodes Lever, Charles
2004-04-23 15:00 ` Trond Myklebust
2004-04-23 16:16   ` Steve Dickson
2004-04-23 15:08 ` Olaf Kirch
  -- strict thread matches above, loose matches on Subject: below --
2004-04-23 16:16 Lever, Charles
2004-04-23 16:27 ` Steve Dickson
2004-04-23 15:17 Lever, Charles
2004-04-23 14:15 Steve Dickson
2004-04-23 14:33 ` Olaf Kirch
2004-04-23 15:50   ` Steve Dickson
2004-04-23 17:55     ` Olaf Kirch
2004-04-23 18:43       ` Steve Dickson
2004-04-23 18:50         ` Olaf Kirch
2004-04-23 20:07           ` Steve Dickson
2004-04-23 14:36 ` Trond Myklebust
2004-04-23 16:01   ` Steve Dickson
2004-04-23 16:21     ` Trond Myklebust
2004-04-23 17:21       ` Steve Dickson
2004-04-23 17:49         ` Trond Myklebust
2004-04-23 19:14           ` Steve Dickson
     [not found] ` <40892DC0.1010001@redhat.com>
2004-04-23 16:04   ` Steve Dickson
2004-05-01 16:13 ` Steve Dickson
2004-05-01 19:25   ` Trond Myklebust
2004-05-01 23:57     ` Steve Dickson
2004-05-02  0:22       ` Trond Myklebust
2004-05-02  3:19         ` Steve Dickson
2004-05-02  3:28           ` Trond Myklebust
2004-05-03 19:50             ` Steve Dickson
2004-05-03 20:15               ` Trond Myklebust
2004-05-03 20:33                 ` Steve Dickson
2004-05-03 21:27                   ` Trond Myklebust
2004-05-04 19:05                     ` Steve Dickson
2004-05-06 17:39 ` Steve Dickson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.