TCP/UDP/etc input path w/o KERNEL_LOCK()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

TCP/UDP/etc input path w/o KERNEL_LOCK()

Martin Pieuchot
New year, new diff.  Assuming we can live with the kqueue(2) races,
here's a diff to remove the KERNEL_LOCK() from all pr_input() routines.

I'd appreciate tests.

Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.533
diff -u -p -r1.533 if.c
--- net/if.c 2 Jan 2018 12:52:17 -0000 1.533
+++ net/if.c 2 Jan 2018 14:32:05 -0000
@@ -926,7 +926,6 @@ if_netisr(void *unused)
 {
  int n, t = 0;
 
- KERNEL_LOCK();
  NET_LOCK();
 
  while ((n = netisr) != 0) {
@@ -940,8 +939,11 @@ if_netisr(void *unused)
  atomic_clearbits_int(&netisr, n);
 
 #if NETHER > 0
- if (n & (1 << NETISR_ARP))
+ if (n & (1 << NETISR_ARP)) {
+ KERNEL_LOCK();
  arpintr();
+ KERNEL_UNLOCK();
+ }
 #endif
  if (n & (1 << NETISR_IP))
  ipintr();
@@ -950,35 +952,52 @@ if_netisr(void *unused)
  ip6intr();
 #endif
 #if NPPP > 0
- if (n & (1 << NETISR_PPP))
+ if (n & (1 << NETISR_PPP)) {
+ KERNEL_LOCK();
  pppintr();
+ KERNEL_UNLOCK();
+ }
 #endif
 #if NBRIDGE > 0
- if (n & (1 << NETISR_BRIDGE))
+ if (n & (1 << NETISR_BRIDGE)) {
+ KERNEL_LOCK();
  bridgeintr();
+ KERNEL_UNLOCK();
+ }
 #endif
 #if NSWITCH > 0
- if (n & (1 << NETISR_SWITCH))
+ if (n & (1 << NETISR_SWITCH)) {
+ KERNEL_LOCK();
  switchintr();
+ KERNEL_UNLOCK();
+ }
 #endif
 #if NPPPOE > 0
- if (n & (1 << NETISR_PPPOE))
+ if (n & (1 << NETISR_PPPOE)) {
+ KERNEL_LOCK();
  pppoeintr();
+ KERNEL_UNLOCK();
+ }
 #endif
 #ifdef PIPEX
- if (n & (1 << NETISR_PIPEX))
+ if (n & (1 << NETISR_PIPEX)) {
+ KERNEL_LOCK();
  pipexintr();
+ KERNEL_UNLOCK();
+ }
 #endif
  t |= n;
  }
 
 #if NPFSYNC > 0
- if (t & (1 << NETISR_PFSYNC))
+ if (t & (1 << NETISR_PFSYNC)) {
+ KERNEL_LOCK();
  pfsyncintr();
+ KERNEL_UNLOCK();
+ }
 #endif
 
  NET_UNLOCK();
- KERNEL_UNLOCK();
 }
 
 void

Reply | Threaded
Open this post in threaded view
|

Re: TCP/UDP/etc input path w/o KERNEL_LOCK()

Martin Pieuchot
On 02/01/18(Tue) 16:04, Martin Pieuchot wrote:
> New year, new diff.  Assuming we can live with the kqueue(2) races,
> here's a diff to remove the KERNEL_LOCK() from all pr_input() routines.
>
> I'd appreciate tests.

I got multiple positive reports, I'd like to commit this to see if
anything breaks next.  Any ok?

