All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: NeilBrown <neilb@suse.de>, NFS <linux-nfs@vger.kernel.org>
Subject: Re: Rename dir on server can cause client to get ESTALE - this time with PATCH
Date: Thu, 1 Dec 2011 02:47:35 +0000	[thread overview]
Message-ID: <20111201024735.GZ2203@ZenIV.linux.org.uk> (raw)
In-Reply-To: <1322706258.2646.6.camel@lade.trondhjem.org>

On Wed, Nov 30, 2011 at 09:24:18PM -0500, Trond Myklebust wrote:

> > As far as I can tell, the idea of open done in ->d_revalidate() is
> > unsalvagable.  It's simply the wrong place for that.  Note that NFS
> > is the only filesystem trying to do atomic open stuff in its ->d_revalidate()
> > and it's not succeeding.
> 
> Not doing an open there is prohibitively expensive, though: you are
> likely to see your cached inode flushed down the toilet if you just drop
> the dentry...

Wrong.  All you really need is to have that attempt to issue OPEN shifted
into ->open() itself.  The only interesting part is that we might need
to drop the original dentry and use a new one for ->f_path.dentry.

Don't drop that dentry; after the case in ->d_revalidate() that would have
attempted that OPEN you would either cross into covering vfsmount (in which
case dentry should be left alone as you are doing now) or issue ->open().
If it's really not valid (i.e. if OPEN yields a different inode), we can
deal with that in ->open() just fine.  The *only* subtle part is how to
deal with "it's a symlink, go away" from the server.  Which will require
changes in do_last().  I have that stuff; it'll need debugging serious
review once posted.  Which I'm going to do over weekend.

      reply	other threads:[~2011-12-01  2:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-14  2:19 Rename dir on server can cause client to get ESTALE NeilBrown
2011-12-01  1:49 ` Rename dir on server can cause client to get ESTALE - this time with PATCH NeilBrown
2011-12-01  2:12   ` Al Viro
2011-12-01  2:24     ` Trond Myklebust
2011-12-01  2:47       ` Al Viro [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111201024735.GZ2203@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=Trond.Myklebust@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.