Re: splsoftnet() in SO_SPLICE

2017-05-15 Thread Alexander Bluhm
On Mon, May 15, 2017 at 02:32:07PM +0200, Martin Pieuchot wrote:
> On 15/05/17(Mon) 13:35, Alexander Bluhm wrote:
> > On Mon, May 15, 2017 at 01:20:02PM +0200, Martin Pieuchot wrote:
> > > Here's the last of the splsoftnet(), is it ok if I replace it with a
> > > KERNEL_LOCK() to make it clear this needs protection?
> > 
> > I think it needs the socket lock.  somove() is modifying the
> > so_splicelen value.  somove() calls soassertlocked().
> 
> Fine, updated diff below.

OK bluhm@

> 
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.183
> diff -u -p -r1.183 uipc_socket.c
> --- kern/uipc_socket.c15 May 2017 12:26:00 -  1.183
> +++ kern/uipc_socket.c15 May 2017 12:31:39 -
> @@ -1862,12 +1862,13 @@ sogetopt(struct socket *so, int level, i
>   case SO_SPLICE:
>   {
>   off_t len;
> - int s = splsoftnet();
> + int s;
>  
> + s = solock(so);
>   m->m_len = sizeof(off_t);
>   len = so->so_sp ? so->so_sp->ssp_len : 0;
>   memcpy(mtod(m, off_t *), &len, sizeof(off_t));
> - splx(s);
> + sounlock(s);
>   break;
>   }
>  #endif /* SOCKET_SPLICE */



Re: splsoftnet() in SO_SPLICE

2017-05-15 Thread Martin Pieuchot
On 15/05/17(Mon) 13:35, Alexander Bluhm wrote:
> On Mon, May 15, 2017 at 01:20:02PM +0200, Martin Pieuchot wrote:
> > Here's the last of the splsoftnet(), is it ok if I replace it with a
> > KERNEL_LOCK() to make it clear this needs protection?
> 
> I think it needs the socket lock.  somove() is modifying the
> so_splicelen value.  somove() calls soassertlocked().

Fine, updated diff below.

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.183
diff -u -p -r1.183 uipc_socket.c
--- kern/uipc_socket.c  15 May 2017 12:26:00 -  1.183
+++ kern/uipc_socket.c  15 May 2017 12:31:39 -
@@ -1862,12 +1862,13 @@ sogetopt(struct socket *so, int level, i
case SO_SPLICE:
{
off_t len;
-   int s = splsoftnet();
+   int s;
 
+   s = solock(so);
m->m_len = sizeof(off_t);
len = so->so_sp ? so->so_sp->ssp_len : 0;
memcpy(mtod(m, off_t *), &len, sizeof(off_t));
-   splx(s);
+   sounlock(s);
break;
}
 #endif /* SOCKET_SPLICE */



Re: splsoftnet() in SO_SPLICE

2017-05-15 Thread Alexander Bluhm
On Mon, May 15, 2017 at 01:20:02PM +0200, Martin Pieuchot wrote:
> Here's the last of the splsoftnet(), is it ok if I replace it with a
> KERNEL_LOCK() to make it clear this needs protection?

I think it needs the socket lock.  somove() is modifying the
so_splicelen value.  somove() calls soassertlocked().

bluhm

> 
> Index: kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.182
> diff -u -p -r1.182 uipc_socket.c
> --- kern/uipc_socket.c2 Apr 2017 23:40:08 -   1.182
> +++ kern/uipc_socket.c15 May 2017 11:18:21 -
> @@ -1860,12 +1862,12 @@ sogetopt(struct socket *so, int level, i
>   case SO_SPLICE:
>   {
>   off_t len;
> - int s = splsoftnet();
>  
> + KERNEL_LOCK();
>   m->m_len = sizeof(off_t);
>   len = so->so_sp ? so->so_sp->ssp_len : 0;
>   memcpy(mtod(m, off_t *), &len, sizeof(off_t));
> - splx(s);
> + KERNEL_UNLOCK();
>   break;
>   }
>  #endif /* SOCKET_SPLICE */



splsoftnet() in SO_SPLICE

2017-05-15 Thread Martin Pieuchot
Here's the last of the splsoftnet(), is it ok if I replace it with a
KERNEL_LOCK() to make it clear this needs protection?

Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.182
diff -u -p -r1.182 uipc_socket.c
--- kern/uipc_socket.c  2 Apr 2017 23:40:08 -   1.182
+++ kern/uipc_socket.c  15 May 2017 11:18:21 -
@@ -1860,12 +1862,12 @@ sogetopt(struct socket *so, int level, i
case SO_SPLICE:
{
off_t len;
-   int s = splsoftnet();
 
+   KERNEL_LOCK();
m->m_len = sizeof(off_t);
len = so->so_sp ? so->so_sp->ssp_len : 0;
memcpy(mtod(m, off_t *), &len, sizeof(off_t));
-   splx(s);
+   KERNEL_UNLOCK();
break;
}
 #endif /* SOCKET_SPLICE */