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

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

On Wed, Nov 19, 2014 at 3:11 AM, Taylor R Campbell
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
>    Date: Tue, 18 Nov 2014 13:31:49 +0900
>    From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
>
>    I have to say sorry for my bad writing; I misled you about
>    "free an ifnet object" due to my forgetfulness about the fact that
>    an ifnet object is normally embedded into a softc of each driver
>    (via ethercom) in NetBSD. (I.e., we cannot free an ifnet object
>    independently.) So with the current implementation, if_detach
>    have to wait for ifput releasing the ifnet object, to prevent
>    the driver from freeing sc. So it's a little bit difficult to use
>    "GC objects later" strategy for ifnet objects.
>
> Normally in cases like this, where there is some resource that many
> things can use (various parts of the network stack) but only one owns
> (the NIC driver), the resource can be destroyed when the owner chooses
> to destroy it (inside the NIC driver's *_detach routine) only after
> (a) locking out new users, and then (b) waiting for all extant users
> to drain:

Yes. That's what I'm doing for ifnet with ifget/ifput. Nevertheless,
I'd copy the optimization of mumble_put where refcnt > 1 :)

>
> void
> mumble_destroy(struct mumble *m)
> {
>
>         mutex_enter(&m->m_lock);
>         m->m_flags |= MUMBLE_DYING;     /* Lock out new users.  */
>         while (0 < m->m_refcnt)         /* Wait for extant ones to drain.  */
>                 cv_wait(&m->m_cv, &m->m_lock);
>         /* We now have exclusive access.  */
>         mutex_exit(&m->m_lock);
>         mutex_destroy(&m->m_lock);
>         cv_destroy(&m->m_cv);
>         pool_put(&mumble_pool, m);
> }
>
> int
> mumble_get(unsigned id, struct mumble **mp)
> {
>         int error = ENOENT;
>
>         MUMBLE_FOREACH(m) {
>                 if (m->m_id == id) {
>                         mutex_enter(&m->m_lock);
>                         if (!ISSET(m->m_flags, MUMBLE_DYING)) {
>                                 atomic_inc_uint(&m->m_refcnt);
>                                 error = 0;
>                                 *mp = m;
>                         }
>                         mutex_exit(&m->m_lock);
>                         break;
>                 }
>         }
>
>         return error;
> }
>
> void
> mumble_put(struct mumble *m)
> {
>         unsigned refcnt;
>
>         do {
>                 refcnt = m->m_refcnt;
>                 if (refcnt == 1) {
>                         mutex_enter(&m->m_lock);
>                         refcnt = atomic_dec_uint_nv(&m->m_refcnt);
>                         if (refcnt == 0)
>                                 cv_broadcast(&m->m_cv);
>                         mutex_exit(&m->m_lock);
>                 }
>         } while (atomic_cas_uint(&m->m_refcnt, refcnt, refcnt - 1) != refcnt);
> }

BTW, do we need break after mutex_exit? Otherwise, m->m_refcnt is likely
to be decremented twice when refcnt == 1.

Thanks,
  ozaki-r