lkml.org 
[lkml]   [2021]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [rcf/patch] netpoll: Make it RT friendly
From
Date
On Fri, 2021-12-17 at 17:41 +0100, Mike Galbraith wrote:
> On Fri, 2021-12-17 at 14:59 +0100, Sebastian Andrzej Siewior wrote:
> > On 2021-12-15 20:54:40 [+0100], Mike Galbraith wrote:
> > >                   In fact, I had already taken the silence as a "Surely
> > > you jest" or such.
> >
> > No, not really. netpoll is currently the least of my worries and I
> > wanted to finish my other network patches before I start wrapping my
> > mind around this in case something changes. But I have currently no idea
> > how to do netpoll run/ detect and so on.
>
> I was gonna toss a rock or two at those "and so on" bits since they're
> wired into stuff my box uses regularly.. but I've yet to find a means
> to make them be more that compiler fodder, so they're safe.

Nah, I was just insufficiently bored.

It's surprising that patchlet worked at all with netpoll_tx_running()
being busted, but whatever, bridge now does its forwarding thing for RT
and !RT, transmitting twice per kmsg, though apparently only 1 actually
hits copper to fly over to console logger. See attached trace. Note
that for that trace I sabotaged it to alternately xmit via fallback.

This will likely end up being done differently (way prettier), but what
the heck, it beats a lump of coal in your xmas stocking.

Ho Ho Hum :)

-Mike

netpoll: Make it RT friendly

PREEMPT_RT cannot alloc/free memory when not preemptible, making
disabling of IRQs across transmission an issue for RT.

Use rcu_read_lock_bh() to provide local exclusion for RT (via
softirq_ctrl.lock) for normal and fallback transmission paths
instead of disabling IRQs. Since zap_completion_queue() callers
ensure pointer stability, replace get_cpu_var() therein with
this_cpu_ptr() to keep it preemptible across kfree().

Disable a couple warnings for RT, and we're done.

v0.kinda-works -> v1 feedback fixes:
remove get_cpu_var() from zap_completion_queue().
fix/test netpoll_tx_running() to work for RT/!RT.
fix/test xmit fallback path for RT.

Signed-off-by: Mike Galbraith <efault@gmx.de>
---
include/linux/netpoll.h | 4 +++-
net/core/netpoll.c | 44 ++++++++++++++++++++++++++++++++------------
2 files changed, 35 insertions(+), 13 deletions(-)

--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -89,9 +89,11 @@ static inline void netpoll_poll_unlock(v
smp_store_release(&napi->poll_owner, -1);
}

+DECLARE_PER_CPU(int, _netpoll_tx_running);
+
static inline bool netpoll_tx_running(struct net_device *dev)
{
- return irqs_disabled();
+ return this_cpu_read(_netpoll_tx_running);
}

#else
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -70,6 +70,27 @@ module_param(carrier_timeout, uint, 0644
#define np_notice(np, fmt, ...) \
pr_notice("%s: " fmt, np->name, ##__VA_ARGS__)

+DEFINE_PER_CPU(int, _netpoll_tx_running);
+EXPORT_PER_CPU_SYMBOL(_netpoll_tx_running);
+
+#define netpoll_tx_begin(flags) \
+ do { \
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT)) \
+ local_irq_save(flags); \
+ else \
+ rcu_read_lock_bh(); \
+ this_cpu_write(_netpoll_tx_running, 1); \
+ } while (0)
+
+#define netpoll_tx_end(flags) \
+ do { \
+ this_cpu_write(_netpoll_tx_running, 0); \
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT)) \
+ local_irq_restore(flags); \
+ else \
+ rcu_read_unlock_bh(); \
+ } while (0)
+
static netdev_tx_t netpoll_start_xmit(struct sk_buff *skb,
struct net_device *dev,
struct netdev_queue *txq)
@@ -102,7 +123,7 @@ static void queue_process(struct work_st
struct netpoll_info *npinfo =
container_of(work, struct netpoll_info, tx_work.work);
struct sk_buff *skb;
- unsigned long flags;
+ unsigned long __maybe_unused flags;

while ((skb = skb_dequeue(&npinfo->txq))) {
struct net_device *dev = skb->dev;
@@ -114,7 +135,7 @@ static void queue_process(struct work_st
continue;
}

- local_irq_save(flags);
+ netpoll_tx_begin(flags);
/* check if skb->queue_mapping is still valid */
q_index = skb_get_queue_mapping(skb);
if (unlikely(q_index >= dev->real_num_tx_queues)) {
@@ -127,13 +148,13 @@ static void queue_process(struct work_st
!dev_xmit_complete(netpoll_start_xmit(skb, dev, txq))) {
skb_queue_head(&npinfo->txq, skb);
HARD_TX_UNLOCK(dev, txq);
- local_irq_restore(flags);
+ netpoll_tx_end(flags);

schedule_delayed_work(&npinfo->tx_work, HZ/10);
return;
}
HARD_TX_UNLOCK(dev, txq);
- local_irq_restore(flags);
+ netpoll_tx_end(flags);
}
}

@@ -243,7 +264,7 @@ static void refill_skbs(void)
static void zap_completion_queue(void)
{
unsigned long flags;
- struct softnet_data *sd = &get_cpu_var(softnet_data);
+ struct softnet_data *sd = this_cpu_ptr(&softnet_data);

if (sd->completion_queue) {
struct sk_buff *clist;
@@ -264,8 +285,6 @@ static void zap_completion_queue(void)
}
}
}
-
- put_cpu_var(softnet_data);
}

