As the upper half of this thread has turned into a style(9) bikeshed, let me replay the lower half and add more locking folks.
The change in question is here: http://svn.freebsd.org/viewvc/base/head/sys/net/pfil.c?r1=173904&r2=186187 On Tuesday 16 December 2008 19:20:40 Robert Watson wrote: > On Tue, 16 Dec 2008, Max Laier wrote: ... > >> - 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. > > Hmm. Interesting observation. However, that approach is not consistent > with lots of other code, so possibly that other code needs to change as > well. For example, ip_init() initializes lots of other global data > structures, including the fragment reassembly queue and protocol switch, > without acquiring locks in such a way as to force visibility on other CPUs. > > I've CC'd John Baldwin, who might be able to comment on this issue more > generally. Should we be, for example, calling { IPQ_LOCK(); IPQ_UNLOCK(); > } after initializing the IP reassembly queues to make sure that > initialization is visible to other CPUs that will be calling IPQ_LOCK() > before using it? > ... > >> - 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. > > We may run into similar problems with VIMAGE destructors now that we are > interested in tearing down network stacks. Again, perhaps jhb can opine > some. -- /"\ Best regards, | mla...@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mla...@efnet / \ ASCII Ribbon Campaign | Against HTML Mail and News _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
