From owner-freebsd-hackers@FreeBSD.ORG Thu Oct 28 03:57:35 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 B778E106564A for ; Thu, 28 Oct 2010 03:57:35 +0000 (UTC) (envelope-from kaduk@mit.edu) Received: from dmz-mailsec-scanner-8.mit.edu (DMZ-MAILSEC-SCANNER-8.MIT.EDU [18.7.68.37]) by mx1.freebsd.org (Postfix) with ESMTP id 6F73B8FC17 for ; Thu, 28 Oct 2010 03:57:35 +0000 (UTC) X-AuditID: 12074425-b7c98ae000000a04-5d-4cc8f4aebf81 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35]) by dmz-mailsec-scanner-8.mit.edu (Symantec Brightmail Gateway) with SMTP id E4.5C.02564.EA4F8CC4; Wed, 27 Oct 2010 23:57:34 -0400 (EDT) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id o9S3vYCp020611; Wed, 27 Oct 2010 23:57:34 -0400 Received: from multics.mit.edu (MULTICS.MIT.EDU [18.187.1.73]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id o9S3vWFT020124 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Wed, 27 Oct 2010 23:57:34 -0400 (EDT) Received: (from kaduk@localhost) by multics.mit.edu (8.12.9.20060308) id o9S3vWNh015943; Wed, 27 Oct 2010 23:57:32 -0400 (EDT) Date: Wed, 27 Oct 2010 23:57:31 -0400 (EDT) From: Benjamin Kaduk To: Kostik Belousov In-Reply-To: <20101027153402.GS2392@deviant.kiev.zoral.com.ua> Message-ID: References: <1288160610.4280.18.camel@apollon> <201010270912.47076.jhb@freebsd.org> <20101027153402.GS2392@deviant.kiev.zoral.com.ua> User-Agent: Alpine 1.10 (GSO 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Brightmail-Tracker: AAAAAA== 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 03:57:35 -0000 On Wed, 27 Oct 2010, Kostik Belousov wrote: > 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=openafs.git;a=blob;f=src/afs/FBSD/osi_vcache.c;h=c2060c74f0155a610d2ea94f3c7f508e8ca4373a;hb=HEAD > 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 ? I am still becoming familiar with the AFS code, but I think this is largely due to a difference in the vfs structure that AFS has been using and the FreeBSD standard. E.g. vop_inactive/vop_reclaim do not actually free filesystem-specific resources, instead keeping a free list of "vcache" entries. So, the original authors of this AFS code were approaching the problem in a somewhat different way. Therefore, are somewhat-orthogonal pools of vcaches and vnodes (with some intersection). If the vcaches are all in use in use, there is a routine which tries to "shake some loose"; if it can free up vcaches, their associated vnodes also need to be cleaned up in some fashion. It may be that no additional code is actually needed to do this, though -- I am not sure. > VFS will (assumedly in a right way) revoke and destroy non-referenced > vnodes when needed. > You are, in essence, suggesting that the body of the conditional should be just "return 1;"? (In my incomplete testing the vref() call was always made.) > Anyway, you need to hold vnode lock before checking for the vnode refcounter. > See the vfs_subr.c:vlrureclaim() for the correct dance with vholdl()/vn_lock() > sequence that does the revocation I mentioned in the race-free way. Unless I am mistaken, the path you refer to still uses vgonel(), which is not available to me. Is it safe to drop and reaquire the interlock between the vgone and vdrop? Many thanks, Ben