svn commit: r186187 - head/sys/net

[Max Laier]

On Tuesday 16 December 2008 18:03:22 Robert Watson wrote:
> Author: rwatson
> Date: Tue Dec 16 17:03:22 2008
> New Revision: 186187
> URL: http://svn.freebsd.org/changeset/base/186187
>
> Log:
>   A few locking fixes and cleanups to pfil hook registration,
>   unregistration, and execution:
>
>   - Add some brackets for clarity and trim a bit of vertical whitespace.
>   - Remove comments that may not contribute to clarity, such as "Lock"
>     before acquiring a lock and "Get memory" before allocating memory.

These were meant as sectioning comments as pfil_add_hook grew larger and 
larger in order to allow WAITOK allocations.  Should probably have been "/* 1. 
get memory */" etc.

>   - During hook registration, don't drop pfil_list_lock between checking
>     for a duplicate and registering the hook, as this leaves a race
>     condition by failing to enforce the "no duplicate hooks" invariant.
>   - Don't lock the hook during registration, since it's not yet in use.

Shouldn't the WLOCK be obtained regardless in order to obtain a memory barrier 
for the reading threads?  pfil_run_hooks doesn't interact with the LIST_LOCK 
so it's unclear in what state the hook head will be when the hook head is 
first read.

>   - Document assumption that hooks will be quiesced before being
>     unregistered.

This should probably go to pfil(9), too.

>   - Don't write-lock hooks during removal because they are assumed
>     quiesced.

Again, this lock also ensures that we see all the changes done in other 
threads to the pfil head recently - otherwise we might leak a hook function 
pointer or even fault due to list inconsistencies.  Again, add/del_hook don't 
interact with the LIST_LOCK.

>   - Rename "done" label to "locked_error" to be clear that it's an error
>     path on the way out of hook execution.
>
>   MFC after:	pretty soon

-- 
/"\  Best regards,                      | mlaier at freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mlaier at EFnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News