All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rps: send IPIs ASAP
@ 2010-04-20  4:08 Changli Gao
  2010-04-20  5:15 ` Tom Herbert
  0 siblings, 1 reply; 5+ messages in thread
From: Changli Gao @ 2010-04-20  4:08 UTC (permalink / raw)
  To: David S. Miller; +Cc: Tom Herbert, Eric Dumazet, netdev, Changli Gao

rps: send IPIs ASAP

In order to reduce latency, we'd better send IPIs ASAP to schedule the
corresponding NAPIs.

For NAPI drivers, we send IPIs immediately, and for the others, we defer them
to NET_RX_SOFTIRQ. In this patch, we move net_rps_action() to the beginning of
net_rx_action() to emulate a softirq with the higher priority than
NET_RX_SOFTIRQ.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/core/dev.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 05a2b29..d8fca21 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2363,6 +2363,10 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
 {
 	struct softnet_data *queue;
 	unsigned long flags;
+#ifdef CONFIG_RPS
+	bool ni = !in_irq();
+	bool ipi = false;
+#endif
 
 	queue = &per_cpu(softnet_data, cpu);
 
@@ -2380,6 +2384,10 @@ enqueue:
 #endif
 			rps_unlock(queue);
 			local_irq_restore(flags);
+#ifdef CONFIG_RPS
+			if (ipi)
+				__smp_call_function_single(cpu, &queue->csd, 0);
+#endif
 			return NET_RX_SUCCESS;
 		}
 
@@ -2389,6 +2397,11 @@ enqueue:
 			if (cpu != smp_processor_id()) {
 				struct softnet_data *myqueue;
 
+				if (ni) {
+					ipi = true;
+					goto enqueue;
+				}
+
 				myqueue = &__get_cpu_var(softnet_data);
 				queue->rps_ipi_next = myqueue->rps_ipi_list;
 				myqueue->rps_ipi_list = queue;
@@ -3337,6 +3350,7 @@ static void net_rps_action(void)
 	struct softnet_data *locqueue = &__get_cpu_var(softnet_data);
 	struct softnet_data *remqueue = locqueue->rps_ipi_list;
 
+	local_irq_disable();
 	if (remqueue) {
 		locqueue->rps_ipi_list = NULL;
 
@@ -3350,9 +3364,10 @@ static void net_rps_action(void)
 							   &remqueue->csd, 0);
 			remqueue = next;
 		}
-	} else
-#endif
+	} else {
 		local_irq_enable();
+	}
+#endif
 }
 
 static void net_rx_action(struct softirq_action *h)
@@ -3362,6 +3377,8 @@ static void net_rx_action(struct softirq_action *h)
 	int budget = netdev_budget;
 	void *have;
 
+	net_rps_action();
+
 	local_irq_disable();
 
 	while (!list_empty(list)) {
@@ -3423,7 +3440,7 @@ static void net_rx_action(struct softirq_action *h)
 		netpoll_poll_unlock(have);
 	}
 out:
-	net_rps_action();
+	local_irq_enable();
 
 #ifdef CONFIG_NET_DMA
 	/*

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] rps: send IPIs ASAP
  2010-04-20  4:08 [PATCH] rps: send IPIs ASAP Changli Gao
@ 2010-04-20  5:15 ` Tom Herbert
  2010-04-20  5:39   ` David Miller
  2010-04-20  5:45   ` Eric Dumazet
  0 siblings, 2 replies; 5+ messages in thread
From: Tom Herbert @ 2010-04-20  5:15 UTC (permalink / raw)
  To: Changli Gao; +Cc: David S. Miller, Eric Dumazet, netdev

On Mon, Apr 19, 2010 at 9:08 PM, Changli Gao <xiaosuo@gmail.com> wrote:
> rps: send IPIs ASAP
>
> In order to reduce latency, we'd better send IPIs ASAP to schedule the
> corresponding NAPIs.
>
A design point of RPS is that we generate at most one IPI per CPU per
device interrupt, which at least offers some predictable coalescing.
With your changes, we would get at most one IPI per packet-- that
could represent a lot more of them.  Did you test this to see what the
impact is in this regard?


