From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?q?G=FCnter_Kukkukk?= Subject: Re: [linux-cifs-client] Re: opendir() returns valid DIR* pointer even in case when that directory no longer exists - it has (just) been removed Date: Wed, 14 Jan 2009 04:03:38 +0100 Message-ID: <200901140403.39372.linux@kukkukk.com> References: <200901090340.44200.linux@kukkukk.com> <524f69650901130906g6c00c14agb0f224a20b7f0f8b@mail.gmail.com> <524f69650901131033p1735d14ej8bed9730004327e9@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "Steve French" , "Jeff Layton" , linux-fsdevel To: linux-cifs-client@lists.samba.org Return-path: Received: from moutng.kundenserver.de ([212.227.126.187]:60279 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751802AbZANDG1 convert rfc822-to-8bit (ORCPT ); Tue, 13 Jan 2009 22:06:27 -0500 In-Reply-To: <524f69650901131033p1735d14ej8bed9730004327e9@mail.gmail.com> Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Am Dienstag, 13. Januar 2009 schrieb Steve French: > On Thu, Jan 8, 2009 at 8:40 PM, G=FCnter 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 =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 ... >=20 > 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 b= e > good enough) >=20 > 2) rmdir sets the inode's time to zero which would force revalidate t= o > go over the network if someone tried a lookup on that, but svn was > redoing the getdents before the lookup >=20 > 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 >=20 >=20 > 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 >=20 > 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 = when > needed */ > + cifsInode =3D CIFS_I(inode); > + cifsInode->time =3D 0; /* force revalidate to go get info > + on parent directory since link cou= nt > + changed and search buffer can no l= onger > + be cached */ > direntry->d_inode->i_ctime =3D inode->i_ctime =3D inode->i_mt= ime =3D > current_fs_time(inode->i_sb); >=20 > I wonder if this also would fix the cleanup problem we saw in some > versions of dbench >=20 > Gunter, > Thanks - good job on those traces and test case (which made it much > easier to debug). >=20 Hi Steve, your patch works here on kernels - 2.6.22.18-0.2-default - 2.6.28 it seems to be an outstanding one, which is fixed now. :-) I've send your patch to the bug reporter (to build a new cifs vfs module...) - till now no response whether subversion 1.5 is failing or not. I'm sure the bug is fixed... Cheers, G=FCnter -- 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