On 20/12/16(Tue) 16:33, Mike Belopuhov wrote:
> Attach:
>
> splassert: sowakeup: want 1 have 0
> Starting stack trace...
> sowakeup() at sowakeup+0x41
> sorwakeup() at sorwakeup+0xb4
> route_input() at route_input+0x2d7
> if_attach() at if_attach+0x58
> xnf_attach() at xnf_attach+0x45f
> config_attach() at config_attach+0x1bc
> xen_attach_device() at xen_attach_device+0x146
> xen_hotplug() at xen_hotplug+0x23f
> taskq_thread() at taskq_thread+0x6c
> end trace frame: 0x0, count: 248
> End of stack trace.
>
> Detach:
>
> splassert: sowakeup: want 1 have 0
> Starting stack trace...
> sowakeup() at sowakeup+0x41
> sorwakeup() at sorwakeup+0xb4
> route_input() at route_input+0x2d7
> if_detach() at if_detach+0x27e
> xnf_detach() at xnf_detach+0x4d
> config_detach() at config_detach+0x13c
> xen_hotplug() at xen_hotplug+0x2e5
> taskq_thread() at taskq_thread+0x6c
> end trace frame: 0x0, count: 249
> End of stack trace.
> xnf2 detached
>
> The taskq running xen_hotplug is KERNEL_LOCK'ed. The diff below solves
> the issue, but is it any good?
>
> diff --git sys/net/if.c sys/net/if.c
> index 1e103564710..a5c25b89560 100644
> --- sys/net/if.c
> +++ sys/net/if.c
> @@ -525,11 +525,11 @@ if_attach(struct ifnet *ifp)
> {
> int s;
>
> - s = splsoftnet();
> + NET_LOCK(s);
> if_attach_common(ifp);
> TAILQ_INSERT_TAIL(&ifnet, ifp, if_list);
> if_attachsetup(ifp);
> - splx(s);
> + NET_UNLOCK(s);
You'll need to release the lock before calling ifc->ifc_create in
if_clone_create() and do the same dance in if_clone_destroy().
But I think that's the way to go. If you can create/destroy
pseudo-interface without panicing we're good.
> void
> @@ -941,6 +941,7 @@ if_detach(struct ifnet *ifp)
>
> ifq_clr_oactive(&ifp->if_snd);
>
> + rw_enter_write(&netlock);
> s = splnet();
> /* Other CPUs must not have a reference before we start destroying. */
> if_idxmap_remove(ifp);
> @@ -1017,6 +1018,7 @@ if_detach(struct ifnet *ifp)
> /* Announce that the interface is gone. */
> rt_ifannouncemsg(ifp, IFAN_DEPARTURE);
> splx(s);
> + rw_exit_write(&netlock);
>
> ifq_destroy(&ifp->if_snd);
> }
Please keep the NET_LOCK/UNLOCK() macro. The places where we
grab/release the `netlock' directly are places that need attention.
