Re: Blocking memory allocation & solock()

[Mark Kettenis]

> Date: Mon, 21 Aug 2017 15:35:30 +0200
> From: Martin Pieuchot 
> 
> On 21/08/17(Mon) 15:33, Martin Pieuchot wrote:
> > I'd like to reduce the number of blocking memory allocations holding
> > the NET_LOCK().
> > 
> > Diff below moves m_get(M_WAIT) before grabbing the socket lock for
> > sogetopt().
> 
> Diff below includes the prototype change.
> 
> ok?

Totally makes sense; ok kettenis@

> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.201
> diff -u -p -r1.201 uipc_socket.c
> --- kern/uipc_socket.c10 Aug 2017 19:20:43 -  1.201
> +++ kern/uipc_socket.c21 Aug 2017 13:22:27 -
> @@ -1755,30 +1755,24 @@ bad:
>  }
>  
>  int
> -sogetopt(struct socket *so, int level, int optname, struct mbuf **mp)
> +sogetopt(struct socket *so, int level, int optname, struct mbuf *m)
>  {
>   int error = 0;
> - struct mbuf *m;
>  
>   soassertlocked(so);
>  
>   if (level != SOL_SOCKET) {
>   if (so->so_proto->pr_ctloutput) {
> - m = m_get(M_WAIT, MT_SOOPTS);
>   m->m_len = 0;
>  
>   error = (*so->so_proto->pr_ctloutput)(PRCO_GETOPT, so,
>   level, optname, m);
> - if (error) {
> - m_free(m);
> + if (error)
>   return (error);
> - }
> - *mp = m;
>   return (0);
>   } else
>   return (ENOPROTOOPT);
>   } else {
> - m = m_get(M_WAIT, MT_SOOPTS);
>   m->m_len = sizeof (int);
>  
>   switch (optname) {
> @@ -1856,13 +1850,10 @@ sogetopt(struct socket *so, int level, i
>   level = dom->dom_protosw->pr_protocol;
>   error = (*so->so_proto->pr_ctloutput)
>   (PRCO_GETOPT, so, level, optname, m);
> - if (error) {
> - (void)m_free(m);
> + if (error)
>   return (error);
> - }
>   break;
>   }
> - (void)m_free(m);
>   return (ENOPROTOOPT);
>  
>  #ifdef SOCKET_SPLICE
> @@ -1887,17 +1878,13 @@ sogetopt(struct socket *so, int level, i
>   &(unp->unp_connid), m->m_len);
>   break;
>   }
> - (void)m_free(m);
>   return (ENOTCONN);
>   }
> - (void)m_free(m);
>   return (EOPNOTSUPP);
>  
>   default:
> - (void)m_free(m);
>   return (ENOPROTOOPT);
>   }
> - *mp = m;
>   return (0);
>   }
>  }
> Index: kern/uipc_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> retrieving revision 1.158
> diff -u -p -r1.158 uipc_syscalls.c
> --- kern/uipc_syscalls.c  10 Aug 2017 19:20:43 -  1.158
> +++ kern/uipc_syscalls.c  21 Aug 2017 13:17:17 -
> @@ -1014,9 +1014,10 @@ sys_getsockopt(struct proc *p, void *v, 
>   goto out;
>   } else
>   valsize = 0;
> + m = m_get(M_WAIT, MT_SOOPTS);
>   so = fp->f_data;
>   s = solock(so);
> - error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), );
> + error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), m);
>   sounlock(s);
>   if (error == 0 && SCARG(uap, val) && valsize && m != NULL) {
>   if (valsize > m->m_len)
> @@ -1026,9 +1027,9 @@ sys_getsockopt(struct proc *p, void *v, 
>   error = copyout(,
>   SCARG(uap, avalsize), sizeof (valsize));
>   }
> + m_free(m);
>  out:
>   FRELE(fp, p);
> - m_free(m);
>   return (error);
>  }
>  
> Index: sys/socketvar.h
> ===
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.74
> diff -u -p -r1.74 socketvar.h
> --- sys/socketvar.h   12 Jul 2017 10:56:47 -  1.74
> +++ sys/socketvar.h   21 Aug 2017 13:15:54 -
> @@ -312,8 +312,7 @@ int   soconnect2(struct socket *so1, struc
>  int  socreate(int dom, struct socket **aso, int type, int proto);
>  int  sodisconnect(struct socket *so);
>  void sofree(struct socket *so);
> -int  sogetopt(struct socket *so, int level, int optname,
> - struct mbuf **mp);
> +int  sogetopt(struct socket *so, int level, int optname, struct mbuf *m);
>  void sohasoutofband(struct socket 

Re: Blocking memory allocation & solock()

[Martin Pieuchot]

On 21/08/17(Mon) 15:33, Martin Pieuchot wrote:
> I'd like to reduce the number of blocking memory allocations holding
> the NET_LOCK().
> 
> Diff below moves m_get(M_WAIT) before grabbing the socket lock for
> sogetopt().

Diff below includes the prototype change.

ok?

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.201
diff -u -p -r1.201 uipc_socket.c
--- kern/uipc_socket.c  10 Aug 2017 19:20:43 -  1.201
+++ kern/uipc_socket.c  21 Aug 2017 13:22:27 -
@@ -1755,30 +1755,24 @@ bad:
 }
 
 int
-sogetopt(struct socket *so, int level, int optname, struct mbuf **mp)
+sogetopt(struct socket *so, int level, int optname, struct mbuf *m)
 {
int error = 0;
-   struct mbuf *m;
 
soassertlocked(so);
 
if (level != SOL_SOCKET) {
if (so->so_proto->pr_ctloutput) {
-   m = m_get(M_WAIT, MT_SOOPTS);
m->m_len = 0;
 
error = (*so->so_proto->pr_ctloutput)(PRCO_GETOPT, so,
level, optname, m);
-   if (error) {
-   m_free(m);
+   if (error)
return (error);
-   }
-   *mp = m;
return (0);
} else
return (ENOPROTOOPT);
} else {
-   m = m_get(M_WAIT, MT_SOOPTS);
m->m_len = sizeof (int);
 
switch (optname) {
@@ -1856,13 +1850,10 @@ sogetopt(struct socket *so, int level, i
level = dom->dom_protosw->pr_protocol;
error = (*so->so_proto->pr_ctloutput)
(PRCO_GETOPT, so, level, optname, m);
-   if (error) {
-   (void)m_free(m);
+   if (error)
return (error);
-   }
break;
}
-   (void)m_free(m);
return (ENOPROTOOPT);
 
 #ifdef SOCKET_SPLICE
@@ -1887,17 +1878,13 @@ sogetopt(struct socket *so, int level, i
&(unp->unp_connid), m->m_len);
break;
}
-   (void)m_free(m);
return (ENOTCONN);
}
-   (void)m_free(m);
return (EOPNOTSUPP);
 
default:
-   (void)m_free(m);
return (ENOPROTOOPT);
}
-   *mp = m;
return (0);
}
 }
Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.158
diff -u -p -r1.158 uipc_syscalls.c
--- kern/uipc_syscalls.c10 Aug 2017 19:20:43 -  1.158
+++ kern/uipc_syscalls.c21 Aug 2017 13:17:17 -
@@ -1014,9 +1014,10 @@ sys_getsockopt(struct proc *p, void *v, 
goto out;
} else
valsize = 0;
+   m = m_get(M_WAIT, MT_SOOPTS);
so = fp->f_data;
s = solock(so);
-   error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), );
+   error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), m);
sounlock(s);
if (error == 0 && SCARG(uap, val) && valsize && m != NULL) {
if (valsize > m->m_len)
@@ -1026,9 +1027,9 @@ sys_getsockopt(struct proc *p, void *v, 
error = copyout(,
SCARG(uap, avalsize), sizeof (valsize));
}
+   m_free(m);
 out:
FRELE(fp, p);
-   m_free(m);
return (error);
 }
 
