From owner-freebsd-hackers@FreeBSD.ORG Thu Oct 28 13:21:12 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 96F78106566B for ; Thu, 28 Oct 2010 13:21:12 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 302718FC18 for ; Thu, 28 Oct 2010 13:21:11 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id o9SDKe3i087007 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 28 Oct 2010 16:20:40 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id o9SDKepQ017420; Thu, 28 Oct 2010 16:20:40 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id o9SDKe0D017419; Thu, 28 Oct 2010 16:20:40 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 28 Oct 2010 16:20:40 +0300 From: Kostik Belousov To: Benjamin Kaduk Message-ID: <20101028132040.GV2392@deviant.kiev.zoral.com.ua> References: <1288160610.4280.18.camel@apollon> <201010270912.47076.jhb@freebsd.org> <20101027153402.GS2392@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rBv5AOddlEkG746b" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Oct 2010 13:21:12 -0000 --rBv5AOddlEkG746b Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 27, 2010 at 11:57:31PM -0400, Benjamin Kaduk wrote: > On Wed, 27 Oct 2010, Kostik Belousov wrote: >=20 > >On Wed, Oct 27, 2010 at 10:59:56AM -0400, Benjamin Kaduk wrote: > >>[1] The old (racy) function is osi_TryEvictVCache, here: > >>http://git.openafs.org/?p=3Dopenafs.git;a=3Dblob;f=3Dsrc/afs/FBSD/osi_v= cache.c;h=3Dc2060c74f0155a610d2ea94f3c7f508e8ca4373a;hb=3DHEAD > >The function looks very strange for much more serious reasons. Why do you > >try to manage the vnode revocation in the filesystem module at all ? >=20 > I am still becoming familiar with the AFS code, but I think this is=20 > largely due to a difference in the vfs structure that AFS has been using= =20 > and the FreeBSD standard. E.g. vop_inactive/vop_reclaim do not actually= =20 > free filesystem-specific resources, instead keeping a free list of=20 > "vcache" entries. So, the original authors of this AFS code were=20 > approaching the problem in a somewhat different way. > Therefore, are somewhat-orthogonal pools of vcaches and vnodes (with some= =20 > intersection). If the vcaches are all in use in use, there is a routine= =20 > which tries to "shake some loose"; if it can free up vcaches, their=20 > associated vnodes also need to be cleaned up in some fashion. It may be= =20 > that no additional code is actually needed to do this, though -- I am not= =20 > sure. I think the best would be to move the code to vop_reclaim(), possibly calli= ng vgone() from vop_inactivate(). >=20 > >VFS will (assumedly in a right way) revoke and destroy non-referenced > >vnodes when needed. > > >=20 > You are, in essence, suggesting that the body of the conditional should= =20 > be just "return 1;"? (In my incomplete testing the vref() call was alway= s=20 > made.) See above. >=20 > >Anyway, you need to hold vnode lock before checking for the vnode=20 > >refcounter. > >See the vfs_subr.c:vlrureclaim() for the correct dance with=20 > >vholdl()/vn_lock() > >sequence that does the revocation I mentioned in the race-free way. >=20 > Unless I am mistaken, the path you refer to still uses vgonel(), which is= =20 > not available to me. Is it safe to drop and reaquire the interlock=20 > between the vgone and vdrop? vgonel() use is an optimization to avoid dropping/reacquiring the vnode interlock. Correct code can call vdrop() after vgone() call, without doing anything with interlock. Please note that vnode lock should be dropped between vgone and vdrop. --rBv5AOddlEkG746b Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAkzJeKYACgkQC3+MBN1Mb4hsrgCdFQG74NlbbWc11/wK8dqwOrRq 1rcAoINyikHIyz7RRAPGQOdFSXlncjPE =cEbt -----END PGP SIGNATURE----- --rBv5AOddlEkG746b--