Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Thu, Nov 13, 2014 at 1:26 PM, Taylor R Campbell
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
>    Date: Thu, 13 Nov 2014 12:43:26 +0900
>    From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
>
>    Here is a new patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>
>    I think the patch reflects rmind's suggestions:
>    - Use pserialize for IFNET_FOREACH
>      - but use a lock for blockable/sleepable critical sections
>    - cpu_intr_p workaround for HW interrupt
>
>    Any comments?
>
> Hmm...some quick notes from a non-expert in sys/net:
>
> - You call malloc(M_WAITOK) while the ifnet lock is held, in
>   if_alloc_sadl_locked, which is not allowed.

Oh, I didn't know that restriction. LOCKDEBUG didn't correct me...

>
> - You call copyout in a pserialize read section, in ifconf, which is
>   not allowed because copyout may block.

Which one? I think I've fixed such usages this time.

>
> - I don't know what cpu_intr_p is working around but it's probably not
>   a good idea!

Yes :-| We have to fix that in the future, but it works as same as it is
until we get rid of all KERNE_LOCK.

>
> Generally, all that you are allowed to do in a pserialize read section
> is read a small piece of information, or grab a reference to a data
> structure which you are then going to use outside the read section.

Yes. I'm implementing a facility of the latter for ifunit:
http://www.netbsd.org/~ozaki-r/ifget-ifput.diff

>
> I don't think it's going to be easy to scalably parallelize this code
> without restructuring it, unless as a stop-gap you use a heaver-weight
> reader-writer lock like the prwlock at
> <https://www.NetBSD.org/~riastradh/tmp/20140517/rwlock/prwlock.c>.
> (No idea how much overhead this might add.)

Could you elaborate how to use it?

Thanks,
  ozaki-r