I missed explanation.
All the code protected by this rwlock is KERNEL_LOCK()’ed. The context
switches which can be caused by memory allocation or NET_LOCK() dances
within pppx_add_session() and pppx_if_destroy() are not under acquired
pppx_if_lk. So there is no concurrent access to structures it’s protects.
> On 11 Apr 2020, at 16:59, Vitaliy Makkoveev <henscheltig...@yahoo.com> wrote:
>
> It protects nothing.
>
> Index: sys/net/if_pppx.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 if_pppx.c
> --- sys/net/if_pppx.c 10 Apr 2020 07:36:52 -0000 1.83
> +++ sys/net/if_pppx.c 11 Apr 2020 10:55:00 -0000
> @@ -165,7 +165,6 @@ pppx_if_cmp(const struct pppx_if *a, con
> return memcmp(&a->pxi_key, &b->pxi_key, sizeof(a->pxi_key));
> }
>
> -struct rwlock pppx_ifs_lk =
> RWLOCK_INITIALIZER("pppxifs");
> RBT_HEAD(pppx_ifs, pppx_if) pppx_ifs = RBT_INITIALIZER(&pppx_ifs);
> RBT_PROTOTYPE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp);
>
> @@ -620,8 +619,6 @@ pppx_if_next_unit(void)
> struct pppx_if *pxi;
> int unit = 0;
>
> - rw_assert_wrlock(&pppx_ifs_lk);
> -
> /* this is safe without splnet since we're not modifying it */
> do {
> int found = 0;
> @@ -650,11 +647,9 @@ pppx_if_find(struct pppx_dev *pxd, int s
> key.pxik_session_id = session_id;
> key.pxik_protocol = protocol;
>
> - rw_enter_read(&pppx_ifs_lk);
> pxi = RBT_FIND(pppx_ifs, &pppx_ifs, (struct pppx_if *)&key);
> if (pxi && pxi->pxi_ready == 0)
> pxi = NULL;
> - rw_exit_read(&pppx_ifs_lk);
>
> return pxi;
> }
> @@ -828,12 +823,10 @@ pppx_add_session(struct pppx_dev *pxd, s
> #endif
>
> /* try to set the interface up */
> - rw_enter_write(&pppx_ifs_lk);
> unit = pppx_if_next_unit();
> if (unit < 0) {
> pool_put(pppx_if_pl, pxi);
> error = ENOMEM;
> - rw_exit_write(&pppx_ifs_lk);
> goto out;
> }
>
> @@ -846,14 +839,12 @@ pppx_add_session(struct pppx_dev *pxd, s
> if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) {
> pool_put(pppx_if_pl, pxi);
> error = EADDRINUSE;
> - rw_exit_write(&pppx_ifs_lk);
> goto out;
> }
>
> if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL)
> panic("%s: pppx_ifs modified while lock was held", __func__);
> LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list);
> - rw_exit_write(&pppx_ifs_lk);
>
> snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
> ifp->if_mtu = req->pr_peer_mru; /* XXX */
> @@ -935,9 +926,8 @@ pppx_add_session(struct pppx_dev *pxd, s
> } else {
> if_addrhooks_run(ifp);
> }
> - rw_enter_write(&pppx_ifs_lk);
> +
> pxi->pxi_ready = 1;
> - rw_exit_write(&pppx_ifs_lk);
>
> out:
> return (error);
> @@ -1038,11 +1028,9 @@ pppx_if_destroy(struct pppx_dev *pxd, st
> if_detach(ifp);
> NET_LOCK();
>
> - rw_enter_write(&pppx_ifs_lk);
> if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
> panic("%s: pppx_ifs modified while lock was held", __func__);
> LIST_REMOVE(pxi, pxi_list);
> - rw_exit_write(&pppx_ifs_lk);
>
> pool_put(pppx_if_pl, pxi);
> }
