Return-Path: linux-nfs-owner@vger.kernel.org Received: from mx1.redhat.com ([209.132.183.28]:34561 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752454Ab2DWNNZ (ORCPT ); Mon, 23 Apr 2012 09:13:25 -0400 Date: Mon, 23 Apr 2012 09:12:55 -0400 From: Jeff Layton To: "J. Bruce Fields" Cc: Miklos Szeredi , Malahal Naineni , Steve Dickson , linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk, hch@infradead.org, michael.brantley@deshaw.com, sven.breuner@itwm.fraunhofer.de, chuck.lever@oracle.com, pstaubach@exagrid.com, trond.myklebust@fys.uio.no, rees@umich.edu Subject: Re: [PATCH RFC v3] vfs: make fstatat retry once on ESTALE errors from getattr call Message-ID: <20120423091255.00f926c4@tlielax.poochiereds.net> In-Reply-To: <20120423130009.GA13681@fieldses.org> References: <1334316311-22331-1-git-send-email-jlayton@redhat.com> <1334749927-26138-1-git-send-email-jlayton@redhat.com> <20120420104055.511e15bc@tlielax.poochiereds.net> <4F91C49D.8070908@RedHat.com> <20120420203725.GA3512@us.ibm.com> <20120420171314.73801874@corrin.poochiereds.net> <20120423080012.7c23ef24@tlielax.poochiereds.net> <20120423130009.GA13681@fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: On Mon, 23 Apr 2012 09:00:09 -0400 "J. Bruce Fields" wrote: > On Mon, Apr 23, 2012 at 08:00:12AM -0400, Jeff Layton wrote: > > On Sun, 22 Apr 2012 07:40:57 +0200 > > Miklos Szeredi wrote: > > > > > On Fri, Apr 20, 2012 at 11:13 PM, Jeff Layton wrote: > > > > On Fri, 20 Apr 2012 15:37:26 -0500 > > > > Malahal Naineni wrote: > > > > > > > >> Steve Dickson [SteveD@redhat.com] wrote: > > > >> > > 2) if we assume that it is fairly representative of one, how can we > > > >> > > achieve retrying indefinitely with NFS, or at least some large finite > > > >> > > amount? > > > >> > The amount of looping would be peer speculation. If the problem can > > > >> > not be handled by one simple retry I would say we simply pass the > > > >> > error up to the app... Its an application issue... > > > >> > > > >> As someone said, ESTALE is an incorrect errno for a path based call. > > > >> How about turning ESTALE into ENOENT after a retry or few retries? > > > >> > > > > > > > > It's not really the same thing. One could envision an application > > > > that's repeatedly renaming a new file on top of another one. The file > > > > is never missing from the namespace of the server, but you could still > > > > end up getting an ESTALE. > > > > > > > > That would break other atomicity guarantees in an even worse way, IMO... > > > > > > For directory operations ESTALE *is* equivalent to ENOENT if already > > > retrying with LOOKUP_REVAL. Think about it. Atomic replacement by > > > another directory with rename(2) is not an excuse here actually. > > > Local filesystems too can end up with IS_DEAD directory after lookup > > > in that case. > > > > > > > Doesn't that violate POSIX? rename(2) is supposed to be atomic, and I > > can't see where there's any exception for that for directories. > > Hm, but that only allows atomic replacement of the last component of a > path. > > Suppose you're looking up a path, you've so far reached intermediate > directory "D", and the next step of the lookup (of some entry in D) > returns ESTALE. Then either: > > - D has since been unlinked, and ENOENT is obviously right. > - D was unlinked and then replaced by something else, in which > case there was still a moment when ENOENT was correct. > - D was replaced atomically by a rename. But for the rename to > work it must have been replacing an empty directory, so there > was still a moment when ENOENT would have been correct. I don't think so...D should always exist in the namespace, so ENOENT would not be correct. Just because it was empty doesn't mean that it didn't exist... > (Exception: if D was actually a regular file or some other > non-directory object, then ENOTDIR would be the right error: > but if you're able to get at least object type atomically with > a lookup, then you should have noticed this already on lookup > of D.) > > I think that's what Miklos meant? > > --b. Here's an example -- suppose we have two directories: /foo and /bar. /bar is empty. We call: rename("/foo","/bar"); ...and at the same time, someone is calling: stat("/bar"); ...the calls race and in this condition the stat() gets ESTALE back -- /bar got replaced after we did the lookup. According to POSIX, the name "/bar" should never be absent from the namespace in this situation, so I'm not sure I understand why returning ENOENT here would be acceptable. -- Jeff Layton