Date: Mon, 11 Apr 2016 14:33:41 +0900
From: Ryota Ozaki <ozak...@netbsd.org>
On Mon, Apr 11, 2016 at 12:31 PM, Taylor R Campbell
<campbell+netbsd-source-change...@mumble.net> 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.'.
