On Sat, Aug 9, 2014 at 8:26 PM, Darren Reed <[email protected]> wrote: > On 7/08/2014 7:49 PM, Ryota Ozaki wrote: >> Hi, >> >> I thought I need more experience of pserialize >> (and lock primitives) to tackle ifnet work. >> So I suspended the work and now I am trying >> another easier task, bpf. >> >> http://www.netbsd.org/~ozaki-r/mpsafe-bpf.diff >> > > This code has race conditions built into it where locks are > dropped for others to be acquiredwith no knowledge stored > anywhere that a critical section of code is being executed:
Right. So I use a global lock and a reference counting mechanism in the new patch(*) as I said in the mail I just sent. (*) http://www.netbsd.org/~ozaki-r/mpsafe-bpf-v2.diff I also modified such codes to not depend on that a target object isn't changed after releasing a lock by, for example, moving a pointer reference of the object to inside a critical section. > > > + mutex_exit(d->bd_lock); > + KERNEL_LOCK(1, NULL); > > What you need here is a perimeter mechanism that doesn't > hold any locks over the inner section of code. I may misunderstand what you mean though, KERNEL_LOCK (splnet) is intended to replicate lock conditions for non-MP-safe functions outside (ifpromisc in this case). Once we make entire networking code MP-safe, we can get rid of KERNEL_LOCK. > > This needs to be fixed: > > error = tsleep(d, PRINET|PCATCH, "bpf", d->bd_rtout); > > - it should actually be a cv_timedwait_sig() on bd_lock. Sure. I changed to use it. Thanks, ozaki-r > > Darren >
