From patchwork Tue Jan 13 18:33:35 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 2194 Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n0DITiE0031481 for ; Tue, 13 Jan 2009 10:29:45 -0800 Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id CC2FE163BC1 for ; Tue, 13 Jan 2009 18:33:35 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.1.7 (2006-10-05) on dp.samba.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=3.8 tests=AWL,BAYES_00, DNS_FROM_RFC_POST,SPF_PASS autolearn=no version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from ey-out-1920.google.com (ey-out-1920.google.com [74.125.78.146]) by lists.samba.org (Postfix) with ESMTP id 53FAD163BBB for ; Tue, 13 Jan 2009 18:33:26 +0000 (GMT) Received: by ey-out-1920.google.com with SMTP id 26so17012eyw.14 for ; Tue, 13 Jan 2009 10:33:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=j2EtRDY92x/93v201L4zeVry2l2kM/WK/9RjwmDGNxw=; b=bsO262IUzDsxMBCCXIM3SF5zzue5K8JTylTtVRhGdLIJH37VbfQTphwSr785Xcsaom IRP0Qj2gErV3DVuoltSJ4s9Kab9W0uuRHqfRzEiQkTDiiaVvlG78t5Cmah11uYWGDwPm fzfBbU+Em4twG3le2nm9s/XrbMiDOgg832fl8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=eAQBLMv1DSD2MVgvmUu03r5FcgymrC02eshrZcvXZHUPhbwnKcJOTa0cyuAvmviw0e Mu4GUVFFooBHSxYKdnYX+aVZlEfbI1JCLp5pxdbGtEMfKc6ZLjQuwo3OGNkbsBbyMDnc 3iN36G7NAMBfOvmjJQxKhC2NBRObYDqK6thGg= Received: by 10.210.126.18 with SMTP id y18mr11707332ebc.5.1231871615496; Tue, 13 Jan 2009 10:33:35 -0800 (PST) Received: by 10.210.57.8 with HTTP; Tue, 13 Jan 2009 10:33:35 -0800 (PST) Message-ID: <524f69650901131033p1735d14ej8bed9730004327e9@mail.gmail.com> Date: Tue, 13 Jan 2009 12:33:35 -0600 From: "Steve French" To: "Jeff Layton" In-Reply-To: <524f69650901130906g6c00c14agb0f224a20b7f0f8b@mail.gmail.com> MIME-Version: 1.0 Content-Disposition: inline References: <200901090340.44200.linux@kukkukk.com> <524f69650901081933j518465b7g130a7f01ebdceff0@mail.gmail.com> <200901100129.32470.linux@kukkukk.com> <524f69650901101013q3ce5e727ye830cef83901ca37@mail.gmail.com> <524f69650901121204v63fb4b84q151a27479109c647@mail.gmail.com> <20090112151335.0e00c74e@tleilax.poochiereds.net> <524f69650901121223h6d068e07r86f0694bf8d6c1a8@mail.gmail.com> <524f69650901121231s3c5608bex7834d645d21d3627@mail.gmail.com> <524f69650901122000i7f4b85fu3b0d5fa43d322efc@mail.gmail.com> <524f69650901130906g6c00c14agb0f224a20b7f0f8b@mail.gmail.com> Cc: linux-fsdevel , "linux-cifs-client@lists.samba.org" , =?ISO-8859-1?Q?G=FCnter_Kukkukk?= Subject: [linux-cifs-client] Re: opendir() returns valid DIR* pointer even in case when that directory no longer exists - it has (just) been removed X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Errors-To: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org On Thu, Jan 8, 2009 at 8:40 PM, Günter Kukkukk wrote: > Hi Steve, Jeff, > > got the following failure notification on irc #samba: > > A user was updating from subversion 1.4 to 1.5, where the > repository is located on a samba share (independent of > unix extensions = Yes or No). > svn 1.4 did work, 1.5 does not. > > The user did a lot of stracing of subversion - and wrote a > testapplet to simulate the failing behaviour. > I've converted the C++ source to C and added some error cases. > > When using "./testdir" on a local file system, "result2" > is always (nil) as expected - cifs vfs behaves different here! > > ./testdir /mnt/cifs/mounted/share > > returns a (failing) valid pointer. > > Some shell scripting: > for a in a b c d e f g h i; > do ./testdir /mnt/cifs/mounted/share; done > > and > for a in a b c d e f g h i; > do ./testdir /mnt/cifs/mounted/share; sleep 1; done > > show different results too ... The cause of the problem turned out to be stranger than I expected. The sequence of events is: 1) cifs_rmdir sets the number of links to zero (probably doesn't need to do both drop_nlink and clear_nlink though, just the clear should be good enough) 2) rmdir sets the inode's time to zero which would force revalidate to go over the network if someone tried a lookup on that, but svn was redoing the getdents before the lookup 3) the app does a getdents on the parent directory, finding an existing pending search so it uses those cached results and recreates the dentry/inode for "magic_dir" from the stale results 4) lookup finds the newly created magic_dir's inode and since it was recently created (inode time is not zero) ... uses it for up to a second In the cifs_unlink case we already reset the time on the parent directory which avoids this problem, but we don't for rmdir (see fix below) - this should fix the problem I wonder if this also would fix the cleanup problem we saw in some versions of dbench Gunter, Thanks - good job on those traces and test case (which made it much easier to debug). diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 5ab9896..da70a54 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -1285,6 +1285,11 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry) cifsInode = CIFS_I(direntry->d_inode); cifsInode->time = 0; /* force revalidate to go get info when needed */ + cifsInode = CIFS_I(inode); + cifsInode->time = 0; /* force revalidate to go get info + on parent directory since link count + changed and search buffer can no longer + be cached */ direntry->d_inode->i_ctime = inode->i_ctime = inode->i_mtime = current_fs_time(inode->i_sb);