Hello,

On Sun, Jan 26, 2020 at 10:06:24AM +1100, Jonathan Matthew wrote:
> Converting vlan(4) to use SMR instead of SRP to protect the instance lists
> is pretty simple.  vlan_input() doesn't keep a reference to vlan_softcs 
> outside
> the read critical section, so we don't need to reference count them any more.
> After the smr_barrier() in vlan_clone_destroy, all pointers to the vlan that
> were obtained from the instance lists have been dropped, so it's safe to free 
> it.
> 
> ok?

    I'm not sure about your diff see below.

</snip>
> @@ -415,7 +393,8 @@ vlan_input(struct ifnet *ifp0, struct mb
>       tag = EVL_VLANOFTAG(m->m_pkthdr.ether_vtag);
>  
>       list = &tagh[TAG_HASH(tag)];
> -     SRPL_FOREACH(sc, &sr, list, sc_list) {
> +     smr_read_enter();
> +     SMR_SLIST_FOREACH(sc, list, sc_list) {
>               if (ifp0->if_index == sc->sc_ifidx0 && tag == sc->sc_tag &&
>                   etype == sc->sc_type)
>                       break;
> @@ -456,14 +435,14 @@ vlan_input(struct ifnet *ifp0, struct mb
>  
>       if_vinput(&sc->sc_if, m);
        ^^^
    here we call if_input() inside a read section. SMR_CALL(9)
    manpage says;

        smr_read_enter(), and exited with smr_read_leave().  These routines
        never block.  Sleeping is not allowed within SMR read-side critical
        section.

    If you can grant if_vinput() won't sleep, then your diff is OK. But I'm not
    sure about it. However I must admit I might be missing something here.

thanks and
regards
sashan

Reply via email to