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

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

On Tue, Nov 18, 2014 at 11:38 AM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> On Mon, Nov 17, 2014 at 11:39 PM, Masao Uebayashi <uebayasi%gmail.com@localhost> wrote:
>> On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>>> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>>
>> There are many copyout()'s while IFNET_LOCK() is taken.
>
> I'm trying to get rid of them.

I'm back to this task :)

Some copyout can be pull out of critical sections straightforwardly
but others cannot. For the latter, I end up using refcount to
deal with sleep/block operations. This is a typical usage of psz
and refcount for such cases:

  IFNET_RENTER(s);
  IFNET_FOREACH_SAFE(ifp) {
    refcount_hold(ifp->if_refcount);
    IFNET_REXIT(s);

    /* sleepable operations on ifp */

    refcount_release(ifp->if_refcount);
    IFNET_RENTER(s);
  }
  IFNET_REXIT(s);

After IFNET_REXIT inside the loop, refcount prevents an ifnet
object from being freed. See the below code; a writer LWP
stops at refcount_wait before trying to free the ifnet object.

  IFNET_LOCK();
  ifindex2ifnet[ifp->if_index] = NULL;
  TAILQ_REMOVE(&ifnet_list, ifp, if_list);

  pserialize_perform(ifnet_psz);
  refcount_wait(ifp->if_refcount);
  IFNET_UNLOCK();

  /* ifp will be freed in the end */

OTOH, the ifp may be freed after refcount_release so we have
to use IFNET_FOREACH_SAFE instead of IFNET_FOREACH to get
a next ifp safely.

Any comments?

Here is a patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
Note that there remains IFNET_LOCK yet (only in if.c). I'm trying
to remove them now.

BTW, the patch is a bit big. If you want to see individual commits,
find them at https://github.com/ozaki-r/netbsd-src/tree/psz-ifnet .

Thanks,
  ozaki-r