> Index: net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.533
> diff -u -p -r1.533 if.c
> --- net/if.c 2 Jan 2018 12:52:17 -0000 1.533
> +++ net/if.c 2 Jan 2018 14:32:05 -0000
> @@ -926,7 +926,6 @@ if_netisr(void *unused)
>  {
>   int n, t = 0;
>  
> - KERNEL_LOCK();
>   NET_LOCK();
>  
>   while ((n = netisr) != 0) {
> @@ -940,8 +939,11 @@ if_netisr(void *unused)
>   atomic_clearbits_int(&netisr, n);
>  
>  #if NETHER > 0
> - if (n & (1 << NETISR_ARP))
> + if (n & (1 << NETISR_ARP)) {
> + KERNEL_LOCK();
>   arpintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>   if (n & (1 << NETISR_IP))
>   ipintr();
> @@ -950,35 +952,52 @@ if_netisr(void *unused)
>   ip6intr();
>  #endif
>  #if NPPP > 0
> - if (n & (1 << NETISR_PPP))
> + if (n & (1 << NETISR_PPP)) {
> + KERNEL_LOCK();
>   pppintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>  #if NBRIDGE > 0
> - if (n & (1 << NETISR_BRIDGE))
> + if (n & (1 << NETISR_BRIDGE)) {
> + KERNEL_LOCK();
>   bridgeintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>  #if NSWITCH > 0
> - if (n & (1 << NETISR_SWITCH))
> + if (n & (1 << NETISR_SWITCH)) {
> + KERNEL_LOCK();
>   switchintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>  #if NPPPOE > 0
> - if (n & (1 << NETISR_PPPOE))
> + if (n & (1 << NETISR_PPPOE)) {
> + KERNEL_LOCK();
>   pppoeintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>  #ifdef PIPEX
> - if (n & (1 << NETISR_PIPEX))
> + if (n & (1 << NETISR_PIPEX)) {
> + KERNEL_LOCK();
>   pipexintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>   t |= n;
>   }
>  
>  #if NPFSYNC > 0
> - if (t & (1 << NETISR_PFSYNC))
> + if (t & (1 << NETISR_PFSYNC)) {
> + KERNEL_LOCK();
>   pfsyncintr();
> + KERNEL_UNLOCK();
> + }
>  #endif
>  
>   NET_UNLOCK();
> - KERNEL_UNLOCK();
>  }
>  
>  void
>

Reply | Threaded
Open this post in threaded view
|

Re: TCP/UDP/etc input path w/o KERNEL_LOCK()

Alexander Bluhm
On Tue, Jan 09, 2018 at 11:24:14AM +0100, Martin Pieuchot wrote:
> On 02/01/18(Tue) 16:04, Martin Pieuchot wrote:
> > New year, new diff.  Assuming we can live with the kqueue(2) races,
> > here's a diff to remove the KERNEL_LOCK() from all pr_input() routines.
> >
> > I'd appreciate tests.
>
> I got multiple positive reports, I'd like to commit this to see if
> anything breaks next.  Any ok?

Let's try it.  OK bluhm@

> > Index: net/if.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.533
> > diff -u -p -r1.533 if.c
> > --- net/if.c 2 Jan 2018 12:52:17 -0000 1.533
> > +++ net/if.c 2 Jan 2018 14:32:05 -0000
> > @@ -926,7 +926,6 @@ if_netisr(void *unused)
> >  {
> >   int n, t = 0;
> >  
> > - KERNEL_LOCK();
> >   NET_LOCK();
> >  
> >   while ((n = netisr) != 0) {
> > @@ -940,8 +939,11 @@ if_netisr(void *unused)
> >   atomic_clearbits_int(&netisr, n);
> >  
> >  #if NETHER > 0
> > - if (n & (1 << NETISR_ARP))
> > + if (n & (1 << NETISR_ARP)) {
> > + KERNEL_LOCK();
> >   arpintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >   if (n & (1 << NETISR_IP))
> >   ipintr();
> > @@ -950,35 +952,52 @@ if_netisr(void *unused)
> >   ip6intr();
> >  #endif
> >  #if NPPP > 0
> > - if (n & (1 << NETISR_PPP))
> > + if (n & (1 << NETISR_PPP)) {
> > + KERNEL_LOCK();
> >   pppintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >  #if NBRIDGE > 0
> > - if (n & (1 << NETISR_BRIDGE))
> > + if (n & (1 << NETISR_BRIDGE)) {
> > + KERNEL_LOCK();
> >   bridgeintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >  #if NSWITCH > 0
> > - if (n & (1 << NETISR_SWITCH))
> > + if (n & (1 << NETISR_SWITCH)) {
> > + KERNEL_LOCK();
> >   switchintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >  #if NPPPOE > 0
> > - if (n & (1 << NETISR_PPPOE))
> > + if (n & (1 << NETISR_PPPOE)) {
> > + KERNEL_LOCK();
> >   pppoeintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >  #ifdef PIPEX
> > - if (n & (1 << NETISR_PIPEX))
> > + if (n & (1 << NETISR_PIPEX)) {
> > + KERNEL_LOCK();
> >   pipexintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >   t |= n;
> >   }
> >  
> >  #if NPFSYNC > 0
> > - if (t & (1 << NETISR_PFSYNC))
> > + if (t & (1 << NETISR_PFSYNC)) {
> > + KERNEL_LOCK();
> >   pfsyncintr();
> > + KERNEL_UNLOCK();
> > + }
> >  #endif
> >  
> >   NET_UNLOCK();
> > - KERNEL_UNLOCK();
> >  }
> >  
> >  void
> >