Date: Thu, 28 Oct 2010 16:20:40 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Benjamin Kaduk <kaduk@mit.edu> Cc: freebsd-hackers@freebsd.org Subject: Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9) Message-ID: <20101028132040.GV2392@deviant.kiev.zoral.com.ua> In-Reply-To: <alpine.GSO.1.10.1010272322280.19200@multics.mit.edu> References: <1288160610.4280.18.camel@apollon> <AANLkTimCN%2BY4OsJRNvdmXidLu8XPHz5RydAhe0phvgnH@mail.gmail.com> <201010270912.47076.jhb@freebsd.org> <alpine.GSO.1.10.1010271043550.19200@multics.mit.edu> <20101027153402.GS2392@deviant.kiev.zoral.com.ua> <alpine.GSO.1.10.1010272322280.19200@multics.mit.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101028132040.GV2392>