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 ...

[Darren Reed]

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