On Thu, Jun 23, 2016 at 12:44 -0400, Ted Unangst wrote:
> Instead of using the old flags and tsleep style lock, switch to rwlock in
> sblock. That's what it's for. More legible, and as a bonus, MP safer.
>
RW_NOSLEEP returns EBUSY if it has to wait, however
old sblock macro would return EWOULDBLOCK (EAGAIN)
instead. This error code is returned all the way
to write system call so you don't want to change
that.
>
> Index: kern/uipc_socket2.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 uipc_socket2.c
> --- kern/uipc_socket2.c 6 Oct 2015 14:38:32 -0000 1.63
> +++ kern/uipc_socket2.c 23 Jun 2016 16:38:41 -0000
> @@ -185,6 +185,9 @@ sonewconn(struct socket *head, int conns
> so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
> so->so_rcv.sb_timeo = head->so_rcv.sb_timeo;
>
> + rw_init(&so->so_rcv.sb_rwl, "sbsndl");
> + rw_init(&so->so_snd.sb_rwl, "sbrcvl");
> +
> soqinsque(head, so, soqueue);
> if ((*so->so_proto->pr_usrreq)(so, PRU_ATTACH, NULL, NULL, NULL,
> curproc)) {
> @@ -286,22 +289,24 @@ sbwait(struct sockbuf *sb)
> * return any error returned from sleep (EINTR).
> */
> int
> -sb_lock(struct sockbuf *sb)
> +sblock(struct sockbuf *sb, int wf)
> {
> int error;
>
> - while (sb->sb_flags & SB_LOCK) {
> - sb->sb_flags |= SB_WANT;
> - error = tsleep(&sb->sb_flags,
> - (sb->sb_flags & SB_NOINTR) ?
> - PSOCK : PSOCK|PCATCH, "netlck", 0);
> - if (error)
> - return (error);
> - }
> - sb->sb_flags |= SB_LOCK;
> - return (0);
> + error = rw_enter(&sb->sb_rwl, RW_WRITE |
> + (sb->sb_flags & SB_NOINTR ? 0 : RW_INTR) |
> + (wf == M_WAITOK ? 0 : RW_NOSLEEP));
> +
> + return (error);
> }
>
> +void
> +sbunlock(struct sockbuf *sb)
> +{
> + rw_exit(&sb->sb_rwl);
> +}
> +
> +
> /*
> * Wakeup processes waiting on a socket buffer.
> * Do asynchronous notification via SIGIO
> @@ -827,7 +832,7 @@ void
> sbflush(struct sockbuf *sb)
> {
>
> - KASSERT((sb->sb_flags & SB_LOCK) == 0);
> + rw_assert_unlocked(&sb->sb_rwl);
>
> while (sb->sb_mbcnt)
> sbdrop(sb, (int)sb->sb_cc);
> Index: sys/socketvar.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.60
> diff -u -p -r1.60 socketvar.h
> --- sys/socketvar.h 25 Feb 2016 07:39:09 -0000 1.60
> +++ sys/socketvar.h 23 Jun 2016 16:40:56 -0000
> @@ -108,13 +108,12 @@ struct socket {
> struct mbuf *sb_lastrecord;/* first mbuf of last record in
> socket buffer */
> struct selinfo sb_sel; /* process selecting read/write */
> + struct rwlock sb_rwl; /* lock */
> int sb_flagsintr; /* flags, changed during interrupt */
> short sb_flags; /* flags, see below */
> u_short sb_timeo; /* timeout for read/write */
> } so_rcv, so_snd;
> #define SB_MAX (256*1024) /* default for max chars in
> sockbuf */
> -#define SB_LOCK 0x01 /* lock on data queue */
> -#define SB_WANT 0x02 /* someone is waiting to lock */
> #define SB_WAIT 0x04 /* someone is waiting for
> data/space */
> #define SB_SEL 0x08 /* someone is selecting */
> #define SB_ASYNC 0x10 /* ASYNC I/O, need signals */
> @@ -218,18 +217,10 @@ struct socket {
> * Unless SB_NOINTR is set on sockbuf, sleep is interruptible.
> * Returns error without lock if sleep is interrupted.
> */
> -#define sblock(sb, wf) ((sb)->sb_flags & SB_LOCK ? \
> - (((wf) == M_WAITOK) ? sb_lock(sb) : EWOULDBLOCK) : \
> - ((sb)->sb_flags |= SB_LOCK, 0))
> +int sblock(struct sockbuf *sb, int wf);
>
> /* release lock on sockbuf sb */
> -#define sbunlock(sb) do {
> \
> - (sb)->sb_flags &= ~SB_LOCK; \
> - if ((sb)->sb_flags & SB_WANT) { \
> - (sb)->sb_flags &= ~SB_WANT; \
> - wakeup((caddr_t)&(sb)->sb_flags); \
> - } \
> -} while (/* CONSTCOND */ 0)
> +void sbunlock(struct sockbuf *sb);
>
> #define SB_EMPTY_FIXUP(sb) do {
> \
> if ((sb)->sb_mb == NULL) { \
>
