From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [PATCH RFC v3] vfs: make fstatat retry once on ESTALE errors from getattr call Date: Mon, 23 Apr 2012 09:50:21 -0400 Message-ID: <20120423095021.1a91a23b@tlielax.poochiereds.net> 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> <20120423091255.00f926c4@tlielax.poochiereds.net> <20120423133412.GB13681@fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Miklos Szeredi , Malahal Naineni , Steve Dickson , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org, hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, michael.brantley-Iq/kdjr4a97QT0dZR+AlfA@public.gmane.org, sven.breuner-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org, chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, pstaubach-83r9SdEf25FBDgjK7y7TUQ@public.gmane.org, trond.myklebust-41N18TsMXrtuMpJDpNschA@public.gmane.org, rees-63aXycvo3TyHXe+LvDLADg@public.gmane.org To: "J. Bruce Fields" Return-path: In-Reply-To: <20120423133412.GB13681-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> Sender: linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-fsdevel.vger.kernel.org On Mon, 23 Apr 2012 09:34:12 -0400 "J. Bruce Fields" wrote: > On Mon, Apr 23, 2012 at 09:12:55AM -0400, Jeff Layton wrote: > > 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org] 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. > > The operation above is a lookup in D, not a lookup of D. > > > 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. > > Yes, agreed, my assertion was just that an ESTALE on a lookup of a > non-final component is probably equivalent to ENOENT. > > I'm not sure if that's what Miklos meant. > Ahh ok, sorry I misunderstood. Yeah in that case I suppose it would be ok to replace ESTALE with ENOENT. Ok, so to illustrate... Suppose we're trying to stat("/bar/baz") instead in the above example. Then we could just return ENOENT instead on an ESTALE return for the reasons that Bruce outlined. If the dir was stale, then there was a at least one point in time where we *know* that "baz" didn't exist. That doesn't seem like it'll work as a general solution though since it wouldn't apply to an ESTALE on the last component. For that we'd need to do something different -- retry the operation in some form, but it might be potential optimization in the path walking code to avoid retrying in some cases. -- Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html