Index: sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.74
diff -u -p -r1.74 socketvar.h
--- sys/socketvar.h 12 Jul 2017 10:56:47 -  1.74
+++ sys/socketvar.h 21 Aug 2017 13:15:54 -
@@ -312,8 +312,7 @@ int soconnect2(struct socket *so1, struc
 intsocreate(int dom, struct socket **aso, int type, int proto);
 intsodisconnect(struct socket *so);
 void   sofree(struct socket *so);
-intsogetopt(struct socket *so, int level, int optname,
-   struct mbuf **mp);
+intsogetopt(struct socket *so, int level, int optname, struct mbuf *m);
 void   sohasoutofband(struct socket *so);
 void   soisconnected(struct socket *so);
 void   soisconnecting(struct socket *so);

Blocking memory allocation & solock()

[Martin Pieuchot]

I'd like to reduce the number of blocking memory allocations holding
the NET_LOCK().

Diff below moves m_get(M_WAIT) before grabbing the socket lock for
sogetopt().

ok?

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.201
diff -u -p -r1.201 uipc_socket.c
--- kern/uipc_socket.c  10 Aug 2017 19:20:43 -  1.201
+++ kern/uipc_socket.c  21 Aug 2017 13:22:27 -
@@ -1755,30 +1755,24 @@ bad:
 }
 
 int
