Re: bridge(4): BRIDGE_MPSAFE by default and applying psref(9)

[christos%astron.com@localhost (Christos Zoulas)]

In article <CAKrYomh23MGVWK0USFTU_+LCbY3Yooh1M8R2QV3dA2EKS6N+_Q%mail.gmail.com@localhost>,
Ryota Ozaki  <ozaki-r%netbsd.org@localhost> wrote:
>Hi,
>
>I prepared two patches for bridge(4):
>  http://www.netbsd.org/~ozaki-r/remove-BRIDGE_MPSAFE.diff
>  http://www.netbsd.org/~ozaki-r/psref-bridge.diff
>
>The former removes BRIDGE_MPSAFE switch and enables the
>MP-safe code by default. After introducing softint-based
>if_input, bridge can run in parallel even without NET_MPSAFE,
>so I think we need to always enable it.
>
>The latter applies psref(9) to bridge. I confirmed the new
>implementation survives load test, which includes repeating
>bridge creations/deletions and member interface
>additions/removals, over several hours.
>
>Note that I notice that we need to tweak shmif of the
>rump kernel because it seems that the interrupt handler
>of shmif can migration between CPUs and so the behavior
>violates a contract of psref. We can fix it by applying
>if_percpuq to shmif, but I'm not sure that's the way
>to go. (I'll ask pooka about the issue.)
>
>Anyway any comments on the patches?

Why bother? Just define them empty?

+#define ACQUIRE_GLOBAL_LOCKS(s)	do { s = 0; } while (0)
+#define RELEASE_GLOBAL_LOCKS(s)	do { (void)s; } while (0)

Nobody sets error here.

+	sc->sc_iflist_psref.bip_psz = pserialize_create();
+	if (sc->sc_iflist_psref.bip_psz == NULL)
+		panic("%s: pserialize_create %d\n", __func__, error);

Early return here:

 	if (bif != NULL) {
-		if (bif->bif_waiting)
-			bif = NULL;
-		else
-			atomic_inc_32(&bif->bif_refs);
+		psref_acquire(psref, &bif->bif_psref,
+		    sc->sc_iflist_psref.bip_class);

	if (bif == NULL)
		return NULL;
	psref_acquire(psref, &bif->bif_psref, sc->sc_iflist_psref.bip_class);
	return bif;

christos