static struct sk_buff *find_skb(struct netpoll *np, int len, int reserve)
@@ -314,7 +333,8 @@ static netdev_tx_t __netpoll_send_skb(st
/* It is up to the caller to keep npinfo alive. */
struct netpoll_info *npinfo;

- lockdep_assert_irqs_disabled();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ lockdep_assert_irqs_disabled();

dev = np->dev;
npinfo = rcu_dereference_bh(dev->npinfo);
@@ -350,7 +370,7 @@ static netdev_tx_t __netpoll_send_skb(st
udelay(USEC_PER_POLL);
}

- WARN_ONCE(!irqs_disabled(),
+ WARN_ONCE(!IS_ENABLED(CONFIG_PREEMPT_RT) && !irqs_disabled(),
"netpoll_send_skb_on_dev(): %s enabled interrupts in poll (%pS)\n",
dev->name, dev->netdev_ops->ndo_start_xmit);

@@ -365,16 +385,16 @@ static netdev_tx_t __netpoll_send_skb(st

netdev_tx_t netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
{
- unsigned long flags;
+ unsigned long __maybe_unused flags;
netdev_tx_t ret;

if (unlikely(!np)) {
dev_kfree_skb_irq(skb);
ret = NET_XMIT_DROP;
} else {
- local_irq_save(flags);
+ netpoll_tx_begin(flags);
ret = __netpoll_send_skb(np, skb);
- local_irq_restore(flags);
+ netpoll_tx_end(flags);
}
return ret;
}

# tracer: nop
#
netconsole.sh-4041 [005] ....... 189.802694: netpoll_setup+0x5/0x460: struct netpoll *np:ffff8881eea7f128
netconsole.sh-4041 [005] ....... 189.802706: __netpoll_setup+0x5/0x240: struct netpoll *np:ffff8881eea7f128, struct net_device *ndev:ffff888104c0a000
netconsole.sh-4041 [005] ....... 189.802707: __netpoll_setup+0x165/0x240: !ndev->npinfo ==> ops->ndo_netpoll_setup()
netconsole.sh-4041 [005] ....... 189.802708: __netpoll_setup+0x5/0x240: struct netpoll *np:ffff88818c733180, struct net_device *ndev:ffff888107de8000
netconsole.sh-4041 [005] ....... 189.802708: __netpoll_setup+0x1cb/0x240: !ndev->npinfo and !ops->ndo_netpoll_setup
netconsole.sh-4041 [005] ....... 189.802708: br_netpoll_setup+0x4b/0xa0 [bridge]: __br_netpoll_enable()
netconsole.sh-4041 [005] ....... 189.802709: br_netpoll_setup+0x69/0xa0 [bridge]: return:0
netconsole.sh-4041 [005] ....... 189.802709: netpoll_setup+0x1fe/0x460: __netpoll_setup() return:0
pr/netcon0-4045 [006] .....13 189.803951: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [006] .....13 189.803954: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [006] .....13 189.803954: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [006] .....13 189.803957: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [006] .....13 189.803957: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [006] .....13 189.803957: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [006] .....13 189.803958: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [006] .....13 189.803958: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [006] .....13 189.803959: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/6:1-101 [006] .....13 189.803970: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/6:1-101 [006] .....13 189.803971: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/6:1-101 [006] .....13 189.803971: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [001] .....13 189.803978: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [001] .....13 189.803981: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [001] .....13 189.803981: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/1:0-25 [001] .....13 189.803986: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [006] .....13 189.803989: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [006] .....13 189.803990: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [006] .....13 189.803990: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/6:1-101 [006] .....13 189.803993: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [001] .....13 189.803998: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [001] .....13 189.804000: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [001] .....13 189.804000: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/1:0-25 [001] .....13 189.804003: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [006] .....13 189.804006: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [006] .....13 189.804007: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [006] .....13 189.804007: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/6:1-101 [006] .....13 189.804010: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
pr/netcon0-4045 [006] .....13 206.226295: __br_forward+0x176/0x1b0 [bridge]: netpoll_tx_running() ==> br_netpoll_send_skb()
pr/netcon0-4045 [006] .....13 206.226299: netpoll_send_skb+0x7f/0x250: netpoll_start_xmit() ==> schedule_delayed_work()
pr/netcon0-4045 [006] .....13 206.226299: netpoll_send_skb+0x1c1/0x250: netpoll_start_xmit() ==> dev_xmit_complete()
kworker/6:1-101 [006] .....13 206.226304: queue_process+0xc3/0x230: netpoll_start_xmit() ==> dev_xmit_complete()
\
 
 \ /
  Last update: 2021-12-21 10:44    [W:0.050 / U:0.772 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site