Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Use sorflush() instead of direct unp_scan() to discard dead unix(4) sockets
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Fri, 22 Mar 2024 20:10:38 +0300

Download raw body.

Thread
On Fri, Mar 22, 2024 at 01:51:44PM +0100, Alexander Bluhm wrote:
> On Tue, Mar 19, 2024 at 05:51:48PM +0300, Vitaliy Makkoveev wrote:
> > This is the part of making `so_rcv' sockbuf of unix(4) socket protected
> > by `sb_mtx' mutex(9) only. This work will remove relocking from the most
> > of send/receive path and make possible simultaneous send/receive on one
> > socket in the most cases.
> > 
> > I need this because the `so_rcv' sockbuf will be protected by `sb_mtx'
> > mutex(9), but unp_scan() does M_WAITOK memory allocation.
> 
> Both unp_scan() and sorflush() may sleep, so I don't see the
> difference.  Maybe it becomes clear with your next diff.
> 
> Is your goal is to put so_rcv.sb_mtx around this?
>         m = sb->sb_mb;
>         memset(&sb->sb_startzero, 0,
>              (caddr_t)&sb->sb_endzero - (caddr_t)&sb->sb_startzero);
> 

Sure. My goal is to remove sockets re-locking from the most of
uipc_*send() path. Since `sb_mtx' mutex(9) becomes the only `so_rcv'
protection, is should be used for all access.

The difference is direct unp_scan() and sorflush() is the mbuf(9) chain.
For the first case is is still linked to the `so_rcv', for the second is
is not. So you don't need to protect it.

I like this diff standalone because the unlinked chain it is much safer. 

> > Actually, this is the same unp_scan(..., unp_discard) call, but with
> > the significant difference. The mbuf(9) chain is unlinked from the
> > receive socket. It is much safer because currently the buffers of
> > discarded sockets still contains addresses of file descriptors.
> 
> OK bluhm@
> 
> > Index: sys/kern/uipc_socket.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> > retrieving revision 1.320
> > diff -u -p -r1.320 uipc_socket.c
> > --- sys/kern/uipc_socket.c	12 Feb 2024 22:48:27 -0000	1.320
> > +++ sys/kern/uipc_socket.c	19 Mar 2024 14:33:25 -0000
> > @@ -65,7 +65,6 @@ void	sotask(void *);
> >  void	soreaper(void *);
> >  void	soput(void *);
> >  int	somove(struct socket *, int);
> > -void	sorflush(struct socket *);
> >  
> >  void	filt_sordetach(struct knote *kn);
> >  int	filt_soread(struct knote *kn, long hint);
> > Index: sys/kern/uipc_usrreq.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> > retrieving revision 1.201
> > diff -u -p -r1.201 uipc_usrreq.c
> > --- sys/kern/uipc_usrreq.c	17 Mar 2024 19:47:08 -0000	1.201
> > +++ sys/kern/uipc_usrreq.c	19 Mar 2024 14:33:26 -0000
> > @@ -1438,7 +1438,7 @@ unp_gc(void *arg __unused)
> >  				 */
> >  				so = unp->unp_socket;
> >  				solock(so);
> > -				unp_scan(so->so_rcv.sb_mb, unp_discard);
> > +				sorflush(so);
> >  				sounlock(so);
> >  			}
> >  		}
> > Index: sys/sys/socketvar.h
> > ===================================================================
> > RCS file: /cvs/src/sys/sys/socketvar.h,v
> > retrieving revision 1.124
> > diff -u -p -r1.124 socketvar.h
> > --- sys/sys/socketvar.h	12 Feb 2024 22:48:27 -0000	1.124
> > +++ sys/sys/socketvar.h	19 Mar 2024 14:33:26 -0000
> > @@ -394,6 +394,7 @@ int	sosend(struct socket *, struct mbuf 
> >  	    struct mbuf *, struct mbuf *, int);
> >  int	sosetopt(struct socket *, int, int, struct mbuf *);
> >  int	soshutdown(struct socket *, int);
> > +void	sorflush(struct socket *);
> >  void	sowakeup(struct socket *, struct sockbuf *);
> >  void	sorwakeup(struct socket *);
> >  void	sowwakeup(struct socket *);
>