Linux NFS development
 help / color / mirror / Atom feed
From: Jeff Layton <jeff.layton@primarydata.com>
To: "J. R. Okajima" <hooanon05g@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] dcache: return -ESTALE not -EBUSY on distributed fs race
Date: Thu, 18 Dec 2014 12:32:23 -0500	[thread overview]
Message-ID: <20141218123223.6c88cdf1@tlielax.poochiereds.net> (raw)
In-Reply-To: <13170.1418920034@jrobl>

On Fri, 19 Dec 2014 01:27:14 +0900
"J. R. Okajima" <hooanon05g@gmail.com> wrote:

> 
> "J. Bruce Fields":
> > Why do you think -EBUSY's the right error in the local filesystem case?
> 
> This busy_or_stale() is another bandaid, based upon your patch, EBUSY
> --> ESTALE.
> Because the msg string of ESTALE is "Stale NFS file handle" on many
> systems, I don't think it a good idea to return it for local fs. If you
> think EIO is better than EBUSY you can change it to eio_or_stale(). If
> you think it is surely not happen on every local fs, then this inline
> function is not necessary.
> 

I think the only way this could happen on a local fs would be for it to
allow hardlinked directories, which is (of course) forbidden. After
all, the comment above __d_unalias specifically mentions that it's
there to handle remotely renamed directories.

I think the patch is almost certainly fine for NFS. The only question
I'd have is whether other distributed filesystems (p9? ceph? gfs2?)
might have an issue here. It seems unlikely though, and most of them
would also benefit from redoing the lookup with LOOKUP_REVAL set in
this situation.

Still, it's probably worth letting this soak in -next for a bit to be
sure we're not missing anything.

Bruce, you can add my:

Acked-by: Jeff Layton <jlayton@primarydata.com>

  reply	other threads:[~2014-12-18 17:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-17 19:59 [PATCH] dcache: return -ESTALE not -EBUSY on distributed fs race J. Bruce Fields
2014-12-17 20:01 ` J. Bruce Fields
2014-12-18 15:50   ` J. R. Okajima
2014-12-18 15:58     ` J. Bruce Fields
2014-12-18 16:27       ` J. R. Okajima
2014-12-18 17:32         ` Jeff Layton [this message]
2014-12-18 23:15         ` Dave Chinner
2014-12-19  2:46           ` J. R. Okajima
2014-12-19 11:09             ` Jeff Layton
2014-12-22  9:53               ` J. R. Okajima
  -- strict thread matches above, loose matches on Subject: below --
2015-02-10 15:55 J. Bruce Fields
2015-02-18 15:03 ` J. Bruce Fields
2015-02-24 21:22   ` J. Bruce Fields

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=20141218123223.6c88cdf1@tlielax.poochiereds.net \
    --to=jeff.layton@primarydata.com \
    --cc=bfields@fieldses.org \
    --cc=hooanon05g@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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