bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 2/9] bpf: allow up to 13 arguments for tracepoints
       [not found] <20200409193543.18115-1-mathieu.desnoyers@efficios.com>
@ 2020-04-09 19:35 ` Mathieu Desnoyers
  2020-04-09 19:35 ` [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter Mathieu Desnoyers
  1 sibling, 0 replies; 4+ messages in thread
From: Mathieu Desnoyers @ 2020-04-09 19:35 UTC (permalink / raw)
  To: Thomas Gleixner, Will Deacon, akpm, Greg Kroah-Hartman,
	K . Prasad, Masami Hiramatsu, rostedt, Alexei Starovoitov
  Cc: linux-kernel, Mathieu Desnoyers, Daniel Borkmann, Ingo Molnar,
	bpf, netdev

Adding an argument to the writeback balance_dirty_pages event requires
to bump the bpf argument count limit from 12 to 13.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 include/linux/kernel.h       | 6 +++---
 include/linux/trace_events.h | 3 +++
 include/trace/bpf_probe.h    | 3 ++-
 kernel/trace/bpf_trace.c     | 8 +++++---
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 9b7a8d74a9d6..20203f762410 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -975,9 +975,9 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #define swap(a, b) \
 	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
 
-/* This counts to 12. Any more, it will return 13th argument. */
-#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
-#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+/* This counts to 13. Any more, it will return 14th argument. */
+#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _n, X...) _n
+#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
 
 #define __CONCAT(a, b) a ## b
 #define CONCATENATE(a, b) __CONCAT(a, b)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5c6943354049..1e3f4782d32f 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -776,6 +776,9 @@ void bpf_trace_run11(struct bpf_prog *prog, u64 arg1, u64 arg2,
 void bpf_trace_run12(struct bpf_prog *prog, u64 arg1, u64 arg2,
 		     u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
 		     u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12);
+void bpf_trace_run13(struct bpf_prog *prog, u64 arg1, u64 arg2,
+		     u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
+		     u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, u64 arg13);
 void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
 			       struct trace_event_call *call, u64 count,
 			       struct pt_regs *regs, struct hlist_head *head,
diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index 1ce3be63add1..1ca87b7a8230 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -52,7 +52,8 @@
 #define __CAST10(a,...) __CAST_TO_U64(a), __CAST9(__VA_ARGS__)
 #define __CAST11(a,...) __CAST_TO_U64(a), __CAST10(__VA_ARGS__)
 #define __CAST12(a,...) __CAST_TO_U64(a), __CAST11(__VA_ARGS__)
-/* tracepoints with more than 12 arguments will hit build error */
+#define __CAST13(a,...) __CAST_TO_U64(a), __CAST12(__VA_ARGS__)
+/* tracepoints with more than 13 arguments will hit build error */
 #define CAST_TO_U64(...) CONCATENATE(__CAST, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
 
 #undef DECLARE_EVENT_CLASS
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1796747a77..67354218b97f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1546,6 +1546,7 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
 #define REPEAT_10(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_9(FN, DL, __VA_ARGS__)
 #define REPEAT_11(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_10(FN, DL, __VA_ARGS__)
 #define REPEAT_12(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_11(FN, DL, __VA_ARGS__)
+#define REPEAT_13(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_12(FN, DL, __VA_ARGS__)
 #define REPEAT(X, FN, DL, ...)		REPEAT_##X(FN, DL, __VA_ARGS__)
 
 #define SARG(X)		u64 arg##X
@@ -1554,14 +1555,14 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
 #define __DL_COM	(,)
 #define __DL_SEM	(;)
 
-#define __SEQ_0_11	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
+#define __SEQ_0_12	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
 
 #define BPF_TRACE_DEFN_x(x)						\
 	void bpf_trace_run##x(struct bpf_prog *prog,			\
-			      REPEAT(x, SARG, __DL_COM, __SEQ_0_11))	\
+			      REPEAT(x, SARG, __DL_COM, __SEQ_0_12))	\
 	{								\
 		u64 args[x];						\
-		REPEAT(x, COPY, __DL_SEM, __SEQ_0_11);			\
+		REPEAT(x, COPY, __DL_SEM, __SEQ_0_12);			\
 		__bpf_trace_run(prog, args);				\
 	}								\
 	EXPORT_SYMBOL_GPL(bpf_trace_run##x)
@@ -1577,6 +1578,7 @@ BPF_TRACE_DEFN_x(9);
 BPF_TRACE_DEFN_x(10);
 BPF_TRACE_DEFN_x(11);
 BPF_TRACE_DEFN_x(12);
+BPF_TRACE_DEFN_x(13);
 
 static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter
       [not found] <20200409193543.18115-1-mathieu.desnoyers@efficios.com>
  2020-04-09 19:35 ` [RFC PATCH 2/9] bpf: allow up to 13 arguments for tracepoints Mathieu Desnoyers
@ 2020-04-09 19:35 ` Mathieu Desnoyers
  2020-04-09 22:33   ` Alexei Starovoitov
  1 sibling, 1 reply; 4+ messages in thread
From: Mathieu Desnoyers @ 2020-04-09 19:35 UTC (permalink / raw)
  To: Thomas Gleixner, Will Deacon, akpm, Greg Kroah-Hartman,
	K . Prasad, Masami Hiramatsu, rostedt, Alexei Starovoitov
  Cc: linux-kernel, Mathieu Desnoyers, Daniel Borkmann, Ingo Molnar,
	bpf, netdev

The symbol global_wb_domain is not GPL-exported to modules. In order
to allow kernel tracers implemented as GPL modules to access the
global writeback domain dirty limit, as used within the
global_dirty_state and balance_dirty_pages trace events, pass
a pointer to the global writeback domain as a new parameter for
those tracepoints.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 include/trace/events/writeback.h | 17 ++++++++++-------
 mm/page-writeback.c              |  9 +++++----
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index d94def25e4dc..8fd774108c9c 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -531,11 +531,13 @@ TRACE_EVENT(writeback_queue_io,
 
 TRACE_EVENT(global_dirty_state,
 
-	TP_PROTO(unsigned long background_thresh,
+	TP_PROTO(struct wb_domain *domain,
+		 unsigned long background_thresh,
 		 unsigned long dirty_thresh
 	),
 
-	TP_ARGS(background_thresh,
+	TP_ARGS(domain,
+		background_thresh,
 		dirty_thresh
 	),
 
@@ -558,7 +560,7 @@ TRACE_EVENT(global_dirty_state,
 		__entry->nr_written	= global_node_page_state(NR_WRITTEN);
 		__entry->background_thresh = background_thresh;
 		__entry->dirty_thresh	= dirty_thresh;
-		__entry->dirty_limit	= global_wb_domain.dirty_limit;
+		__entry->dirty_limit	= domain->dirty_limit;
 	),
 
 	TP_printk("dirty=%lu writeback=%lu unstable=%lu "
@@ -625,7 +627,8 @@ TRACE_EVENT(bdi_dirty_ratelimit,
 
 TRACE_EVENT(balance_dirty_pages,
 
-	TP_PROTO(struct bdi_writeback *wb,
+	TP_PROTO(struct wb_domain *domain,
+		 struct bdi_writeback *wb,
 		 unsigned long thresh,
 		 unsigned long bg_thresh,
 		 unsigned long dirty,
@@ -638,7 +641,7 @@ TRACE_EVENT(balance_dirty_pages,
 		 long pause,
 		 unsigned long start_time),
 
-	TP_ARGS(wb, thresh, bg_thresh, dirty, bdi_thresh, bdi_dirty,
+	TP_ARGS(domain, wb, thresh, bg_thresh, dirty, bdi_thresh, bdi_dirty,
 		dirty_ratelimit, task_ratelimit,
 		dirtied, period, pause, start_time),
 
@@ -664,8 +667,8 @@ TRACE_EVENT(balance_dirty_pages,
 		unsigned long freerun = (thresh + bg_thresh) / 2;
 		strscpy_pad(__entry->bdi, bdi_dev_name(wb->bdi), 32);
 
-		__entry->limit		= global_wb_domain.dirty_limit;
-		__entry->setpoint	= (global_wb_domain.dirty_limit +
+		__entry->limit		= domain->dirty_limit;
+		__entry->setpoint	= (domain->dirty_limit +
 						freerun) / 2;
 		__entry->dirty		= dirty;
 		__entry->bdi_setpoint	= __entry->setpoint *
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 7326b54ab728..c76aae305360 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -443,9 +443,8 @@ static void domain_dirty_limits(struct dirty_throttle_control *dtc)
 	dtc->thresh = thresh;
 	dtc->bg_thresh = bg_thresh;
 
-	/* we should eventually report the domain in the TP */
 	if (!gdtc)
-		trace_global_dirty_state(bg_thresh, thresh);
+		trace_global_dirty_state(&global_wb_domain, bg_thresh, thresh);
 }
 
 /**
@@ -1736,7 +1735,8 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 		 * do a reset, as it may be a light dirtier.
 		 */
 		if (pause < min_pause) {
-			trace_balance_dirty_pages(wb,
+			trace_balance_dirty_pages(&global_wb_domain,
+						  wb,
 						  sdtc->thresh,
 						  sdtc->bg_thresh,
 						  sdtc->dirty,
@@ -1765,7 +1765,8 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 		}
 
 pause:
-		trace_balance_dirty_pages(wb,
+		trace_balance_dirty_pages(&global_wb_domain,
+					  wb,
 					  sdtc->thresh,
 					  sdtc->bg_thresh,
 					  sdtc->dirty,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter
  2020-04-09 19:35 ` [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter Mathieu Desnoyers
@ 2020-04-09 22:33   ` Alexei Starovoitov
  2020-04-10 11:54     ` Mathieu Desnoyers
  0 siblings, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2020-04-09 22:33 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Thomas Gleixner, Will Deacon, akpm, Greg Kroah-Hartman,
	K . Prasad, Masami Hiramatsu, rostedt, Alexei Starovoitov,
	linux-kernel, Daniel Borkmann, Ingo Molnar, bpf, netdev

On Thu, Apr 09, 2020 at 03:35:37PM -0400, Mathieu Desnoyers wrote:
>  		if (pause < min_pause) {
> -			trace_balance_dirty_pages(wb,
> +			trace_balance_dirty_pages(&global_wb_domain,
> +						  wb,
>  						  sdtc->thresh,
>  						  sdtc->bg_thresh,
>  						  sdtc->dirty,

argh. 13 arguments to single function ?!
Currently the call site looks like:
                        trace_balance_dirty_pages(wb,
                                                  sdtc->thresh,
                                                  sdtc->bg_thresh,
                                                  sdtc->dirty,
                                                  sdtc->wb_thresh,
                                                  sdtc->wb_dirty,
                                                  dirty_ratelimit,
                                                  task_ratelimit,
                                                  pages_dirtied,
                                                  period,
                                                  min(pause, 0L),
                                                  start_time);
Just pass sdtc as a pointer instead.
Then another wb argument will be fine.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter
  2020-04-09 22:33   ` Alexei Starovoitov
@ 2020-04-10 11:54     ` Mathieu Desnoyers
  0 siblings, 0 replies; 4+ messages in thread
From: Mathieu Desnoyers @ 2020-04-10 11:54 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Thomas Gleixner, Will Deacon, Andrew Morton, Greg Kroah-Hartman,
	K, Masami Hiramatsu, rostedt, Alexei Starovoitov, linux-kernel,
	Daniel Borkmann, Ingo Molnar, bpf, netdev

----- On Apr 9, 2020, at 6:33 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:

> On Thu, Apr 09, 2020 at 03:35:37PM -0400, Mathieu Desnoyers wrote:
>>  		if (pause < min_pause) {
>> -			trace_balance_dirty_pages(wb,
>> +			trace_balance_dirty_pages(&global_wb_domain,
>> +						  wb,
>>  						  sdtc->thresh,
>>  						  sdtc->bg_thresh,
>>  						  sdtc->dirty,
> 
> argh. 13 arguments to single function ?!
> Currently the call site looks like:
>                        trace_balance_dirty_pages(wb,
>                                                  sdtc->thresh,
>                                                  sdtc->bg_thresh,
>                                                  sdtc->dirty,
>                                                  sdtc->wb_thresh,
>                                                  sdtc->wb_dirty,
>                                                  dirty_ratelimit,
>                                                  task_ratelimit,
>                                                  pages_dirtied,
>                                                  period,
>                                                  min(pause, 0L),
>                                                  start_time);
> Just pass sdtc as a pointer instead.
> Then another wb argument will be fine.

Good point! In order to do that I would need to move the declaration
of struct dirty_throttle_control from mm/page-writeback.c to
include/linux/writeback.h so it can be used from the tracepoint.

Seems fair ?

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-04-10 11:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200409193543.18115-1-mathieu.desnoyers@efficios.com>
2020-04-09 19:35 ` [RFC PATCH 2/9] bpf: allow up to 13 arguments for tracepoints Mathieu Desnoyers
2020-04-09 19:35 ` [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter Mathieu Desnoyers
2020-04-09 22:33   ` Alexei Starovoitov
2020-04-10 11:54     ` Mathieu Desnoyers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).