From owner-cvs-src@FreeBSD.ORG Sat Aug 28 21:19:16 2004 Return-Path: Delivered-To: cvs-src@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 680) id 9681F16A4CF; Sat, 28 Aug 2004 21:19:16 +0000 (GMT) Date: Sat, 28 Aug 2004 21:19:16 +0000 From: Darren Reed To: Max Laier Message-ID: <20040828211916.GA63019@hub.freebsd.org> References: <200408271516.i7RFGO8L061926@repoman.freebsd.org> <200408272253.14317.max@love2party.net> <20040828164043.GA86995@hub.freebsd.org> <200408281950.33363.max@love2party.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200408281950.33363.max@love2party.net> User-Agent: Mutt/1.4.1i cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: Andre Oppermann cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/share/man/man4 ipfirewall.4 src/share/man/man9 pfil.9 src/sys/alpha/conf GENERIC src/sys/amd64/conf GENERIC src/sys/conf NOTES files options src/sys/i386/conf GENERIC src/sys/ia64/conf GENERIC SKI src/sys/modules/bridge Makefile ... X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Aug 2004 21:19:16 -0000 On Sat, Aug 28, 2004 at 07:50:26PM +0200, Max Laier wrote: > > > The check inside pfil_run_hooks() was introduced > > > before PFIL_HOOKS got part of GENERIC to avoid the lock operation for > > > empty hooks. It is okay to remove it now that we do the check earlier. A > > > wrapper is called for, nontheless! > > > > Right, it needs removing and wasn't - just don't say it is right how it is. > > Never did. Though I really do not belive that it incurres a performance loss > as ph_busy_count will be cached together with ph_want_write anyway ... stupid > micro-optimizations ... It's not micro-optimising that I'm worried about, it's that checking the same thing twice in a row being suggestive that someone is changing code without having a full grasp of what they're doing. > > I wonder if ph_busy_lock could be made to disappear and its use replaced > > with a check for TAILQ_EMPTY() ? This would nto require a lock/unlock. > > While TAILQ_EMPTY is okay it'd require a direction lookup and that would > result in more overhead ... It shouldnt. Despite the macro taking a pointer, the tailq head is in the pfil_head struct, itself, so the end result should be the same as checking ph_busy_count ? Oh, except for 64bit systems ;) Darren