linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Oleg Drokin <green@linuxhacker.ru>
Cc: "J. Bruce Fields" <bfields@redhat.com>,
	linux-nfs@vger.kernel.org,
	"<linux-kernel@vger.kernel.org> Mailing List"
	<linux-kernel@vger.kernel.org>,
	"<linux-fsdevel@vger.kernel.org>" <linux-fsdevel@vger.kernel.org>
Subject: Re: NFS/d_splice_alias breakage
Date: Fri, 3 Jun 2016 05:53:01 +0100	[thread overview]
Message-ID: <20160603045301.GP14480@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20160603044253.GO14480@ZenIV.linux.org.uk>

On Fri, Jun 03, 2016 at 05:42:53AM +0100, Al Viro wrote:
> On Fri, Jun 03, 2016 at 05:26:48AM +0100, Al Viro wrote:
> 
> > Looks like the right thing to do would be to do d_drop() at no_open:,
> > just before we call nfs_lookup().  If not earlier, actually...  How
> > about the following?
> 
> A bit of rationale: dentry in question is negative and attempt to open
> it has just failed; in case it's really negative we did that d_drop()
> anyway (and then unconditionally rehashed it).  In case when we proceed to
> nfs_lookup() and it does not fail, we'll have it rehashed there (with the
> right inode).  What do we lose from doing d_drop() on *any* error here?
> It's negative, with dubious validity.  In the worst case, we had open
> and lookup fail, but it's just us - the sucker really is negative and
> somebody else would be able to revalidate it.  If we drop it here (and
> not rehash later), that somebody else will have to allocate an in-core
> dentry before doing lookup or atomic_open.  Which is negligible extra
> burden.

I suspect that it got broken in commit 275bb3078 (NFSv4: Move dentry
instantiation into the NFSv4-specific atomic open code).  Prior to that
commit, d_drop() was there (error or no error).  Looks like 3.10+, indeed.
I agree that we shouldn't drop it on successful open, but it needs to be
done on errors.  All of them, not just ENOENT, as in that commit.

  reply	other threads:[~2016-06-03  4:53 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02 22:46 NFS/d_splice_alias breakage Oleg Drokin
2016-06-02 23:59 ` [PATCH] Allow d_splice_alias to accept hashed dentries green
2016-06-03  0:25   ` Oleg Drokin
2016-06-03  0:44 ` NFS/d_splice_alias breakage Trond Myklebust
2016-06-03  0:54   ` Oleg Drokin
2016-06-03  3:26     ` Al Viro
2016-06-03  3:38       ` Al Viro
2016-06-03  3:28   ` Al Viro
2016-06-03  3:37 ` Al Viro
2016-06-03  3:43   ` Oleg Drokin
2016-06-03  4:26     ` Al Viro
2016-06-03  4:42       ` Al Viro
2016-06-03  4:53         ` Al Viro [this message]
2016-06-03  4:58       ` Oleg Drokin
2016-06-03  5:56         ` Al Viro
2016-06-06 23:36           ` Oleg Drokin
2016-06-10  1:33             ` Oleg Drokin
2016-06-10 16:49               ` Oleg Drokin
2016-06-20 13:25           ` Oleg Drokin
2016-06-20 14:08             ` Al Viro
2016-06-20 14:54               ` Trond Myklebust
2016-06-20 15:28                 ` Al Viro
2016-06-20 15:43               ` Anna Schumaker
2016-06-20 15:45                 ` Oleg Drokin
2016-06-20 15:47                 ` Trond Myklebust
2016-06-03 16:38       ` Dcache oops Oleg Drokin
2016-06-03 18:22         ` Al Viro
2016-06-03 18:35           ` Oleg Drokin
2016-06-03 20:07             ` Al Viro
2016-06-03 21:17               ` Oleg Drokin
2016-06-03 21:46                 ` Al Viro
2016-06-03 22:17                   ` Al Viro
2016-06-03 21:18               ` Linus Torvalds
2016-06-03 21:26                 ` Al Viro
2016-06-03 22:00                   ` Linus Torvalds
2016-06-03 22:23                     ` Al Viro
2016-06-03 22:29                       ` Al Viro
2016-06-03 22:36                       ` Linus Torvalds
2016-06-03 22:42                         ` Oleg Drokin
2016-06-03 22:43                         ` Al Viro
2016-06-03 22:37                       ` Al Viro
2016-06-03 22:49                         ` Oleg Drokin
2016-06-03 23:58                         ` Oleg Drokin
2016-06-04  0:56                           ` Al Viro
2016-06-04 12:25                             ` Jeff Layton
2016-06-04 16:12                             ` Oleg Drokin

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=20160603045301.GP14480@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=bfields@redhat.com \
    --cc=green@linuxhacker.ru \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).