linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf
@ 2022-12-16  9:03 Linyu Yuan
  2022-12-16  9:03 ` [PATCH v4 2/2] trace: acquire buffer from temparary trace sequence Linyu Yuan
  2023-01-24 19:46 ` [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Steven Rostedt
  0 siblings, 2 replies; 5+ messages in thread
From: Linyu Yuan @ 2022-12-16  9:03 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu; +Cc: linux-kernel, bpf, Linyu Yuan

when experiment change for trace operation, the files under
include/trace/stages/ will be changed, perf/bpf trace may compile fail.

add new file stage8_perf_call.h and stage9_bpf_call.h under
include/trace/stages/, it will make sure no missing change when expriment.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v4: fix build isse due to file rename
v3: consider perf as stage 8, bpf as stage 9, file name purpose only.
v2: not exist

 include/trace/bpf_probe.h               | 51 +------------------------------
 include/trace/perf.h                    | 52 +-------------------------------
 include/trace/stages/stage8_perf_call.h | 53 +++++++++++++++++++++++++++++++++
 include/trace/stages/stage9_bpf_call.h  | 52 ++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 101 deletions(-)
 create mode 100644 include/trace/stages/stage8_perf_call.h
 create mode 100644 include/trace/stages/stage9_bpf_call.h

diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index 155c495..dfa39f5 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -4,56 +4,7 @@
 
 #ifdef CONFIG_BPF_EVENTS
 
-#undef __entry
-#define __entry entry
-
-#undef __get_dynamic_array
-#define __get_dynamic_array(field)	\
-		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
-
-#undef __get_dynamic_array_len
-#define __get_dynamic_array_len(field)	\
-		((__entry->__data_loc_##field >> 16) & 0xffff)
-
-#undef __get_str
-#define __get_str(field) ((char *)__get_dynamic_array(field))
-
-#undef __get_bitmask
-#define __get_bitmask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_cpumask
-#define __get_cpumask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_sockaddr
-#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
-
-#undef __get_rel_dynamic_array
-#define __get_rel_dynamic_array(field)	\
-		((void *)(&__entry->__rel_loc_##field) +	\
-		 sizeof(__entry->__rel_loc_##field) +		\
-		 (__entry->__rel_loc_##field & 0xffff))
-
-#undef __get_rel_dynamic_array_len
-#define __get_rel_dynamic_array_len(field)	\
-		((__entry->__rel_loc_##field >> 16) & 0xffff)
-
-#undef __get_rel_str
-#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
-
-#undef __get_rel_bitmask
-#define __get_rel_bitmask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_cpumask
-#define __get_rel_cpumask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_sockaddr
-#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
-
-#undef __perf_count
-#define __perf_count(c)	(c)
-
-#undef __perf_task
-#define __perf_task(t)	(t)
+#include "stages/stage9_bpf_call.h"
 
 /* cast any integer, pointer, or small struct to u64 */
 #define UINTTYPE(size) \
diff --git a/include/trace/perf.h b/include/trace/perf.h
index 8f3bf1e..57e1661 100644
--- a/include/trace/perf.h
+++ b/include/trace/perf.h
@@ -4,57 +4,7 @@
 
 #ifdef CONFIG_PERF_EVENTS
 
-#undef __entry
-#define __entry entry
-
-#undef __get_dynamic_array
-#define __get_dynamic_array(field)	\
-		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
-
-#undef __get_dynamic_array_len
-#define __get_dynamic_array_len(field)	\
-		((__entry->__data_loc_##field >> 16) & 0xffff)
-
-#undef __get_str
-#define __get_str(field) ((char *)__get_dynamic_array(field))
-
-#undef __get_bitmask
-#define __get_bitmask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_cpumask
-#define __get_cpumask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_sockaddr
-#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
-
-#undef __get_rel_dynamic_array
-#define __get_rel_dynamic_array(field)	\
-		((void *)__entry +					\
-		 offsetof(typeof(*__entry), __rel_loc_##field) +	\
-		 sizeof(__entry->__rel_loc_##field) +			\
-		 (__entry->__rel_loc_##field & 0xffff))
-
-#undef __get_rel_dynamic_array_len
-#define __get_rel_dynamic_array_len(field)	\
-		((__entry->__rel_loc_##field >> 16) & 0xffff)
-
-#undef __get_rel_str
-#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
-
-#undef __get_rel_bitmask
-#define __get_rel_bitmask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_cpumask
-#define __get_rel_cpumask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_sockaddr
-#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
-
-#undef __perf_count
-#define __perf_count(c)	(__count = (c))
-
-#undef __perf_task
-#define __perf_task(t)	(__task = (t))
+#include "stages/stage8_perf_call.h"
 
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
diff --git a/include/trace/stages/stage8_perf_call.h b/include/trace/stages/stage8_perf_call.h
new file mode 100644
index 0000000..54c8045
--- /dev/null
+++ b/include/trace/stages/stage8_perf_call.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#undef __entry
+#define __entry entry
+
+#undef __get_dynamic_array
+#define __get_dynamic_array(field)	\
+		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
+
+#undef __get_dynamic_array_len
+#define __get_dynamic_array_len(field)	\
+		((__entry->__data_loc_##field >> 16) & 0xffff)
+
+#undef __get_str
+#define __get_str(field) ((char *)__get_dynamic_array(field))
+
+#undef __get_bitmask
+#define __get_bitmask(field) ((char *)__get_dynamic_array(field))
+
+#undef __get_cpumask
+#define __get_cpumask(field) ((char *)__get_dynamic_array(field))
+
+#undef __get_sockaddr
+#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
+
+#undef __get_rel_dynamic_array
+#define __get_rel_dynamic_array(field)	\
+		((void *)__entry +					\
+		 offsetof(typeof(*__entry), __rel_loc_##field) +	\
+		 sizeof(__entry->__rel_loc_##field) +			\
+		 (__entry->__rel_loc_##field & 0xffff))
+
+#undef __get_rel_dynamic_array_len
+#define __get_rel_dynamic_array_len(field)	\
+		((__entry->__rel_loc_##field >> 16) & 0xffff)
+
+#undef __get_rel_str
+#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
+
+#undef __get_rel_bitmask
+#define __get_rel_bitmask(field) ((char *)__get_rel_dynamic_array(field))
+
+#undef __get_rel_cpumask
+#define __get_rel_cpumask(field) ((char *)__get_rel_dynamic_array(field))
+
+#undef __get_rel_sockaddr
+#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
+
+#undef __perf_count
+#define __perf_count(c)	(__count = (c))
+
+#undef __perf_task
+#define __perf_task(t)	(__task = (t))
diff --git a/include/trace/stages/stage9_bpf_call.h b/include/trace/stages/stage9_bpf_call.h
new file mode 100644
index 0000000..6dbcbdf
--- /dev/null
+++ b/include/trace/stages/stage9_bpf_call.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#undef __entry
+#define __entry entry
+
+#undef __get_dynamic_array
+#define __get_dynamic_array(field)	\
+		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
+
+#undef __get_dynamic_array_len
+#define __get_dynamic_array_len(field)	\
+		((__entry->__data_loc_##field >> 16) & 0xffff)
+
+#undef __get_str
+#define __get_str(field) ((char *)__get_dynamic_array(field))
+
+#undef __get_bitmask
+#define __get_bitmask(field) ((char *)__get_dynamic_array(field))
+
+#undef __get_cpumask
+#define __get_cpumask(field) ((char *)__get_dynamic_array(field))
+
+#undef __get_sockaddr
+#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
+
+#undef __get_rel_dynamic_array
+#define __get_rel_dynamic_array(field)	\
+		((void *)(&__entry->__rel_loc_##field) +	\
+		 sizeof(__entry->__rel_loc_##field) +		\
+		 (__entry->__rel_loc_##field & 0xffff))
+
+#undef __get_rel_dynamic_array_len
+#define __get_rel_dynamic_array_len(field)	\
+		((__entry->__rel_loc_##field >> 16) & 0xffff)
+
+#undef __get_rel_str
+#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
+
+#undef __get_rel_bitmask
+#define __get_rel_bitmask(field) ((char *)__get_rel_dynamic_array(field))
+
+#undef __get_rel_cpumask
+#define __get_rel_cpumask(field) ((char *)__get_rel_dynamic_array(field))
+
+#undef __get_rel_sockaddr
+#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
+
+#undef __perf_count
+#define __perf_count(c)	(c)
+
+#undef __perf_task
+#define __perf_task(t)	(t)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH v4 2/2] trace: acquire buffer from temparary trace sequence
  2022-12-16  9:03 [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Linyu Yuan
@ 2022-12-16  9:03 ` Linyu Yuan
  2023-01-24 21:44   ` Steven Rostedt
  2023-01-24 19:46 ` [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Steven Rostedt
  1 sibling, 1 reply; 5+ messages in thread
From: Linyu Yuan @ 2022-12-16  9:03 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu; +Cc: linux-kernel, bpf, Linyu Yuan

there is one dwc3 trace event declare as below,
DECLARE_EVENT_CLASS(dwc3_log_event,
	TP_PROTO(u32 event, struct dwc3 *dwc),
	TP_ARGS(event, dwc),
	TP_STRUCT__entry(
		__field(u32, event)
		__field(u32, ep0state)
		__dynamic_array(char, str, DWC3_MSG_MAX)
	),
	TP_fast_assign(
		__entry->event = event;
		__entry->ep0state = dwc->ep0state;
	),
	TP_printk("event (%08x): %s", __entry->event,
			dwc3_decode_event(__get_str(str), DWC3_MSG_MAX,
				__entry->event, __entry->ep0state))
);
the problem is when trace function called, it will allocate up to
DWC3_MSG_MAX bytes from trace event buffer, but never fill the buffer
during fast assignment, it only fill the buffer when output function are
called, so this means if output function are not called, the buffer will
never used.

add __get_buf(len) which acquiree buffer from iter->tmp_seq when trace
output function called, it allow user write string to acquired buffer.

the mentioned dwc3 trace event will changed as below,
DECLARE_EVENT_CLASS(dwc3_log_event,
	TP_PROTO(u32 event, struct dwc3 *dwc),
	TP_ARGS(event, dwc),
	TP_STRUCT__entry(
		__field(u32, event)
		__field(u32, ep0state)
	),
	TP_fast_assign(
		__entry->event = event;
		__entry->ep0state = dwc->ep0state;
	),
	TP_printk("event (%08x): %s", __entry->event,
		dwc3_decode_event(__get_buf(DWC3_MSG_MAX), DWC3_MSG_MAX,
				__entry->event, __entry->ep0state))
);.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v4: no change
v3: fix comment from maintainer in v2

 include/linux/trace_seq.h                  |  6 ++++++
 include/trace/stages/stage3_trace_output.h |  8 ++++++++
 include/trace/stages/stage7_class_define.h |  1 +
 kernel/trace/trace_seq.c                   | 22 ++++++++++++++++++++++
 4 files changed, 37 insertions(+)

diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
index 5a2c650..4a3bbab 100644
--- a/include/linux/trace_seq.h
+++ b/include/linux/trace_seq.h
@@ -95,6 +95,7 @@ extern void trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp,
 extern int trace_seq_hex_dump(struct trace_seq *s, const char *prefix_str,
 			      int prefix_type, int rowsize, int groupsize,
 			      const void *buf, size_t len, bool ascii);
+char *trace_seq_acquire(struct trace_seq *s, size_t len);
 
 #else /* CONFIG_TRACING */
 static inline void trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
@@ -138,6 +139,11 @@ static inline int trace_seq_path(struct trace_seq *s, const struct path *path)
 {
 	return 0;
 }
+
+static inline char *trace_seq_acquire(struct trace_seq *s, size_t len)
+{
+	return NULL;
+}
 #endif /* CONFIG_TRACING */
 
 #endif /* _LINUX_TRACE_SEQ_H */
diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index 66374df..f60c453 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -139,3 +139,11 @@
 		u64 ____val = (u64)(value);		\
 		(u32) do_div(____val, NSEC_PER_SEC);	\
 	})
+
+/* Macros with flow control statements should be avoided */
+#undef __get_buf
+#define __get_buf(len)							\
+	({								\
+		WARN_ON_ONCE(seq_buf_buffer_left(&p->seq) < (len));	\
+		trace_seq_acquire(p, (len));				\
+	})
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index 8795429..bcb960d 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -23,6 +23,7 @@
 #undef __get_rel_sockaddr
 #undef __print_array
 #undef __print_hex_dump
+#undef __get_buf
 
 /*
  * The below is not executed in the kernel. It is only what is
diff --git a/kernel/trace/trace_seq.c b/kernel/trace/trace_seq.c
index 9c90b3a..c900b7c 100644
--- a/kernel/trace/trace_seq.c
+++ b/kernel/trace/trace_seq.c
@@ -403,3 +403,25 @@ int trace_seq_hex_dump(struct trace_seq *s, const char *prefix_str,
 	return 1;
 }
 EXPORT_SYMBOL(trace_seq_hex_dump);
+
+/**
+ * trace_seq_acquire - acquire seq buffer with size len
+ * @s: trace sequence descriptor
+ * @len: size of buffer to be acquired
+ *
+ * acquire buffer with size of @len from trace_seq for output usage,
+ * user can fill string into that buffer.
+ *
+ * Returns start address of acquired buffer.
+ *
+ * it allow multiple usage in one trace output function call.
+ */
+char *trace_seq_acquire(struct trace_seq *s, size_t len)
+{
+	char *ret = trace_seq_buffer_ptr(s);
+
+	seq_buf_commit(&s->seq, len);
+
+	return ret;
+}
+EXPORT_SYMBOL(trace_seq_acquire);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf
  2022-12-16  9:03 [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Linyu Yuan
  2022-12-16  9:03 ` [PATCH v4 2/2] trace: acquire buffer from temparary trace sequence Linyu Yuan
@ 2023-01-24 19:46 ` Steven Rostedt
  2023-01-30  3:12   ` Linyu Yuan
  1 sibling, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2023-01-24 19:46 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: Masami Hiramatsu, linux-kernel, bpf

On Fri, 16 Dec 2022 17:03:04 +0800
Linyu Yuan <quic_linyyuan@quicinc.com> wrote:

> when experiment change for trace operation, the files under
> include/trace/stages/ will be changed, perf/bpf trace may compile fail.

Sorry for the late reply, I was expecting some feedback from the perf
and/or BPF folks.

> 
> add new file stage8_perf_call.h and stage9_bpf_call.h under
> include/trace/stages/, it will make sure no missing change when expriment.
> 

Actually, these are not part of the stage sequence. In fact they could
possibly be replaced *with* the stage sequences (which I think I'll apply):

For example:

diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index 155c495b89ea..1f7fc1fc590c 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -4,50 +4,7 @@
 
 #ifdef CONFIG_BPF_EVENTS
 
-#undef __entry
-#define __entry entry
-
-#undef __get_dynamic_array
-#define __get_dynamic_array(field)	\
-		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
-
-#undef __get_dynamic_array_len
-#define __get_dynamic_array_len(field)	\
-		((__entry->__data_loc_##field >> 16) & 0xffff)
-
-#undef __get_str
-#define __get_str(field) ((char *)__get_dynamic_array(field))
-
-#undef __get_bitmask
-#define __get_bitmask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_cpumask
-#define __get_cpumask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_sockaddr
-#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
-
-#undef __get_rel_dynamic_array
-#define __get_rel_dynamic_array(field)	\
-		((void *)(&__entry->__rel_loc_##field) +	\
-		 sizeof(__entry->__rel_loc_##field) +		\
-		 (__entry->__rel_loc_##field & 0xffff))
-
-#undef __get_rel_dynamic_array_len
-#define __get_rel_dynamic_array_len(field)	\
-		((__entry->__rel_loc_##field >> 16) & 0xffff)
-
-#undef __get_rel_str
-#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
-
-#undef __get_rel_bitmask
-#define __get_rel_bitmask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_cpumask
-#define __get_rel_cpumask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_sockaddr
-#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
+#include "stages/stage6_event_callback.h"
 
 #undef __perf_count
 #define __perf_count(c)	(c)
diff --git a/include/trace/perf.h b/include/trace/perf.h
index 8f3bf1e17707..2c11181c82e0 100644
--- a/include/trace/perf.h
+++ b/include/trace/perf.h
@@ -4,51 +4,7 @@
 
 #ifdef CONFIG_PERF_EVENTS
 
-#undef __entry
-#define __entry entry
-
-#undef __get_dynamic_array
-#define __get_dynamic_array(field)	\
-		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
-
-#undef __get_dynamic_array_len
-#define __get_dynamic_array_len(field)	\
-		((__entry->__data_loc_##field >> 16) & 0xffff)
-
-#undef __get_str
-#define __get_str(field) ((char *)__get_dynamic_array(field))
-
-#undef __get_bitmask
-#define __get_bitmask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_cpumask
-#define __get_cpumask(field) (char *)__get_dynamic_array(field)
-
-#undef __get_sockaddr
-#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
-
-#undef __get_rel_dynamic_array
-#define __get_rel_dynamic_array(field)	\
-		((void *)__entry +					\
-		 offsetof(typeof(*__entry), __rel_loc_##field) +	\
-		 sizeof(__entry->__rel_loc_##field) +			\
-		 (__entry->__rel_loc_##field & 0xffff))
-
-#undef __get_rel_dynamic_array_len
-#define __get_rel_dynamic_array_len(field)	\
-		((__entry->__rel_loc_##field >> 16) & 0xffff)
-
-#undef __get_rel_str
-#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
-
-#undef __get_rel_bitmask
-#define __get_rel_bitmask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_cpumask
-#define __get_rel_cpumask(field) (char *)__get_rel_dynamic_array(field)
-
-#undef __get_rel_sockaddr
-#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
+#include "stages/stage6_event_callback.h"
 
 #undef __perf_count
 #define __perf_count(c)	(__count = (c))
diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace/stages/stage6_event_callback.h
index 49c32394b53f..919b1a4da980 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -2,6 +2,9 @@
 
 /* Stage 6 definitions for creating trace events */
 
+/* Reuse some of the stage 3 macros */
+#include "stage3_trace_output.h"
+
 #undef __entry
 #define __entry entry
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/2] trace: acquire buffer from temparary trace sequence
  2022-12-16  9:03 ` [PATCH v4 2/2] trace: acquire buffer from temparary trace sequence Linyu Yuan
@ 2023-01-24 21:44   ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2023-01-24 21:44 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: Masami Hiramatsu, linux-kernel, bpf

On Fri, 16 Dec 2022 17:03:05 +0800
Linyu Yuan <quic_linyyuan@quicinc.com> wrote:

> diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
> index 66374df..f60c453 100644
> --- a/include/trace/stages/stage3_trace_output.h
> +++ b/include/trace/stages/stage3_trace_output.h
> @@ -139,3 +139,11 @@
>  		u64 ____val = (u64)(value);		\
>  		(u32) do_div(____val, NSEC_PER_SEC);	\
>  	})
> +
> +/* Macros with flow control statements should be avoided */
> +#undef __get_buf
> +#define __get_buf(len)							\
> +	({								\
> +		WARN_ON_ONCE(seq_buf_buffer_left(&p->seq) < (len));	\

The WARN_ON_ONCE() should be part of the trace_seq_acquire().

> +		trace_seq_acquire(p, (len));				\
> +	})
> diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
> index 8795429..bcb960d 100644
> --- a/include/trace/stages/stage7_class_define.h
> +++ b/include/trace/stages/stage7_class_define.h
> @@ -23,6 +23,7 @@
>  #undef __get_rel_sockaddr
>  #undef __print_array
>  #undef __print_hex_dump
> +#undef __get_buf
>  
>  /*
>   * The below is not executed in the kernel. It is only what is
> diff --git a/kernel/trace/trace_seq.c b/kernel/trace/trace_seq.c
> index 9c90b3a..c900b7c 100644
> --- a/kernel/trace/trace_seq.c
> +++ b/kernel/trace/trace_seq.c
> @@ -403,3 +403,25 @@ int trace_seq_hex_dump(struct trace_seq *s, const char *prefix_str,
>  	return 1;
>  }
>  EXPORT_SYMBOL(trace_seq_hex_dump);
> +
> +/**
> + * trace_seq_acquire - acquire seq buffer with size len
> + * @s: trace sequence descriptor
> + * @len: size of buffer to be acquired
> + *
> + * acquire buffer with size of @len from trace_seq for output usage,
> + * user can fill string into that buffer.
> + *
> + * Returns start address of acquired buffer.
> + *
> + * it allow multiple usage in one trace output function call.
> + */
> +char *trace_seq_acquire(struct trace_seq *s, size_t len)
> +{
> +	char *ret = trace_seq_buffer_ptr(s);

	if (!WARN_ON_ONCE(seq_buf_buffer_left(&p->seq) < (len)))
> +
> +		seq_buf_commit(&s->seq, len);

As seq_buf_commit() should never be called without enough length.

-- Steve

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(trace_seq_acquire);
> -- 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf
  2023-01-24 19:46 ` [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Steven Rostedt
@ 2023-01-30  3:12   ` Linyu Yuan
  0 siblings, 0 replies; 5+ messages in thread
From: Linyu Yuan @ 2023-01-30  3:12 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Masami Hiramatsu, linux-kernel, bpf


On 1/25/2023 3:46 AM, Steven Rostedt wrote:
> On Fri, 16 Dec 2022 17:03:04 +0800
> Linyu Yuan <quic_linyyuan@quicinc.com> wrote:
>
>> when experiment change for trace operation, the files under
>> include/trace/stages/ will be changed, perf/bpf trace may compile fail.
> Sorry for the late reply, I was expecting some feedback from the perf
> and/or BPF folks.
>
>> add new file stage8_perf_call.h and stage9_bpf_call.h under
>> include/trace/stages/, it will make sure no missing change when expriment.
>>
> Actually, these are not part of the stage sequence. In fact they could


thanks for explanation.


> possibly be replaced *with* the stage sequences (which I think I'll apply):
>
> For example:
>
> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
> index 155c495b89ea..1f7fc1fc590c 100644
> --- a/include/trace/bpf_probe.h
> +++ b/include/trace/bpf_probe.h
> @@ -4,50 +4,7 @@
>   
>   #ifdef CONFIG_BPF_EVENTS
>   
> -#undef __entry
> -#define __entry entry
> -
> -#undef __get_dynamic_array
> -#define __get_dynamic_array(field)	\
> -		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
> -
> -#undef __get_dynamic_array_len
> -#define __get_dynamic_array_len(field)	\
> -		((__entry->__data_loc_##field >> 16) & 0xffff)
> -
> -#undef __get_str
> -#define __get_str(field) ((char *)__get_dynamic_array(field))
> -
> -#undef __get_bitmask
> -#define __get_bitmask(field) (char *)__get_dynamic_array(field)
> -
> -#undef __get_cpumask
> -#define __get_cpumask(field) (char *)__get_dynamic_array(field)
> -
> -#undef __get_sockaddr
> -#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
> -
> -#undef __get_rel_dynamic_array
> -#define __get_rel_dynamic_array(field)	\
> -		((void *)(&__entry->__rel_loc_##field) +	\
> -		 sizeof(__entry->__rel_loc_##field) +		\
> -		 (__entry->__rel_loc_##field & 0xffff))
> -
> -#undef __get_rel_dynamic_array_len
> -#define __get_rel_dynamic_array_len(field)	\
> -		((__entry->__rel_loc_##field >> 16) & 0xffff)
> -
> -#undef __get_rel_str
> -#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
> -
> -#undef __get_rel_bitmask
> -#define __get_rel_bitmask(field) (char *)__get_rel_dynamic_array(field)
> -
> -#undef __get_rel_cpumask
> -#define __get_rel_cpumask(field) (char *)__get_rel_dynamic_array(field)
> -
> -#undef __get_rel_sockaddr
> -#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
> +#include "stages/stage6_event_callback.h"
>   
>   #undef __perf_count
>   #define __perf_count(c)	(c)
> diff --git a/include/trace/perf.h b/include/trace/perf.h
> index 8f3bf1e17707..2c11181c82e0 100644
> --- a/include/trace/perf.h
> +++ b/include/trace/perf.h
> @@ -4,51 +4,7 @@
>   
>   #ifdef CONFIG_PERF_EVENTS
>   
> -#undef __entry
> -#define __entry entry
> -
> -#undef __get_dynamic_array
> -#define __get_dynamic_array(field)	\
> -		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
> -
> -#undef __get_dynamic_array_len
> -#define __get_dynamic_array_len(field)	\
> -		((__entry->__data_loc_##field >> 16) & 0xffff)
> -
> -#undef __get_str
> -#define __get_str(field) ((char *)__get_dynamic_array(field))
> -
> -#undef __get_bitmask
> -#define __get_bitmask(field) (char *)__get_dynamic_array(field)
> -
> -#undef __get_cpumask
> -#define __get_cpumask(field) (char *)__get_dynamic_array(field)
> -
> -#undef __get_sockaddr
> -#define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field))
> -
> -#undef __get_rel_dynamic_array
> -#define __get_rel_dynamic_array(field)	\
> -		((void *)__entry +					\
> -		 offsetof(typeof(*__entry), __rel_loc_##field) +	\
> -		 sizeof(__entry->__rel_loc_##field) +			\
> -		 (__entry->__rel_loc_##field & 0xffff))
> -
> -#undef __get_rel_dynamic_array_len
> -#define __get_rel_dynamic_array_len(field)	\
> -		((__entry->__rel_loc_##field >> 16) & 0xffff)
> -
> -#undef __get_rel_str
> -#define __get_rel_str(field) ((char *)__get_rel_dynamic_array(field))
> -
> -#undef __get_rel_bitmask
> -#define __get_rel_bitmask(field) (char *)__get_rel_dynamic_array(field)
> -
> -#undef __get_rel_cpumask
> -#define __get_rel_cpumask(field) (char *)__get_rel_dynamic_array(field)
> -
> -#undef __get_rel_sockaddr
> -#define __get_rel_sockaddr(field) ((struct sockaddr *)__get_rel_dynamic_array(field))
> +#include "stages/stage6_event_callback.h"
>   
>   #undef __perf_count
>   #define __perf_count(c)	(__count = (c))
> diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace/stages/stage6_event_callback.h
> index 49c32394b53f..919b1a4da980 100644
> --- a/include/trace/stages/stage6_event_callback.h
> +++ b/include/trace/stages/stage6_event_callback.h
> @@ -2,6 +2,9 @@
>   
>   /* Stage 6 definitions for creating trace events */
>   
> +/* Reuse some of the stage 3 macros */
> +#include "stage3_trace_output.h"
> +
>   #undef __entry
>   #define __entry entry
>   

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-30  3:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16  9:03 [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Linyu Yuan
2022-12-16  9:03 ` [PATCH v4 2/2] trace: acquire buffer from temparary trace sequence Linyu Yuan
2023-01-24 21:44   ` Steven Rostedt
2023-01-24 19:46 ` [PATCH v4 1/2] trace: add new file under include/trace/stages/ for perf/bpf Steven Rostedt
2023-01-30  3:12   ` Linyu Yuan

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).