Re: Simplify bridge(4)

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

   Date: Mon, 28 Mar 2016 16:07:19 +0900
   From: Ryota Ozaki <ozaki.ryota%gmail.com@localhost>

   On Fri, Mar 25, 2016 at 11:40 PM, Taylor R Campbell
   <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
   > No need for the _PSZ variants here: the bridge list is used only under
   > an exclusive lock.
   >
   > Actually...  I don't see any users of that list!  Do we need it any
   > more?  Why was it there to begin with?
   >
   > Looks like it was added in the first commit and never used.  Let's get
   > rid of it!  (Separate commit.)

   Heh, that's true. Removed!

Great!

   So patches are updated:
     http://www.netbsd.org/~ozaki-r/bridge-psz-list.diff
     http://www.netbsd.org/~ozaki-r/bridge-default-mpsafe.diff

Thanks, I'll take a look when I have a chance.

   BTW, I found a possible race condition in bridge_output:
   [...]
   bif can be freed after bridge_release_member on another CPU,
   however, LIST_FOREACH_READER touches bif after it.

   IIUC, using psref solves the issue like this:

       BRIDGE_PSZ_RENTER(s);
       bif = (&sc->sc_iflist)->lh_first;
       while (bif != LIST_END(&sc->sc_iflist)) {
           struct bridge_iflist *next;

           membar_datadep_consumer();

           psref_acquire(&psref, &bif->target);
           BRIDGE_PSZ_REXIT(s);

           // sleepable operations

           BRIDGE_PSZ_RENTER(s);
           next = bif->field.le_next;
           psref_release(&psref, &bif->target);
           bif = next;
       }
       BRIDGE_PSZ_REXIT(s);

   So I'm thinking the above patches should be committed
   after introducing psref.

You don't *need* to switch to psref for this to work: it is sufficient
for bridge_release_member to work inside a pserialize read section.
Currently the only reason it doesn't work inside a pserialize read
section is that it acquires an adaptive lock -- but it doesn't look
like it actually needs to acquire that lock.

However, as an aside, I am a little puzzled by the protocol that
bridge_try_hold_bif, bridge_release_member, and bridge_delete_member
follow.

(Also: Use LIST_FIRST and LIST_NEXT, not .lh_first and .le_next!)