From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Steve French" Subject: Re: opendir() returns valid DIR* pointer even in case when that directory no longer exists - it has (just) been removed Date: Tue, 13 Jan 2009 12:33:35 -0600 Message-ID: <524f69650901131033p1735d14ej8bed9730004327e9@mail.gmail.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "=?ISO-8859-1?Q?G=FCnter_Kukkukk?=" , "linux-cifs-client@lists.samba.org" , linux-fsdevel To: "Jeff Layton" Return-path: Received: from nf-out-0910.google.com ([64.233.182.190]:47761 "EHLO nf-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752577AbZAMSdh convert rfc822-to-8bit (ORCPT ); Tue, 13 Jan 2009 13:33:37 -0500 Received: by nf-out-0910.google.com with SMTP id d3so24014nfc.21 for ; Tue, 13 Jan 2009 10:33:35 -0800 (PST) In-Reply-To: <524f69650901130906g6c00c14agb0f224a20b7f0f8b@mail.gmail.com> Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Jan 8, 2009 at 8:40 PM, G=FCnter Kukkukk wr= ote: > 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 =3D 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 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 =3D CIFS_I(direntry->d_inode); cifsInode->time =3D 0; /* force revalidate to go get info wh= en needed */ + cifsInode =3D CIFS_I(inode); + cifsInode->time =3D 0; /* force revalidate to go get info + on parent directory since link count + changed and search buffer can no lon= ger + be cached */ direntry->d_inode->i_ctime =3D inode->i_ctime =3D inode->i_mtim= e =3D current_fs_time(inode->i_sb); 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). --=20 Thanks, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html