> For NAPI drivers, we send IPIs immediately, and for the others, we defer them
> to NET_RX_SOFTIRQ. In this patch, we move net_rps_action() to the beginning of
> net_rx_action() to emulate a softirq with the higher priority than
> NET_RX_SOFTIRQ.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
>  net/core/dev.c |   23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 05a2b29..d8fca21 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2363,6 +2363,10 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
>  {
>        struct softnet_data *queue;
>        unsigned long flags;
> +#ifdef CONFIG_RPS
> +       bool ni = !in_irq();
> +       bool ipi = false;
> +#endif
>
>        queue = &per_cpu(softnet_data, cpu);
>
> @@ -2380,6 +2384,10 @@ enqueue:
>  #endif
>                        rps_unlock(queue);
>                        local_irq_restore(flags);
> +#ifdef CONFIG_RPS
> +                       if (ipi)
> +                               __smp_call_function_single(cpu, &queue->csd, 0);
> +#endif
>                        return NET_RX_SUCCESS;
>                }
>
> @@ -2389,6 +2397,11 @@ enqueue:
>                        if (cpu != smp_processor_id()) {
>                                struct softnet_data *myqueue;
>
> +                               if (ni) {
> +                                       ipi = true;
> +                                       goto enqueue;
> +                               }
> +
>                                myqueue = &__get_cpu_var(softnet_data);
>                                queue->rps_ipi_next = myqueue->rps_ipi_list;
>                                myqueue->rps_ipi_list = queue;
> @@ -3337,6 +3350,7 @@ static void net_rps_action(void)
>        struct softnet_data *locqueue = &__get_cpu_var(softnet_data);
>        struct softnet_data *remqueue = locqueue->rps_ipi_list;
>
> +       local_irq_disable();
>        if (remqueue) {
>                locqueue->rps_ipi_list = NULL;
>
> @@ -3350,9 +3364,10 @@ static void net_rps_action(void)
>                                                           &remqueue->csd, 0);
>                        remqueue = next;
>                }
> -       } else
> -#endif
> +       } else {
>                local_irq_enable();
> +       }
> +#endif
>  }
>
>  static void net_rx_action(struct softirq_action *h)
> @@ -3362,6 +3377,8 @@ static void net_rx_action(struct softirq_action *h)
>        int budget = netdev_budget;
>        void *have;
>
> +       net_rps_action();
> +
>        local_irq_disable();
>
>        while (!list_empty(list)) {
> @@ -3423,7 +3440,7 @@ static void net_rx_action(struct softirq_action *h)
>                netpoll_poll_unlock(have);
>        }
>  out:
> -       net_rps_action();
> +       local_irq_enable();
>
>  #ifdef CONFIG_NET_DMA
>        /*
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] rps: send IPIs ASAP
  2010-04-20  5:15 ` Tom Herbert
@ 2010-04-20  5:39   ` David Miller
  2010-04-20  5:45   ` Eric Dumazet
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2010-04-20  5:39 UTC (permalink / raw)
  To: therbert; +Cc: xiaosuo, eric.dumazet, netdev

From: Tom Herbert <therbert@google.com>
Date: Mon, 19 Apr 2010 22:15:58 -0700

> Did you test this to see what the impact is in this regard?

Changli it would help immensely if you posted performance
test results along with your changes.

I can see quite obviously that this change will completely undo the
intentional batching of IPIs done by RPS.  IPIs are expensive and we
should batch things as much as possible here.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] rps: send IPIs ASAP
  2010-04-20  5:15 ` Tom Herbert
  2010-04-20  5:39   ` David Miller
@ 2010-04-20  5:45   ` Eric Dumazet
  2010-04-20  9:17     ` Changli Gao
  1 sibling, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2010-04-20  5:45 UTC (permalink / raw)
  To: Tom Herbert; +Cc: Changli Gao, David S. Miller, netdev

Le lundi 19 avril 2010 à 22:15 -0700, Tom Herbert a écrit :
> On Mon, Apr 19, 2010 at 9:08 PM, Changli Gao <xiaosuo@gmail.com> wrote:
> > rps: send IPIs ASAP
> >
> > In order to reduce latency, we'd better send IPIs ASAP to schedule the
> > corresponding NAPIs.
> >
> A design point of RPS is that we generate at most one IPI per CPU per
> device interrupt, which at least offers some predictable coalescing.
> With your changes, we would get at most one IPI per packet-- that
> could represent a lot more of them.  Did you test this to see what the
> impact is in this regard?
> 

I agree with you Tom. Coalescing IPI is probably better.

If the receiver CPU got a single packet in its RX handling, latency will
be the same anyway.

If the receiver CPU got many packets, chance is high we are in a stress
situation, and coalescing is a win in this case.

I am currently testing a patch to call net_rps_action() at the beginning
of process_backlog() (if we have a non null ipi_rps_list pointer)

Will post a patch with bench results




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] rps: send IPIs ASAP
  2010-04-20  5:45   ` Eric Dumazet
@ 2010-04-20  9:17     ` Changli Gao
  0 siblings, 0 replies; 5+ messages in thread
From: Changli Gao @ 2010-04-20  9:17 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tom Herbert, David S. Miller, netdev

On Tue, Apr 20, 2010 at 1:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 19 avril 2010 à 22:15 -0700, Tom Herbert a écrit :
>> On Mon, Apr 19, 2010 at 9:08 PM, Changli Gao <xiaosuo@gmail.com> wrote:
>> > rps: send IPIs ASAP
>> >
>> > In order to reduce latency, we'd better send IPIs ASAP to schedule the
>> > corresponding NAPIs.
>> >
>> A design point of RPS is that we generate at most one IPI per CPU per
>> device interrupt, which at least offers some predictable coalescing.
>> With your changes, we would get at most one IPI per packet-- that
>> could represent a lot more of them.  Did you test this to see what the
>> impact is in this regard?
>>
>
> I agree with you Tom. Coalescing IPI is probably better.
>
> If the receiver CPU got a single packet in its RX handling, latency will
> be the same anyway.

I did the "ping -f" test again, and found that the differences of RTT
I got before were noises. It seems your "shortcut net_rps_action()"
patch eliminates the differences.

>
> If the receiver CPU got many packets, chance is high we are in a stress
> situation, and coalescing is a win in this case.
>
> I am currently testing a patch to call net_rps_action() at the beginning
> of process_backlog() (if we have a non null ipi_rps_list pointer)
>
> Will post a patch with bench results
>

It sounds like a better idea.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-04-20  9:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-20  4:08 [PATCH] rps: send IPIs ASAP Changli Gao
2010-04-20  5:15 ` Tom Herbert
2010-04-20  5:39   ` David Miller
2010-04-20  5:45   ` Eric Dumazet
2010-04-20  9:17     ` Changli Gao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.