From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Please test: shared solock for all intet sockets within knote(9) routines
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: tech@openbsd.org
Date: Mon, 29 Jan 2024 11:30:17 +0300

Forgot to say, I'm stable with both diffs.

On Sun, Jan 28, 2024 at 10:25:14PM +0300, Vitaliy Makkoveev wrote:
> On Sun, Jan 28, 2024 at 12:54:24PM +0100, Alexander Bluhm wrote:
> > On Fri, Jan 26, 2024 at 07:59:02PM +0300, Vitaliy Makkoveev wrote:
> > > Not for commit yet, but could be interesting. The idea is the same as
> > > for PCB layer: take shared netlock and exclusive `so_lock' to serialize
> > > access to the socket. The tcp(4) sockets permit this because knote(9)
> > > routines do read-only access to sockets.
> > 
> > Passes regress with witness.
> > 
> > OK bluhm@
> > 
> > > Should provide some additional performance.
> > 
> > My setup is currently busy with some other test run.
> > Can do performance comparison in a few days.
> 
> Thanks for attention. Unfortunately current soassertlocked() is very
> dumb for inet case:
> 
> soassertlocked(struct socket *so)
> {       
>         switch (so->so_proto->pr_domain->dom_family) {
>         case PF_INET:
>         case PF_INET6:
>                 NET_ASSERT_LOCKED();
>                 break;
>         default:
>                 rw_assert_wrlock(&so->so_lock);
>                 break;
>         }
> }
> 
> Could we update it first and check `so_lock' state for shared netlock
> case? Since we already use solock_shared(), I want to raise assertion
> instead of panic.
> 
> Index: sys/kern/uipc_socket2.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.140
> diff -u -p -r1.140 uipc_socket2.c
> --- sys/kern/uipc_socket2.c	11 Jan 2024 14:15:11 -0000	1.140
> +++ sys/kern/uipc_socket2.c	28 Jan 2024 19:18:48 -0000
> @@ -444,7 +444,14 @@ soassertlocked(struct socket *so)
>  	switch (so->so_proto->pr_domain->dom_family) {
>  	case PF_INET:
>  	case PF_INET6:
> -		NET_ASSERT_LOCKED();
> +		if (rw_status(&netlock) == RW_READ) {
> +			NET_ASSERT_LOCKED();
> +
> +			if (splassert_ctl > 0 &&
> +			    rw_status(&so->so_lock) != RW_WRITE)
> +				splassert_fail(0, RW_WRITE, __func__);
> +		} else
> +			NET_ASSERT_LOCKED_EXCLUSIVE();
>  		break;
>  	default:
>  		rw_assert_wrlock(&so->so_lock);
>