From: Alexander Bluhm Subject: Re: fstat socket buffer mutex To: Vitaliy Makkoveev Cc: tech@openbsd.org Date: Sat, 9 Nov 2024 16:06:40 +0100 On Sat, Nov 09, 2024 at 05:54:06PM +0300, Vitaliy Makkoveev wrote: > > On 9 Nov 2024, at 17:50, Vitaliy Makkoveev wrote: > > > >> On 9 Nov 2024, at 17:38, Alexander Bluhm wrote: > >> > >> Hi, > >> > >> I wonder if we should also lock so_snd in soo_stat(). Take so_rcv > >> and so_snd mutex together to provide consistent view to userland. > >> > >> ok? > >> > > > > We set SS_CANTRCVMORE and SS_CANTSENDMORE independently from each > > other, no consistency between them. > > > > I locked so_rcv to keep consistency between sb_cc and sb_state, > > but no locks required for so_snd because we only check sb_state. I am not a fan of unlocked read access. As this is not performance critical, can we just put a mutex here? This is easier than adding READ_ONLY() or something like that. And it is consistent to so_rcv. ok? > solock_shared() was kept because tcp(4) sockets rely on it and I > see no reason to introduce if-tcp-solock branching here. In the > future I plan to push solock() down to pru_sence(). Currently I am going through TCP output path and check where solock_shared() is sufficient. Then we can set PR_MPSOCKET for TCP. bluhm Index: kern/sys_socket.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/kern/sys_socket.c,v diff -u -p -r1.65 sys_socket.c --- kern/sys_socket.c 30 Apr 2024 17:59:15 -0000 1.65 +++ kern/sys_socket.c 9 Nov 2024 15:00:38 -0000 @@ -153,8 +153,10 @@ soo_stat(struct file *fp, struct stat *u so->so_rcv.sb_cc != 0) ub->st_mode |= S_IRUSR | S_IRGRP | S_IROTH; mtx_leave(&so->so_rcv.sb_mtx); + mtx_enter(&so->so_snd.sb_mtx); if ((so->so_snd.sb_state & SS_CANTSENDMORE) == 0) ub->st_mode |= S_IWUSR | S_IWGRP | S_IWOTH; + mtx_leave(&so->so_snd.sb_mtx); ub->st_uid = so->so_euid; ub->st_gid = so->so_egid; (void)pru_sense(so, ub);