Re: CVS commit: src/sys/net

[Taylor R Campbell <campbell+netbsd-source-changes-d%mumble.net@localhost>]

   Date: Mon, 11 Apr 2016 14:33:41 +0900
   From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>

   On Mon, Apr 11, 2016 at 12:31 PM, Taylor R Campbell
   <campbell+netbsd-source-changes-d%mumble.net@localhost> wrote:
   > Nice, thanks for doing this!  Have you tried subjecting it to load,
   > with options DIAGNOSTIC?

   Yes. It passes ATF tests (that enable DIAGNOSTIC).

OK, great!  However, I wonder how much load they subject it to, if
they failed to catch the missing membar_producer in the list code
bfeore.

   >    -       LIST_REMOVE(bif, bif_next);
   >    +       PSLIST_WRITER_REMOVE(bif, bif_next);
   >    +       PSLIST_ENTRY_DESTROY(bif, bif_next);
   >
   >            BRIDGE_PSZ_PERFORM(sc);
   >
   > This order is incorrect.  You cannot destroy the entry until you have
   > confirmed all readers are done with it.  You must pserialize_perform
   > before doing PSLIST_ENTRY_DESTROY.  See the note in the pslist(9) man
   > page about this:
   >
   > [...]

   I see. (I should be claimed RTFM :-/)

I clarified some wording early on in the DESCRIPTION section of the
man page.  Please let me know if there's any verbiage in there that's
confusing or hard to follow!

   >    @@ -972,7 +984,8 @@ retry:
   >            }
   >
   >            i = 0;
   >    -       LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
   >    +       PSLIST_READER_FOREACH(bif, &sc->sc_iflist, struct bridge_iflist,
   >    +           bif_next) {
   >
   > PSLIST_WRITER_FOREACH

   Hm, so we need to choose a macro not what we do in the loop but how the loop
   is protected. Indeed in terms of use of membar.

Correct.  This is why PSLIST_READER_FOREACH is in the section `READER
OPERATIONS' of the man page, which is opened by `The following
operations may be performed on list heads and entries when the caller
is in a passively serialized read section.'.