Hello,

</snip>
> >     I'm not sure about your diff see below.
> 
> visa@ pointed out that PF_LOCK will introduce a potential sleep inside 
> if_vinput(),
> making this unsafe, so here's a less optimistic diff where only the vlan list
> traversal is inside the SMR read section, and vlan_softcs are still 
> refcounted.
> I probably need to rework some other unsent SRP->SMR diffs now too.

    less optimistic variant looks better. Though I have one
    question related to refcnt_finalize().

</snip>

> @@ -257,6 +239,7 @@ vlan_clone_destroy(struct ifnet *ifp)
>  
>       ether_ifdetach(ifp);
>       if_detach(ifp);
> +     smr_barrier();
>       refcnt_finalize(&sc->sc_refcnt, "vlanrefs");
    ^^^^
    I think a protection should be added so there will be at most 1 thread
    calling to refcnt_finalize(). I've noticed vlan_clone_destroy() does
    ->sc_dead = 1; couple line earlier under protection of NET_LOCK(). Would it
    make sense to do if (->sc_dead == 1) first? We would just bail out if
    ->sc_dead will be set already, because there is vlan_clone_destroy() in
    progress already.

    Otherwise the change looks good.

thanks and
regards
sashan

Reply via email to