lkml.org 
[lkml]   [2020]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectQuestion on 5.4.55 merge into 5.4-rt

When merging 5.4.55 into 5.4-rt I hit the following conflict:

static void flush_backlog(struct work_struct *work)
{
struct sk_buff *skb, *tmp;
struct softnet_data *sd;

local_bh_disable();
sd = this_cpu_ptr(&softnet_data);

local_irq_disable();
rps_lock(sd);
skb_queue_walk_safe(&sd->input_pkt_queue, skb, tmp) {
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, &sd->input_pkt_queue);
<<<<<<< HEAD
__skb_queue_tail(&sd->tofree_queue, skb);
=======
dev_kfree_skb_irq(skb);
>>>>>>> v5.4.55
input_queue_head_incr(sd);
}
}

The diff of 5.4.54 -> 5.4.55 of this code is:

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5229,7 +5229,7 @@ static void flush_backlog(struct work_struct *work)
skb_queue_walk_safe(&sd->input_pkt_queue, skb, tmp) {
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, &sd->input_pkt_queue);
- kfree_skb(skb);
+ dev_kfree_skb_irq(skb);
input_queue_head_incr(sd);
}
}

From upstream commit:

7df5cb75cfb8a ("dev: Defer free of skbs in flush_backlog")

According to that commit, it looks like kfree_skb() shouldn't be called
with irqs disabled (yeah for RT!). It now calls dev_kfree_skb_irq()
which puts the skb on the softnet_data.completion_queue, and raises the
NET_TX_SOFTIRQ to do the freeing.


This is similar to what v5.4-rt does, which a diff of 5.4.54 -> v5.4-rt:

@@ -5229,7 +5234,7 @@ static void flush_backlog(struct work_struct *work)
skb_queue_walk_safe(&sd->input_pkt_queue, skb, tmp) {
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, &sd->input_pkt_queue);
- kfree_skb(skb);
+ __skb_queue_tail(&sd->tofree_queue, skb);
input_queue_head_incr(sd);
}
}
@@ -5239,11 +5244,14 @@ static void flush_backlog(struct work_struct *work)
skb_queue_walk_safe(&sd->process_queue, skb, tmp) {
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, &sd->process_queue);
- kfree_skb(skb);
+ __skb_queue_tail(&sd->tofree_queue, skb);
input_queue_head_incr(sd);
}
}
+ if (!skb_queue_empty(&sd->tofree_queue))
+ raise_softirq_irqoff(NET_RX_SOFTIRQ);
local_bh_enable();
+
}


Where we are doing something slightly different. Placing the skb on the
sd->tofree_queue and raising NET_RX_SOFTIQ instead.
Now that the vanilla stable 5.4 kernel doesn't call kfree_skb() from
irqs_disabled, can I safely revert this entire change?

Is it safe to call kfree_skb() from local_bh_disable()?

I'm assuming it is, but just want to clarify. I'll be continuing
merging latest stable (with this revert), but please yell if you think
it will break?

Thanks!

-- Steve

\
 
 \ /
  Last update: 2020-08-15 00:54    [W:0.054 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site