net: batch skb dequeueing from softnet input_pkt_queue
Subsystems (2)
PATCH v2 1/1 net: batch skb dequeueing from softnet input_pkt_queue
batch skb dequeueing from softnet input_pkt_queue
batch skb dequeueing from softnet input_pkt_queue to reduce potential lock contention and irq disabling/enabling.
----
include/linux/netdevice.h | 2 ++ net/core/dev.c | 45 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 6 deletions(-)
include/linux/netdevice.h +2 -0
@@ -1335,6 +1335,8 @@ struct softnet_data {struct call_single_data csd____cacheline_aligned_in_smp ;#endif struct sk_buff_head input_pkt_queue; +struct sk_buff_head processing_queue; +volatile bool flush_processing_queue;struct napi_struct backlog; };
net/core/dev.c +39 -6
94 lines not on this page · raw message
Quoted 5 lines · Changli Gao
batch skb dequeueing from softnet
input_pkt_queuebatch skb dequeueing from softnet
input_pkt_queueto reduce potential lock contention and irq disabling/enabling.
Very interesting idea, but implementation is too complex, and probably buggy, in a area that too few people understand today.
Could you keep it as simple as possible ?
Quoted 10 lines
Signed-off-by Changli Gao <[email protected]>----
include/linux/netdevice.h | 2 ++ net/core/dev.c | 45 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 6 deletions(-)include/linux/netdevice.h +0
-0
include/linux/netdevice.h struct softnet_data { · line 1338
Quoted 4 lines
include/linux/netdevice.h +1
-0@@ -1335,6 +1335,8 @@ struct softnet_data {struct call_single_data csd____cacheline_aligned_in_smp ;#endif struct sk_buff_head input_pkt_queue; +struct sk_buff_head processing_queue;
Probably not necessary.
include/linux/netdevice.h struct softnet_data { · line 1339
Quoted 1 line
include/linux/netdevice.h +1
-0+volatile bool flush_processing_queue;
Use of 'volatile' is strongly discouraged, I would say, forbidden.
Its usually a sign of 'I dont exactly what memory ordering I need, so I throw a volatile just in case'. We live in a world full of RCU, read , write, full barriers. And these apis are well documented.
Quoted 31 lines
include/linux/netdevice.h +0
-0struct napi_struct backlog; };net/core/dev.c +7
-2@@ -2324,6 +2324,11 @@ static void trigger_softirq(void *data) }#endif /* CONFIG_SMP */ +static inline u32 softnet_input_qlen (struct softnet_data *queue) +{ +return queue->input_pkt_queue.qlen + queue->processing_queue.qlen; +} +/* * enqueue_to_backlog is called to queue an skb to a per CPU backlog * queue (may be a remote CPU queue). @@ -2339,8 +2344,8 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu)__get_cpu_var (netdev_rx_stat).total++;rps_lock (queue); -if (queue->input_pkt_queue.qlen <= netdev_max_backlog) { -if (queue->input_pkt_queue.qlen) { +if (softnet_input_qlen (queue) <= netdev_max_backlog) { +if (softnet_input_qlen (queue)) { enqueue:__skb_queue_tail (&queue->input_pkt_queue, skb);rps_unlock (queue);
net/core/dev.c static void flush_backlog(void *arg) · line 2811
Quoted 4 lines
net/core/dev.c +1
-0@@ -2803,6 +2808,7 @@ static void flush_backlog(void *arg)__skb_unlink (skb, &queue->input_pkt_queue);kfree_skb (skb); } + queue->flush_processing_queue = true;
Probably not necessary
net/core/dev.c static int process_backlog(struct napi_struct *napi, int quota) · line 3121
Quoted 8 lines
net/core/dev.c +1
-0rps_unlock (queue); }@@ -3112,14 +3118,23 @@ static int process_backlog(struct napi_struct *napi, int quota)struct softnet_data *queue = &__get_cpu_var (softnet_data);unsigned long start_time = jiffies; +if (queue->flush_processing_queue) {
Really... this is bloat IMHO
Quoted 24 lines
net/core/dev.c +10
-2+struct sk_buff *skb; + + queue->flush_processing_queue = false; +while ((skb =__skb_dequeue (&queue->processing_queue))) +kfree_skb (skb); + } + napi->weight = weight_p; do { struct sk_buff *skb; local_irq_disable(); rps_lock(queue); - skb = __skb_dequeue(&queue->input_pkt_queue); - if (!skb) { + skb_queue_splice_tail_init(&queue->input_pkt_queue, + &queue->processing_queue); + if (skb_queue_empty(&queue->processing_queue)) { __napi_complete(napi); rps_unlock(queue); local_irq_enable();@@ -3128,9 +3143,22 @@ static int process_backlog(struct napi_struct *napi, int quota)rps_unlock (queue);local_irq_enable ();
net/core/dev.c static int process_backlog(struct napi_struct *napi, int quota) · line 3152
Quoted 10 lines
net/core/dev.c +7
-2-__netif_receive_skb (skb); - }while (++work < quota && jiffies == start_time); +while ((skb =__skb_dequeue (&queue->processing_queue))) { +__netif_receive_skb (skb); +if (++work < quota && jiffies == start_time) +continue ; +if (!queue->flush_processing_queue) + goto out; + queue->flush_processing_queue = false;
once again, ... so much code for a unlikely event...
Quoted 23 lines
net/core/dev.c +11
-0+while ((skb =__skb_dequeue (&queue->processing_queue))) { +__netif_receive_skb (skb); + ++work; + } + goto out; + } + }while (1 ); +out:return work; }@@ -5487,6 +5515,9 @@ static int dev_cpu_callback(struct notifier_block *nfb,raise_softirq_irqoff (NET_TX_SOFTIRQ );local_irq_enable (); +while ((skb =__skb_dequeue (&oldsd->processing_queue))) +netif_rx (skb); +/* Process offline CPU's input_pkt_queue */ while ((skb =__skb_dequeue (&oldsd->input_pkt_queue)))netif_rx (skb);
net/core/dev.c static int __init net_dev_init(void) · line 5747
Quoted 8 lines
net/core/dev.c +2
-0@@ -5709,6 +5740,8 @@ static int __init net_dev_init(void) queue = &per_cpu (softnet_data, i);skb_queue_head_init (&queue->input_pkt_queue); +skb_queue_head_init (&queue->processing_queue); + queue->flush_processing_queue = false; queue->completion_queue =NULL ;INIT_LIST_HEAD (&queue->poll_list);
I advise to keep it simple.
My suggestion would be to limit this patch only to process_backlog().
Really if you touch other areas, there is too much risk.
Perform sort of skb_queue_splice_tail_init() into a local (stack) queue, but the trick is to not touch input_pkt_queue.qlen, so that we dont slow down enqueue_to_backlog().
Process at most 'quota' skbs (or jiffies limit).
relock queue.
input_pkt_queue.qlen -= number_of_handled_skbs;
In the unlikely event we have unprocessed skbs in local queue, re-insert the remaining skbs at head of input_pt_queue.
Consider if input_pkt_queue.qlen is 0 or not, to call __napi_complete(napi); or not :)
Quoted 7 lines · Eric Dumazet
Probably not necessary.
Quoted 1 line
+ volatile bool
flush_processing_queue;Use of 'volatile' is strongly discouraged, I would say, forbidden.
volatile is used to avoid compiler optimization.
Quoted 4 lines
Its usually a sign of 'I dont exactly what memory ordering I need, so I throw a volatile just in case'. We live in a world full of RCU, read , write, full barriers. And these apis are well documented.
There isn't memory accessing order problem.
Quoted 8 lines
net/core/dev.c static void flush_backlog(void *arg) · line 2811
Quoted 5 lines
net/core/dev.c +1
-0@@ -2803,6 +2808,7 @@ static void flush_backlog(void *arg) __skb_unlink(skb, &queue->input_pkt_queue); kfree_skb(skb); } + queue->flush_processing_queue = true;Probably not necessary
If flush_backlog() is called when there are still packets in processing_queue, there maybe some packets refer to the netdev gone, if we remove this line.
Quoted 10 lines
net/core/dev.c static int process_backlog(struct napi_struct *napi, int quota) · line 3121
Quoted 8 lines
net/core/dev.c +1
-0rps_unlock(queue); }@@ -3112,14 +3118,23 @@ static int process_backlog(struct napi_struct *napi, int quota) struct softnet_data *queue = &__get_cpu_var(softnet_data); unsigned long start_time = jiffies; + if (queue->flush_processing_queue) {Really... this is bloat IMHO
Any better idea?
Quoted 9 lines
Quoted 2 lines
I advise to keep it simple.
My suggestion would be to limit this patch only to
process_backlog().Really if you touch other areas, there is too much risk.
Quoted 9 lines
Perform sort of
skb_queue_splice_tail_init()into a local (stack) queue, but the trick is to not touch input_pkt_queue.qlen, so that we dont slow downenqueue_to_backlog().Process at most 'quota' skbs (or jiffies limit).
relock queue.
input_pkt_queue.qlen -=number_of_handled_skbs;
Oh no, in order to let latter packets in as soon as possible, we have to update qlen immediately.
Signature · 2 lines
Regards， Changli Gao([email protected])
Quoted 10 lines · Changli Gao
Quoted 7 lines · Eric Dumazet
Probably not necessary.
Quoted 1 line
+ volatile bool
flush_processing_queue;Use of 'volatile' is strongly discouraged, I would say, forbidden.
volatile is used to avoid compiler optimization.
volatile might be used on special macros only, not to guard a variable. volatile was pre SMP days. We need something better defined these days.
Quoted 8 lines
Quoted 5 lines
Its usually a sign of 'I dont exactly what memory ordering I need, so I throw a volatile just in case'. We live in a world full of RCU, read , write, full barriers. And these apis are well documented.
There isn't memory accessing order problem.
Really ? If you persist using this volatile thing, then you'll have to convince Mr Linus Torvald himself. Prepare your best guns, because in fact you'll have to convince about one hundred people that know for sure how volatile is weak.
Quoted 9 lines
Quoted 8 lines
net/core/dev.c static void flush_backlog(void *arg) · line 2811
Quoted 5 lines
net/core/dev.c +1
-0@@ -2803,6 +2808,7 @@ static void flush_backlog(void *arg)__skb_unlink (skb, &queue->input_pkt_queue);kfree_skb (skb); } + queue->flush_processing_queue = true;Probably not necessary
Quoted 3 lines
If
flush_backlog()is called when there are still packets inprocessing_queue, there maybe some packets refer to the netdev gone, if we remove this line.
We dont need this "processing_queue". Once you remove it, there is no extra work to perform.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
net/core/dev.c +0
-0rps_unlock (queue); }@@ -3112,14 +3118,23 @@ static int process_backlog(struct napi_struct *napi, int quota)struct softnet_data *queue = &__get_cpu_var (softnet_data);unsigned long start_time = jiffies;
Quoted 7 lines
Quoted 3 lines
net/core/dev.c static int process_backlog(struct napi_struct *napi, int quota) · line 3121
Quoted 1 line
net/core/dev.c +1
-0+if (queue->flush_processing_queue) {Really... this is bloat IMHO
Any better idea?
Yes, I have explained it to you.
Quoted 13 lines
Quoted 13 lines
Quoted 2 lines
I advise to keep it simple.
My suggestion would be to limit this patch only to
process_backlog().Really if you touch other areas, there is too much risk.
Perform sort of
skb_queue_splice_tail_init()into a local (stack) queue, but the trick is to not touch input_pkt_queue.qlen, so that we dont slow downenqueue_to_backlog().
Quoted 9 lines
Quoted 5 lines
Process at most 'quota' skbs (or jiffies limit).
relock queue.
input_pkt_queue.qlen -=number_of_handled_skbs;Oh no, in order to let latter packets in as soon as possible, we have to update qlen immediately.
Absolutely not. You missed something apparently.
You pay the price at each packet enqueue, because you have to compute the sum of two lengthes, and guess what, if you do this you have a cache line miss in one of the operand. Your patch as is is suboptimal.
Remember : this batch mode should not change packet queueing at all, only speed it because of less cache line misses.
Quoted 16 lines · Eric Dumazet
Quoted 5 lines · Changli Gao
Quoted 2 lines · Eric Dumazet
Use of 'volatile' is strongly discouraged, I would say, forbidden.
volatile is used to avoid compiler optimization.
volatile might be used on special macros only, not to guard a variable. volatile was pre SMP days. We need something better defined these days.
Quoted 6 lines
Quoted 5 lines
Its usually a sign of 'I dont exactly what memory ordering I need, so I throw a volatile just in case'. We live in a world full of RCU, read , write, full barriers. And these apis are well documented.
Quoted 7 lines
Quoted 2 lines
There isn't memory accessing order problem.
Really ? If you persist using this volatile thing, then you'll have to convince Mr Linus Torvald himself. Prepare your best guns, because in fact you'll have to convince about one hundred people that know for sure how volatile is weak.
$ cat Documentation/volatile-considered-harmful.txt Why the "volatile" type class should not be used ------------------------------------------------
C programmers have often taken volatile to mean that the variable could be changed outside of the current thread of execution; as a result, they are sometimes tempted to use it in kernel code when shared data structures are being used. In other words, they have been known to treat volatile types as a sort of easy atomic variable, which they are not. The use of volatile in kernel code is almost never correct; this document describes why.
The key point to understand with regard to volatile is that its purpose is to suppress optimization, which is almost never what one really wants to do. In the kernel, one must protect shared data structures against unwanted concurrent access, which is very much a different task. The process of protecting against unwanted concurrency will also avoid almost all optimization-related problems in a more efficient way.
Like volatile, the kernel primitives which make concurrent access to data safe (spinlocks, mutexes, memory barriers, etc.) are designed to prevent unwanted optimization. If they are being used properly, there will be no need to use volatile as well. If volatile is still necessary, there is almost certainly a bug in the code somewhere. In properly-written kernel code, volatile can only serve to slow things down.
Consider a typical block of kernel code:
spin_lock(&the_lock);
do_something_on(&shared_data);
do_something_else_with(&shared_data);
spin_unlock(&the_lock);If all the code follows the locking rules, the value of shared_data cannot change unexpectedly while the_lock is held. Any other code which might want to play with that data will be waiting on the lock. The spinlock primitives act as memory barriers - they are explicitly written to do so - meaning that data accesses will not be optimized across them. So the compiler might think it knows what will be in shared_data, but the spin_lock() call, since it acts as a memory barrier, will force it to forget anything it knows. There will be no optimization problems with accesses to that data.
If shared_data were declared volatile, the locking would still be necessary. But the compiler would also be prevented from optimizing access to shared_data _within_ the critical section, when we know that nobody else can be working with it. While the lock is held, shared_data is not volatile. When dealing with shared data, proper locking makes volatile unnecessary - and potentially harmful.
The volatile storage class was originally meant for memory-mapped I/O registers. Within the kernel, register accesses, too, should be protected by locks, but one also does not want the compiler "optimizing" register accesses within a critical section. But, within the kernel, I/O memory accesses are always done through accessor functions; accessing I/O memory directly through pointers is frowned upon and does not work on all architectures. Those accessors are written to prevent unwanted optimization, so, once again, volatile is unnecessary.
Another situation where one might be tempted to use volatile is when the processor is busy-waiting on the value of a variable. The right way to perform a busy wait is:
while (my_variable != what_i_want)
cpu_relax();The cpu_relax() call can lower CPU power consumption or yield to a hyperthreaded twin processor; it also happens to serve as a compiler barrier, so, once again, volatile is unnecessary. Of course, busy- waiting is generally an anti-social act to begin with.
There are still a few rare situations where volatile makes sense in the kernel:
- The above-mentioned accessor functions might use volatile on architectures where direct I/O memory access does work. Essentially, each accessor call becomes a little critical section on its own and ensures that the access happens as expected by the programmer.
- Inline assembly code which changes memory, but which has no other visible side effects, risks being deleted by GCC. Adding the volatile keyword to asm statements will prevent this removal.
- The jiffies variable is special in that it can have a different value every time it is referenced, but it can be read without any special locking. So jiffies can be volatile, but the addition of other variables of this type is strongly frowned upon. Jiffies is considered to be a "stupid legacy" issue (Linus's words) in this regard; fixing it would be more trouble than it is worth.
- Pointers to data structures in coherent memory which might be modified by I/O devices can, sometimes, legitimately be volatile. A ring buffer used by a network adapter, where that adapter changes pointers to indicate which descriptors have been processed, is an example of this type of situation.
For most code, none of the above justifications for volatile apply. As a result, the use of volatile is likely to be seen as a bug and will bring additional scrutiny to the code. Developers who are tempted to use volatile should take a step back and think about what they are truly trying to accomplish.
Patches to remove volatile variables are generally welcome - as long as they come with a justification which shows that the concurrency issues have been properly thought through.
NOTES
-----
[1] http://lwn.net/Articles/233481/
[2] http://lwn.net/Articles/233482/
CREDITS
-------
Original impetus and research by Randy Dunlap
Written by Jonathan Corbet
Improvements via comments from Satyam Sharma, Johannes Stezenbach, Jesper
Juhl, Heikki Orsila, H. Peter Anvin, Philipp Hahn, and Stefan Richter.
Quoted 7 lines · Eric Dumazet
Quoted 6 lines · Changli Gao
Quoted 5 lines · Eric Dumazet
Probably not necessary.
Quoted 1 line
+ volatile bool
flush_processing_queue;
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Use of 'volatile' is strongly discouraged, I would say, forbidden.
volatile is used to avoid compiler optimization.
volatile might be used on special macros only, not to guard a variable. volatile was pre SMP days. We need something better defined these days.
flush_processing_queue is only accessed on the same CPU, so no volatile is needed. I'll remove it in the next version.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
net/core/dev.c static void flush_backlog(void *arg) · line 2811
Quoted 5 lines
net/core/dev.c +1
-0@@ -2803,6 +2808,7 @@ static void flush_backlog(void *arg) __skb_unlink(skb, &queue->input_pkt_queue); kfree_skb(skb); } + queue->flush_processing_queue = true;Probably not necessary
Quoted 6 lines
Quoted 3 lines
If
flush_backlog()is called when there are still packets inprocessing_queue, there maybe some packets refer to the netdev gone, if we remove this line.We dont need this "
processing_queue". Once you remove it, there is no extra work to perform.
OK. If we make processing_queue is a stack variable. When quota or jiffies limit is reached, we have to splice processing_queue back to input_pkt_queue. If flush_backlog() is called before the processing_queue is spliced, there will still packets which refer to the NIC going. Then these packets are queued to input_pkt_queue. When process_backlog() is called again, the dev field of these skbs are wild...
Oh, my GOD. When RPS is enabled, if flush_backlog(eth0) is called on CPU1 when a skb0(eth0) is dequeued from CPU0's softnet and isn't queued to CPU1's softnet, what will happen?
Quoted 24 lines
Quoted 14 lines
Quoted 14 lines
Quoted 3 lines
I advise to keep it simple.
My suggestion would be to limit this patch only to
process_backlog().Really if you touch other areas, there is too much risk.
Perform sort of
skb_queue_splice_tail_init()into a local (stack) queue, but the trick is to not touch input_pkt_queue.qlen, so that we dont slow downenqueue_to_backlog().Quoted 9 lines
Quoted 5 lines
Process at most 'quota' skbs (or jiffies limit).
relock queue.
input_pkt_queue.qlen -=number_of_handled_skbs;Oh no, in order to let latter packets in as soon as possible, we have to update qlen immediately.
Quoted 9 lines
Absolutely not. You missed something apparently.
You pay the price at each packet enqueue, because you have to compute the sum of two lengthes, and guess what, if you do this you have a cache line miss in one of the operand. Your patch as is is suboptimal.
Remember : this batch mode should not change packet queueing at all, only speed it because of less cache line misses.
WoW, is it really so expensive?
Signature · 2 lines
Regards， Changli Gao([email protected])
Quoted 8 lines · Changli Gao
OK. If we make
processing_queueis a stack variable. When quota or jiffies limit is reached, we have to spliceprocessing_queueback toinput_pkt_queue. Ifflush_backlog()is called before theprocessing_queueis spliced, there will still packets which refer to the NIC going. Then these packets are queued toinput_pkt_queue. Whenprocess_backlog()is called again, the dev field of these skbs are wild...
This is a problem of cooperation between flush_backlog() and process_backlog(). Dont allow flush_backlog() to return if process_backlog() is running. Exactly as before, but lock acquisition done in flush_backlog() should be a bit smarter.
Quoted 4 lines
Oh, my GOD. When RPS is enabled, if
flush_backlog(eth0)is called on CPU1 when a skb0(eth0) is dequeued from CPU0's softnet and isn't queued to CPU1's softnet, what will happen?
I am a bit lost here. flush_backlog() drops skbs, not requeue them.
Quoted 7 lines
Quoted 7 lines
Absolutely not. You missed something apparently.
You pay the price at each packet enqueue, because you have to compute the sum of two lengthes, and guess what, if you do this you have a cache line miss in one of the operand. Your patch as is is suboptimal.
Quoted 6 lines
Quoted 3 lines
Remember : this batch mode should not change packet queueing at all, only speed it because of less cache line misses.
WoW, is it really so expensive?
Yes. Whole point of your idea is to remove cache line misses.
They cost much more than a spinlock/unlock pair
Quoted 6 lines · Eric Dumazet
This is a problem of cooperation between
flush_backlog()andprocess_backlog(). Dont allowflush_backlog()to return ifprocess_backlog()is running. Exactly as before, but lock acquisition done inflush_backlog()should be a bit smarter.
flush_backlog() is called in IRQ context. Unless you disable irq in process_backlog(), you can't block flush_backlog().
Quoted 8 lines
Quoted 5 lines
Oh, my GOD. When RPS is enabled, if
flush_backlog(eth0)is called on CPU1 when a skb0(eth0) is dequeued from CPU0's softnet and isn't queued to CPU1's softnet, what will happen?I am a bit lost here.
flush_backlog()drops skbs, not requeue them.
I mean flush_backlog() don't drop all the packets, whose dev point to a special net_device and can't be processed before the net_device disappers.
Signature · 2 lines
Regards， Changli Gao([email protected])
Quoted 11 lines · Changli Gao
Quoted 6 lines · Eric Dumazet
This is a problem of cooperation between
flush_backlog()andprocess_backlog(). Dont allowflush_backlog()to return ifprocess_backlog()is running. Exactly as before, but lock acquisition done inflush_backlog()should be a bit smarter.
flush_backlog()is called in IRQ context. Unless you disable irq inprocess_backlog(), you can't blockflush_backlog().
There is nothing preventing flush_backlog() to be done differently you know. It was done like that because it was the most simple thing to do given the (basic) constraints. Now if the constraints change, implementation might change too. It is slow path (in most setups) and some extra work to keep fast path really fast is ok.
netdevice are dismantled and we respect an RCU grace period before freeing. process_backlog() is done inside a rcu lock, so everything is possible.
Quoted 9 lines · Changli Gao
Quoted 6 lines · Eric Dumazet
Probably not necessary.
Quoted 1 line
+ volatile bool
flush_processing_queue;Use of 'volatile' is strongly discouraged, I would say, forbidden.
volatile is used to avoid compiler optimization.
Would it be reasonable to use ACCESS_ONCE() where this variable is used?
Thanx, Paul
Quoted 63 lines
Quoted 4 lines
Its usually a sign of 'I dont exactly what memory ordering I need, so I throw a volatile just in case'. We live in a world full of RCU, read , write, full barriers. And these apis are well documented.
There isn't memory accessing order problem.
Quoted 8 lines
net/core/dev.c static void flush_backlog(void *arg) · line 2811
Quoted 5 lines
net/core/dev.c +1
-0@@ -2803,6 +2808,7 @@ static void flush_backlog(void *arg) __skb_unlink(skb, &queue->input_pkt_queue); kfree_skb(skb); } + queue->flush_processing_queue = true;Probably not necessary
If
flush_backlog()is called when there are still packets inprocessing_queue, there maybe some packets refer to the netdev gone, if we remove this line.Quoted 10 lines
net/core/dev.c static int process_backlog(struct napi_struct *napi, int quota) · line 3121
Quoted 8 lines
net/core/dev.c +1
-0rps_unlock(queue); }@@ -3112,14 +3118,23 @@ static int process_backlog(struct napi_struct *napi, int quota) struct softnet_data *queue = &__get_cpu_var(softnet_data); unsigned long start_time = jiffies; + if (queue->flush_processing_queue) {Really... this is bloat IMHO
Any better idea?
Quoted 9 lines
Quoted 2 lines
I advise to keep it simple.
My suggestion would be to limit this patch only to
process_backlog().Really if you touch other areas, there is too much risk.
Quoted 9 lines
Perform sort of
skb_queue_splice_tail_init()into a local (stack) queue, but the trick is to not touch input_pkt_queue.qlen, so that we dont slow downenqueue_to_backlog().Process at most 'quota' skbs (or jiffies limit).
relock queue.
input_pkt_queue.qlen -=number_of_handled_skbs;Oh no, in order to let latter packets in as soon as possible, we have to update qlen immediately.
Signature · 6 lines
Regards， Changli Gao([email protected]) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 5 lines · Paul E. McKenney
Quoted 4 lines · Changli Gao
Quoted 3 lines · Eric Dumazet
Probably not necessary.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
+ volatile bool
flush_processing_queue;Use of 'volatile' is strongly discouraged, I would say, forbidden.
volatile is used to avoid compiler optimization.
Would it be reasonable to use
ACCESS_ONCE()where this variable is used?
Oh, thanks. ACCESS_ONCE() is just what I need.
Signature · 2 lines
Regards， Changli Gao([email protected])