Skip to content

Commit 6510ea9

Browse files
Sebastian Andrzej Siewiorkuba-moo
authored andcommitted
net: Use this_cpu_inc() to increment net->core_stats
The macro dev_core_stats_##FIELD##_inc() disables preemption and invokes netdev_core_stats_alloc() to return a per-CPU pointer. netdev_core_stats_alloc() will allocate memory on its first invocation which breaks on PREEMPT_RT because it requires non-atomic context for memory allocation. This can be avoided by enabling preemption in netdev_core_stats_alloc() assuming the caller always disables preemption. It might be better to replace local_inc() with this_cpu_inc() now that dev_core_stats_##FIELD##_inc() gained a preempt-disable section and does not rely on already disabled preemption. This results in less instructions on x86-64: local_inc: | incl %gs:__preempt_count(%rip) # __preempt_count | movq 488(%rdi), %rax # _1->core_stats, _22 | testq %rax, %rax # _22 | je .L585 #, | add %gs:this_cpu_off(%rip), %rax # this_cpu_off, tcp_ptr__ | .L586: | testq %rax, %rax # _27 | je .L587 #, | incq (%rax) # _6->a.counter | .L587: | decl %gs:__preempt_count(%rip) # __preempt_count this_cpu_inc(), this patch: | movq 488(%rdi), %rax # _1->core_stats, _5 | testq %rax, %rax # _5 | je .L591 #, | .L585: | incq %gs:(%rax) # _18->rx_dropped Use unsigned long as type for the counter. Use this_cpu_inc() to increment the counter. Use a plain read of the counter. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/r/YmbO0pxgtKpCw4SY@linutronix.de Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent acb16b3 commit 6510ea9

File tree

2 files changed

+14
-21
lines changed

2 files changed

+14
-21
lines changed

include/linux/netdevice.h

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,10 @@ struct net_device_stats {
199199
* Try to fit them in a single cache line, for dev_get_stats() sake.
200200
*/
201201
struct net_device_core_stats {
202-
local_t rx_dropped;
203-
local_t tx_dropped;
204-
local_t rx_nohandler;
205-
} __aligned(4 * sizeof(local_t));
202+
unsigned long rx_dropped;
203+
unsigned long tx_dropped;
204+
unsigned long rx_nohandler;
205+
} __aligned(4 * sizeof(unsigned long));
206206

207207
#include <linux/cache.h>
208208
#include <linux/skbuff.h>
@@ -3843,30 +3843,27 @@ static __always_inline bool __is_skb_forwardable(const struct net_device *dev,
38433843
return false;
38443844
}
38453845

3846-
struct net_device_core_stats *netdev_core_stats_alloc(struct net_device *dev);
3846+
struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev);
38473847

3848-
static inline struct net_device_core_stats *dev_core_stats(struct net_device *dev)
3848+
static inline struct net_device_core_stats __percpu *dev_core_stats(struct net_device *dev)
38493849
{
38503850
/* This READ_ONCE() pairs with the write in netdev_core_stats_alloc() */
38513851
struct net_device_core_stats __percpu *p = READ_ONCE(dev->core_stats);
38523852

38533853
if (likely(p))
3854-
return this_cpu_ptr(p);
3854+
return p;
38553855

38563856
return netdev_core_stats_alloc(dev);
38573857
}
38583858

38593859
#define DEV_CORE_STATS_INC(FIELD) \
38603860
static inline void dev_core_stats_##FIELD##_inc(struct net_device *dev) \
38613861
{ \
3862-
struct net_device_core_stats *p; \
3862+
struct net_device_core_stats __percpu *p; \
38633863
\
3864-
preempt_disable(); \
38653864
p = dev_core_stats(dev); \
3866-
\
38673865
if (p) \
3868-
local_inc(&p->FIELD); \
3869-
preempt_enable(); \
3866+
this_cpu_inc(p->FIELD); \
38703867
}
38713868
DEV_CORE_STATS_INC(rx_dropped)
38723869
DEV_CORE_STATS_INC(tx_dropped)

net/core/dev.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10304,7 +10304,7 @@ void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
1030410304
}
1030510305
EXPORT_SYMBOL(netdev_stats_to_stats64);
1030610306

10307-
struct net_device_core_stats *netdev_core_stats_alloc(struct net_device *dev)
10307+
struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev)
1030810308
{
1030910309
struct net_device_core_stats __percpu *p;
1031010310

@@ -10315,11 +10315,7 @@ struct net_device_core_stats *netdev_core_stats_alloc(struct net_device *dev)
1031510315
free_percpu(p);
1031610316

1031710317
/* This READ_ONCE() pairs with the cmpxchg() above */
10318-
p = READ_ONCE(dev->core_stats);
10319-
if (!p)
10320-
return NULL;
10321-
10322-
return this_cpu_ptr(p);
10318+
return READ_ONCE(dev->core_stats);
1032310319
}
1032410320
EXPORT_SYMBOL(netdev_core_stats_alloc);
1032510321

@@ -10356,9 +10352,9 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
1035610352

1035710353
for_each_possible_cpu(i) {
1035810354
core_stats = per_cpu_ptr(p, i);
10359-
storage->rx_dropped += local_read(&core_stats->rx_dropped);
10360-
storage->tx_dropped += local_read(&core_stats->tx_dropped);
10361-
storage->rx_nohandler += local_read(&core_stats->rx_nohandler);
10355+
storage->rx_dropped += READ_ONCE(core_stats->rx_dropped);
10356+
storage->tx_dropped += READ_ONCE(core_stats->tx_dropped);
10357+
storage->rx_nohandler += READ_ONCE(core_stats->rx_nohandler);
1036210358
}
1036310359
}
1036410360
return storage;

0 commit comments

Comments
 (0)