Skip to contentSkip to conversation

rps: send IPIs ASAP

PATCH In review Changli Gao 4 participants 5 messages Apr 20, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rps: send IPIs ASAP

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.

----

 net/core/dev.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)
net/core/dev.c +20 -3
index 05a2b29..d8fca21 100644
@@ -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
 	/*
#1 lore raw
Quoted 5 lines · Changli Gao

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?

Quoted 87 lines

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.

----
 net/core/dev.c |   23 ++++++++++++++++++++---

 1 file changed, 20 insertions(+), 3 deletions(-)
net/core/dev.c +19 -2
index 05a2b29..d8fca21 100644
@@ -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
       /*

maintainer
#2 lore raw

From: Tom Herbert <[email protected]>
Date: Mon, 19 Apr 2010 22:15:58 -0700

Quoted 1 line

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.

#3 lore raw
In reply to Tom Herbert
Quoted 12 lines · Tom Herbert
Quoted 5 lines · Changli Gao

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

#4 lore raw
Quoted 14 lines · Eric Dumazet
Quoted 12 lines · Tom Herbert
Quoted 5 lines · Changli Gao

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?

Quoted 4 lines

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.

Quoted 9 lines

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.

Signature · 2 lines
Regards,
Changli Gao([email protected])