* [PATCH net-next v3 1/2] once: implement DO_ONCE_LITE for non-fast-path "do once" functionality
2021-06-28 13:50 [PATCH net-next v3 0/2] net: update netdev_rx_csum_fault() print dump only once Tanner Love
@ 2021-06-28 13:50 ` Tanner Love
2021-06-28 15:14 ` Steven Rostedt
2021-06-28 13:50 ` [PATCH net-next v3 2/2] net: update netdev_rx_csum_fault() print dump only once Tanner Love
2021-06-28 23:10 ` [PATCH net-next v3 0/2] " patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Tanner Love @ 2021-06-28 13:50 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-arch, David S . Miller, Eric Dumazet,
Mahesh Bandewar, Darrick J . Wong, Arnd Bergmann, Petr Mladek,
Sergey Senozhatsky, Steven Rostedt, John Ogness, Ingo Molnar,
Jakub Kicinski, Andrii Nakryiko, Antoine Tenart,
Alexander Lobakin, Wei Wang, Taehee Yoo, Yunsheng Lin,
Willem de Bruijn, Tanner Love
From: Tanner Love <tannerlove@google.com>
Certain uses of "do once" functionality reside outside of fast path,
and so do not require jump label patching via static keys, making
existing DO_ONCE undesirable in such cases.
Replace uses of __section(".data.once") with DO_ONCE_LITE(_IF)?
This patch changes the return values of xfs_printk_once, printk_once,
and printk_deferred_once. Before, they returned whether the print was
performed, but now, they always return true. This is okay because the
return values of the following macros are entirely ignored throughout
the kernel:
- xfs_printk_once
- xfs_warn_once
- xfs_notice_once
- xfs_info_once
- printk_once
- pr_emerg_once
- pr_alert_once
- pr_crit_once
- pr_err_once
- pr_warn_once
- pr_notice_once
- pr_info_once
- pr_devel_once
- pr_debug_once
- printk_deferred_once
- orc_warn
Changes
v3:
- Expand commit message to explain why changing return values of
xfs_printk_once, printk_once, printk_deferred_once is benign
v2:
- Fix i386 build warnings
Signed-off-by: Tanner Love <tannerlove@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
---
fs/xfs/xfs_message.h | 13 +++----------
include/asm-generic/bug.h | 37 +++++++------------------------------
include/linux/once_lite.h | 24 ++++++++++++++++++++++++
include/linux/printk.h | 23 +++--------------------
kernel/trace/trace.h | 13 +++----------
5 files changed, 40 insertions(+), 70 deletions(-)
create mode 100644 include/linux/once_lite.h
diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
index 7ec1a9207517..bb9860ec9a93 100644
--- a/fs/xfs/xfs_message.h
+++ b/fs/xfs/xfs_message.h
@@ -2,6 +2,8 @@
#ifndef __XFS_MESSAGE_H
#define __XFS_MESSAGE_H 1
+#include <linux/once_lite.h>
+
struct xfs_mount;
extern __printf(2, 3)
@@ -41,16 +43,7 @@ do { \
} while (0)
#define xfs_printk_once(func, dev, fmt, ...) \
-({ \
- static bool __section(".data.once") __print_once; \
- bool __ret_print_once = !__print_once; \
- \
- if (!__print_once) { \
- __print_once = true; \
- func(dev, fmt, ##__VA_ARGS__); \
- } \
- unlikely(__ret_print_once); \
-})
+ DO_ONCE_LITE(func, dev, fmt, ##__VA_ARGS__)
#define xfs_emerg_ratelimited(dev, fmt, ...) \
xfs_printk_ratelimited(xfs_emerg, dev, fmt, ##__VA_ARGS__)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index b402494883b6..bafc51f483c4 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -4,6 +4,7 @@
#include <linux/compiler.h>
#include <linux/instrumentation.h>
+#include <linux/once_lite.h>
#define CUT_HERE "------------[ cut here ]------------\n"
@@ -140,39 +141,15 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
})
#ifndef WARN_ON_ONCE
-#define WARN_ON_ONCE(condition) ({ \
- static bool __section(".data.once") __warned; \
- int __ret_warn_once = !!(condition); \
- \
- if (unlikely(__ret_warn_once && !__warned)) { \
- __warned = true; \
- WARN_ON(1); \
- } \
- unlikely(__ret_warn_once); \
-})
+#define WARN_ON_ONCE(condition) \
+ DO_ONCE_LITE_IF(condition, WARN_ON, 1)
#endif
-#define WARN_ONCE(condition, format...) ({ \
- static bool __section(".data.once") __warned; \
- int __ret_warn_once = !!(condition); \
- \
- if (unlikely(__ret_warn_once && !__warned)) { \
- __warned = true; \
- WARN(1, format); \
- } \
- unlikely(__ret_warn_once); \
-})
+#define WARN_ONCE(condition, format...) \
+ DO_ONCE_LITE_IF(condition, WARN, 1, format)
-#define WARN_TAINT_ONCE(condition, taint, format...) ({ \
- static bool __section(".data.once") __warned; \
- int __ret_warn_once = !!(condition); \
- \
- if (unlikely(__ret_warn_once && !__warned)) { \
- __warned = true; \
- WARN_TAINT(1, taint, format); \
- } \
- unlikely(__ret_warn_once); \
-})
+#define WARN_TAINT_ONCE(condition, taint, format...) \
+ DO_ONCE_LITE_IF(condition, WARN_TAINT, 1, taint, format)
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
diff --git a/include/linux/once_lite.h b/include/linux/once_lite.h
new file mode 100644
index 000000000000..861e606b820f
--- /dev/null
+++ b/include/linux/once_lite.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ONCE_LITE_H
+#define _LINUX_ONCE_LITE_H
+
+#include <linux/types.h>
+
+/* Call a function once. Similar to DO_ONCE(), but does not use jump label
+ * patching via static keys.
+ */
+#define DO_ONCE_LITE(func, ...) \
+ DO_ONCE_LITE_IF(true, func, ##__VA_ARGS__)
+#define DO_ONCE_LITE_IF(condition, func, ...) \
+ ({ \
+ static bool __section(".data.once") __already_done; \
+ bool __ret_do_once = !!(condition); \
+ \
+ if (unlikely(__ret_do_once && !__already_done)) { \
+ __already_done = true; \
+ func(__VA_ARGS__); \
+ } \
+ unlikely(__ret_do_once); \
+ })
+
+#endif /* _LINUX_ONCE_LITE_H */
diff --git a/include/linux/printk.h b/include/linux/printk.h
index fe7eb2351610..885379a1c9a1 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -8,6 +8,7 @@
#include <linux/linkage.h>
#include <linux/cache.h>
#include <linux/ratelimit_types.h>
+#include <linux/once_lite.h>
extern const char linux_banner[];
extern const char linux_proc_banner[];
@@ -436,27 +437,9 @@ extern int kptr_restrict;
#ifdef CONFIG_PRINTK
#define printk_once(fmt, ...) \
-({ \
- static bool __section(".data.once") __print_once; \
- bool __ret_print_once = !__print_once; \
- \
- if (!__print_once) { \
- __print_once = true; \
- printk(fmt, ##__VA_ARGS__); \
- } \
- unlikely(__ret_print_once); \
-})
+ DO_ONCE_LITE(printk, fmt, ##__VA_ARGS__)
#define printk_deferred_once(fmt, ...) \
-({ \
- static bool __section(".data.once") __print_once; \
- bool __ret_print_once = !__print_once; \
- \
- if (!__print_once) { \
- __print_once = true; \
- printk_deferred(fmt, ##__VA_ARGS__); \
- } \
- unlikely(__ret_print_once); \
-})
+ DO_ONCE_LITE(printk_deferred, fmt, ##__VA_ARGS__)
#else
#define printk_once(fmt, ...) \
no_printk(fmt, ##__VA_ARGS__)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index cd80d046c7a5..d5d8c088a55d 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -20,6 +20,7 @@
#include <linux/irq_work.h>
#include <linux/workqueue.h>
#include <linux/ctype.h>
+#include <linux/once_lite.h>
#ifdef CONFIG_FTRACE_SYSCALLS
#include <asm/unistd.h> /* For NR_SYSCALLS */
@@ -99,16 +100,8 @@ enum trace_type {
#include "trace_entries.h"
/* Use this for memory failure errors */
-#define MEM_FAIL(condition, fmt, ...) ({ \
- static bool __section(".data.once") __warned; \
- int __ret_warn_once = !!(condition); \
- \
- if (unlikely(__ret_warn_once && !__warned)) { \
- __warned = true; \
- pr_err("ERROR: " fmt, ##__VA_ARGS__); \
- } \
- unlikely(__ret_warn_once); \
-})
+#define MEM_FAIL(condition, fmt, ...) \
+ DO_ONCE_LITE_IF(condition, pr_err, "ERROR: " fmt, ##__VA_ARGS__)
/*
* syscalls are special, and need special handling, this is why
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next v3 2/2] net: update netdev_rx_csum_fault() print dump only once
2021-06-28 13:50 [PATCH net-next v3 0/2] net: update netdev_rx_csum_fault() print dump only once Tanner Love
2021-06-28 13:50 ` [PATCH net-next v3 1/2] once: implement DO_ONCE_LITE for non-fast-path "do once" functionality Tanner Love
@ 2021-06-28 13:50 ` Tanner Love
2021-06-28 23:10 ` [PATCH net-next v3 0/2] " patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: Tanner Love @ 2021-06-28 13:50 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, linux-arch, David S . Miller, Eric Dumazet,
Mahesh Bandewar, Darrick J . Wong, Arnd Bergmann, Petr Mladek,
Sergey Senozhatsky, Steven Rostedt, John Ogness, Ingo Molnar,
Jakub Kicinski, Andrii Nakryiko, Antoine Tenart,
Alexander Lobakin, Wei Wang, Taehee Yoo, Yunsheng Lin,
Willem de Bruijn, Tanner Love
From: Tanner Love <tannerlove@google.com>
Printing this stack dump multiple times does not provide additional
useful information, and consumes time in the data path. Printing once
is sufficient.
Changes
v2: Format indentation properly
Signed-off-by: Tanner Love <tannerlove@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
---
net/core/dev.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 991d09b67bd9..d609366da95c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -148,6 +148,7 @@
#include <net/devlink.h>
#include <linux/pm_runtime.h>
#include <linux/prandom.h>
+#include <linux/once_lite.h>
#include "net-sysfs.h"
@@ -3487,13 +3488,16 @@ EXPORT_SYMBOL(__skb_gso_segment);
/* Take action when hardware reception checksum errors are detected. */
#ifdef CONFIG_BUG
+static void do_netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
+{
+ pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
+ skb_dump(KERN_ERR, skb, true);
+ dump_stack();
+}
+
void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
{
- if (net_ratelimit()) {
- pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
- skb_dump(KERN_ERR, skb, true);
- dump_stack();
- }
+ DO_ONCE_LITE(do_netdev_rx_csum_fault, dev, skb);
}
EXPORT_SYMBOL(netdev_rx_csum_fault);
#endif
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next v3 0/2] net: update netdev_rx_csum_fault() print dump only once
2021-06-28 13:50 [PATCH net-next v3 0/2] net: update netdev_rx_csum_fault() print dump only once Tanner Love
2021-06-28 13:50 ` [PATCH net-next v3 1/2] once: implement DO_ONCE_LITE for non-fast-path "do once" functionality Tanner Love
2021-06-28 13:50 ` [PATCH net-next v3 2/2] net: update netdev_rx_csum_fault() print dump only once Tanner Love
@ 2021-06-28 23:10 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-28 23:10 UTC (permalink / raw)
To: Tanner Love
Cc: netdev, linux-kernel, linux-arch, davem, edumazet, maheshb,
djwong, arnd, pmladek, senozhatsky, rostedt, john.ogness, mingo,
kuba, andriin, atenart, alobakin, weiwan, ap420073, linyunsheng,
willemb, tannerlove
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Mon, 28 Jun 2021 09:50:05 -0400 you wrote:
> From: Tanner Love <tannerlove@google.com>
>
> First patch implements DO_ONCE_LITE to abstract uses of the ".data.once"
> trick. It is defined in its own, new header file -- rather than
> alongside the existing DO_ONCE in include/linux/once.h -- because
> include/linux/once.h includes include/linux/jump_label.h, and this
> causes the build to break for some architectures if
> include/linux/once.h is included in include/linux/printk.h or
> include/asm-generic/bug.h.
>
> [...]
Here is the summary with links:
- [net-next,v3,1/2] once: implement DO_ONCE_LITE for non-fast-path "do once" functionality
https://git.kernel.org/netdev/net-next/c/a358f40600b3
- [net-next,v3,2/2] net: update netdev_rx_csum_fault() print dump only once
https://git.kernel.org/netdev/net-next/c/127d7355abb3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread