tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Making bpf MPSAFE (was Re: struct ifnet and ifaddr handling ...)



On Tue, Aug 26, 2014 at 4:49 AM, Mindaugas Rasiukevicius
<rmind%netbsd.org@localhost> wrote:
> Ryota Ozaki <ozaki-r%netbsd.org@localhost> 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
>>
>
> As Darren mentioned - there are various bugs in the code (also, malloc
> change to M_NOWAIT is unhandled).  You cannot drop the lock on the floor
> and expect the state to stay consistent.  Something has to preserve it.
>
> The following pattern applies both to the locks and pserialize(9):
>
>         pserialize_read_enter();
>         obj = lookup();
>         pserialize_read_exit();
>         // the object is volatile here, it might be already destroyed
>
> Nothing prevents obj from being destroyed after the critical path unless
> you acquire some form of a reference during the lookup.

Sure. So my next version introduces a reference counting mechanism to
ensure that a referencing object is not destroyed between locks.
And I simply use a global lock (bpf_mtx) to protect a referencing
object for non-performance-critical paths.

Here is the second patch: http://www.netbsd.org/~ozaki-r/mpsafe-bpf-v2.diff

I described how locks are used at the head of bpf.c.

I finally give up using pserialize for bpf because adapting pserialize
for bpf is difficult and also there are small benefits on performance.

  ozaki-r

>
>> BTW, I worry about there is no easy way to
>> know if a function in a critical section
>> blocks/sleeps or not. So I wrote a patch to
>> detect that: http://www.netbsd.org/~ozaki-r/debug-pserialize.diff
>> Is it meaningful?
>
> Why per-LWP rather than per-CPU diagnostic counter?  On the other hand, if
> we start adding per-CPU counters, then we might as well start implementing
> RCU. :)
>
> --
> Mindaugas


Home | Main Index | Thread Index | Old Index