Messages in this thread | | | Date | Tue, 19 Sep 2023 21:42:23 +0800 | Subject | Re: [PATCH net-next v4] net/core: Introduce netdev_core_stats_inc() for trace | From | Yajun Deng <> |
| |
On 2023/9/19 17:10, Paolo Abeni wrote: > On Mon, 2023-09-18 at 10:40 +0800, Yajun Deng wrote: >> Although there is a kfree_skb_reason() helper function that can be used to >> find the reason why this skb is dropped, but most callers didn't increase >> one of rx_dropped, tx_dropped, rx_nohandler and rx_otherhost_dropped. >> >> For the users, people are more concerned about why the dropped in ip >> is increasing. >> >> Introduce netdev_core_stats_inc() for trace. Also, move dev_core_stats() >> and netdev_core_stats_alloc() to dev.c, as they are not called externally. >> >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> >> Suggested-by: Alexander Lobakin <aleksander.lobakin@intel.com> >> --- >> v4: Introduce netdev_core_stats_inc() instead of export dev_core_stats_*_inc() >> v3: __cold should be added to the netdev_core_stats_alloc(). >> v2: use __cold instead of inline in dev_core_stats(). >> v1: https://lore.kernel.org/netdev/20230911082016.3694700-1-yajun.deng@linux.dev/ >> --- >> include/linux/netdevice.h | 21 ++++----------------- >> net/core/dev.c | 26 ++++++++++++++++++++++++-- >> 2 files changed, 28 insertions(+), 19 deletions(-) >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index 0896aaa91dd7..cddd4873b5b0 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -3980,32 +3980,19 @@ static __always_inline bool __is_skb_forwardable(const struct net_device *dev, >> return false; >> } >> >> -struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev); >> - >> -static inline struct net_device_core_stats __percpu *dev_core_stats(struct net_device *dev) >> -{ >> - /* This READ_ONCE() pairs with the write in netdev_core_stats_alloc() */ >> - struct net_device_core_stats __percpu *p = READ_ONCE(dev->core_stats); >> - >> - if (likely(p)) >> - return p; >> - >> - return netdev_core_stats_alloc(dev); >> -} >> +void netdev_core_stats_inc(struct net_device *dev, u32 offset); >> >> #define DEV_CORE_STATS_INC(FIELD) \ >> static inline void dev_core_stats_##FIELD##_inc(struct net_device *dev) \ >> { \ >> - struct net_device_core_stats __percpu *p; \ >> - \ >> - p = dev_core_stats(dev); \ >> - if (p) \ >> - this_cpu_inc(p->FIELD); \ >> + netdev_core_stats_inc(dev, \ >> + offsetof(struct net_device_core_stats, FIELD)); \ >> } >> DEV_CORE_STATS_INC(rx_dropped) >> DEV_CORE_STATS_INC(tx_dropped) >> DEV_CORE_STATS_INC(rx_nohandler) >> DEV_CORE_STATS_INC(rx_otherhost_dropped) >> +#undef DEV_CORE_STATS_INC >> >> static __always_inline int ____dev_forward_skb(struct net_device *dev, >> struct sk_buff *skb, >> diff --git a/net/core/dev.c b/net/core/dev.c >> index ccff2b6ef958..f4cccdf05450 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -10475,7 +10475,8 @@ void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64, >> } >> EXPORT_SYMBOL(netdev_stats_to_stats64); >> >> -struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev) >> +static __cold struct net_device_core_stats __percpu *netdev_core_stats_alloc( >> + struct net_device *dev) >> { >> struct net_device_core_stats __percpu *p; >> >> @@ -10488,7 +10489,28 @@ struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device >> /* This READ_ONCE() pairs with the cmpxchg() above */ >> return READ_ONCE(dev->core_stats); >> } >> -EXPORT_SYMBOL(netdev_core_stats_alloc); >> + >> +static inline struct net_device_core_stats __percpu *netdev_core_stats( >> + struct net_device *dev) >> +{ >> + /* This READ_ONCE() pairs with the write in netdev_core_stats_alloc() */ >> + struct net_device_core_stats __percpu *p = READ_ONCE(dev->core_stats); >> + >> + if (likely(p)) >> + return p; >> + >> + return netdev_core_stats_alloc(dev); >> +} >> + >> +void netdev_core_stats_inc(struct net_device *dev, u32 offset) >> +{ >> + struct net_device_core_stats __percpu *p; >> + >> + p = netdev_core_stats(dev); >> + if (p) >> + this_cpu_inc(*(unsigned long *)((void *)p + offset)); > The above is causing a lot of compile warning, as it's discarding the > (required) __percpu annotation. > > You need to first access the per cpu pointer and then reach for the > relevant offset. Indeed, thanks. > > Cheers, > > Paolo >
| |