-sogetopt(struct socket *so, int level, int optname, struct mbuf **mp)
+sogetopt(struct socket *so, int level, int optname, struct mbuf *m)
 {
int error = 0;
-   struct mbuf *m;
 
soassertlocked(so);
 
if (level != SOL_SOCKET) {
if (so->so_proto->pr_ctloutput) {
-   m = m_get(M_WAIT, MT_SOOPTS);
m->m_len = 0;
 
error = (*so->so_proto->pr_ctloutput)(PRCO_GETOPT, so,
level, optname, m);
-   if (error) {
-   m_free(m);
+   if (error)
return (error);
-   }
-   *mp = m;
return (0);
} else
return (ENOPROTOOPT);
} else {
-   m = m_get(M_WAIT, MT_SOOPTS);
m->m_len = sizeof (int);
 
switch (optname) {
@@ -1856,13 +1850,10 @@ sogetopt(struct socket *so, int level, i
level = dom->dom_protosw->pr_protocol;
error = (*so->so_proto->pr_ctloutput)
(PRCO_GETOPT, so, level, optname, m);
-   if (error) {
-   (void)m_free(m);
+   if (error)
return (error);
-   }
break;
}
-   (void)m_free(m);
return (ENOPROTOOPT);
 
 #ifdef SOCKET_SPLICE
@@ -1887,17 +1878,13 @@ sogetopt(struct socket *so, int level, i
&(unp->unp_connid), m->m_len);
break;
}
-   (void)m_free(m);
return (ENOTCONN);
}
-   (void)m_free(m);
return (EOPNOTSUPP);
 
default:
-   (void)m_free(m);
return (ENOPROTOOPT);
}
-   *mp = m;
return (0);
}
 }
Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.158
diff -u -p -r1.158 uipc_syscalls.c
--- kern/uipc_syscalls.c10 Aug 2017 19:20:43 -  1.158
+++ kern/uipc_syscalls.c21 Aug 2017 13:17:17 -
@@ -1014,9 +1014,10 @@ sys_getsockopt(struct proc *p, void *v, 
goto out;
} else
valsize = 0;
+   m = m_get(M_WAIT, MT_SOOPTS);
so = fp->f_data;
s = solock(so);
-   error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), );
+   error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), m);
sounlock(s);
if (error == 0 && SCARG(uap, val) && valsize && m != NULL) {
if (valsize > m->m_len)
@@ -1026,9 +1027,9 @@ sys_getsockopt(struct proc *p, void *v, 
error = copyout(,
SCARG(uap, avalsize), sizeof (valsize));
}
+   m_free(m);
 out:
FRELE(fp, p);
-   m_free(m);
return (error);
 }