[PATCH 0/3] tracing: more list_empty(perf_events) checks

[PATCH 0/3] tracing: more list_empty(perf_events) checks

[Oleg Nesterov]

Hello.

Steven, we already discussed this a bit some time ago...

DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because
of __perf_task(), but perhaps we can change other
perf_trace_buf_submit(task => NULL) callers.


And can't we factor out WARN_ONCE(size > PERF_MAX_TRACE_SIZE) ?
See 3/3. I won't argue if you dislike it.

Oleg.

 include/trace/ftrace.h          |    4 ----
 kernel/trace/trace_event_perf.c |   10 ++++++++--
 kernel/trace/trace_kprobe.c     |    6 ------
 kernel/trace/trace_syscalls.c   |   24 ++++++++----------------
 kernel/trace/trace_uprobe.c     |    2 --
 5 files changed, 16 insertions(+), 30 deletions(-)

[PATCH 1/3] tracing/function: Avoid perf_trace_buf_*() if event_function.perf_events is empty

[Oleg Nesterov]

perf_trace_buf_prepare() + perf_trace_buf_submit(head, task => NULL)
make no sense if hlist_empty(head). Change perf_ftrace_function_call()
to check event_function.perf_events beforehand.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/trace/trace_event_perf.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 84b1e04..12df557 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -266,6 +266,10 @@ perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
 	struct pt_regs regs;
 	int rctx;
 
+	head = this_cpu_ptr(event_function.perf_events);
+	if (hlist_empty(head))
+		return;
+
 #define ENTRY_SIZE (ALIGN(sizeof(struct ftrace_entry) + sizeof(u32), \
 		    sizeof(u64)) - sizeof(u32))
 
@@ -279,8 +283,6 @@ perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
 
 	entry->ip = ip;
 	entry->parent_ip = parent_ip;
-
-	head = this_cpu_ptr(event_function.perf_events);
 	perf_trace_buf_submit(entry, ENTRY_SIZE, rctx, 0,
 			      1, &regs, head, NULL);
 
-- 
1.5.5.1

[PATCH 2/3] tracing/syscall: Avoid perf_trace_buf_*() if sys_data->perf_events is empty

[Oleg Nesterov]

perf_trace_buf_prepare() + perf_trace_buf_submit(head, task => NULL)
make no sense if hlist_empty(head). Change perf_syscall_enter/exit()
to check sys_data->{enter,exit}_event->perf_events beforehand.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/trace/trace_syscalls.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 8f2ac73..4d1bd5d 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -553,6 +553,10 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	if (!sys_data)
 		return;
 
+	head = this_cpu_ptr(sys_data->enter_event->perf_events);
+	if (hlist_empty(head))
+		return;
+
 	/* get the size after alignment with the u32 buffer size field */
 	size = sizeof(unsigned long) * sys_data->nb_args + sizeof(*rec);
 	size = ALIGN(size + sizeof(u32), sizeof(u64));
@@ -570,8 +574,6 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	rec->nr = syscall_nr;
 	syscall_get_arguments(current, regs, 0, sys_data->nb_args,
 			       (unsigned long *)&rec->args);
-
-	head = this_cpu_ptr(sys_data->enter_event->perf_events);
 	perf_trace_buf_submit(rec, size, rctx, 0, 1, regs, head, NULL);
 }
 
@@ -629,6 +631,10 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
 	if (!sys_data)
 		return;
 
+	head = this_cpu_ptr(sys_data->exit_event->perf_events);
+	if (hlist_empty(head))
+		return;
+
 	/* We can probably do that at build time */
 	size = ALIGN(sizeof(*rec) + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
@@ -648,8 +654,6 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
 
 	rec->nr = syscall_nr;
 	rec->ret = syscall_get_return_value(current, regs);
-
-	head = this_cpu_ptr(sys_data->exit_event->perf_events);
 	perf_trace_buf_submit(rec, size, rctx, 0, 1, regs, head, NULL);
 }
 
-- 
1.5.5.1

[PATCH 3/3] tracing/perf: Move the PERF_MAX_TRACE_SIZE check into perf_trace_buf_prepare()

[Oleg Nesterov]

Every perf_trace_buf_prepare() caller does
WARN_ONCE(size > PERF_MAX_TRACE_SIZE, message) and "message" is
almost the same.

Shift this WARN_ONCE() into perf_trace_buf_prepare(). This changes
the meaning of _ONCE, but I think this is fine.

	- 4947014 2932448 10104832  17984294  1126b26 vmlinux
	+ 4948422 2932448 10104832  17985702  11270a6 vmlinux

on my build.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/trace/ftrace.h          |    4 ----
 kernel/trace/trace_event_perf.c |    4 ++++
 kernel/trace/trace_kprobe.c     |    6 ------
 kernel/trace/trace_syscalls.c   |   12 ------------
 kernel/trace/trace_uprobe.c     |    2 --
 5 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 19edd7f..c162a57 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -666,10 +666,6 @@ perf_trace_##call(void *__data, proto)					\
 			     sizeof(u64));				\
 	__entry_size -= sizeof(u32);					\
 									\
-	if (WARN_ONCE(__entry_size > PERF_MAX_TRACE_SIZE,		\
-		      "profile buffer not large enough"))		\
-		return;							\
-									\
 	entry = (struct ftrace_raw_##call *)perf_trace_buf_prepare(	\
 		__entry_size, event_call->event.type, &__regs, &rctx);	\
 	if (!entry)							\
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 12df557..80c36bc 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -236,6 +236,10 @@ __kprobes void *perf_trace_buf_prepare(int size, unsigned short type,
 
 	BUILD_BUG_ON(PERF_MAX_TRACE_SIZE % sizeof(unsigned long));
 
+	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
+			"perf buffer not large enough"))
+		return NULL;
+
 	pc = preempt_count();
 
 	*rctxp = perf_swevent_get_recursion_context();
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b95f683..156a4d8 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1088,9 +1088,6 @@ kprobe_perf_func(struct trace_probe *tp, struct pt_regs *regs)
 	__size = sizeof(*entry) + tp->size + dsize;
 	size = ALIGN(__size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
-	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
-		     "profile buffer not large enough"))
-		return;
 
 	entry = perf_trace_buf_prepare(size, call->event.type, regs, &rctx);
 	if (!entry)
@@ -1122,9 +1119,6 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
 	__size = sizeof(*entry) + tp->size + dsize;
 	size = ALIGN(__size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
-	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
-		     "profile buffer not large enough"))
-		return;
 
 	entry = perf_trace_buf_prepare(size, call->event.type, regs, &rctx);
 	if (!entry)
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 4d1bd5d..d17246e 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -562,10 +562,6 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	size = ALIGN(size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
 
-	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
-		      "perf buffer not large enough"))
-		return;
-
 	rec = (struct syscall_trace_enter *)perf_trace_buf_prepare(size,
 				sys_data->enter_event->event.type, regs, &rctx);
 	if (!rec)
@@ -639,14 +635,6 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
 	size = ALIGN(sizeof(*rec) + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
 
-	/*
-	 * Impossible, but be paranoid with the future
-	 * How to put this check outside runtime?
-	 */
-	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
-		"exit event has grown above perf buffer size"))
-		return;
-
 	rec = (struct syscall_trace_exit *)perf_trace_buf_prepare(size,
 				sys_data->exit_event->event.type, regs, &rctx);
 	if (!rec)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 32494fb..4a32b52 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -816,8 +816,6 @@ static void uprobe_perf_print(struct trace_uprobe *tu,
 
 	size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
 	size = ALIGN(size + tu->size + sizeof(u32), sizeof(u64)) - sizeof(u32);
-	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE, "profile buffer not large enough"))
-		return;
 
 	preempt_disable();
 	head = this_cpu_ptr(call->perf_events);
-- 
1.5.5.1

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Oleg Nesterov]

On 06/17, Oleg Nesterov wrote:
>
> DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because
> of __perf_task()

Perhaps we can do something like below?

Then we can

	1. kill __perf_addr(), __perf_count(), __perf_task() and
	   TP_perf_assign()

	2. Add the fast path check

		if (builtin_constant(__task) && !__task &&
		    hlist_empty(head)))
			return;

	   into perf_trace_call() right after ftrace_get_offsets_call().

Doesn't look very nice, it relies on the fact that
ftrace_get_offsets_call(args) evaluates TP_perf_arg()'s hidden in
TP_ARGS().

OTOH, the whole point of include/trace/ is to create the code which
nobody except the maintainers can understand. At least I certainly
can't ;) So perhaps this is fine.

Oleg.

--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -57,7 +57,7 @@ DECLARE_EVENT_CLASS(sched_wakeup_template,
 
 	TP_PROTO(struct task_struct *p, int success),
 
-	TP_ARGS(p, success),
+	TP_ARGS(TP_perf_arg(__task, p), success),
 
 	TP_STRUCT__entry(
 		__array(	char,	comm,	TASK_COMM_LEN	)
@@ -73,9 +73,6 @@ DECLARE_EVENT_CLASS(sched_wakeup_template,
 		__entry->prio		= p->prio;
 		__entry->success	= success;
 		__entry->target_cpu	= task_cpu(p);
-	)
-	TP_perf_assign(
-		__perf_task(p);
 	),
 
 	TP_printk("comm=%s pid=%d prio=%d success=%d target_cpu=%03d",
@@ -313,7 +310,7 @@ DECLARE_EVENT_CLASS(sched_stat_template,
 
 	TP_PROTO(struct task_struct *tsk, u64 delay),
 
-	TP_ARGS(tsk, delay),
+	TP_ARGS(TP_perf_arg(__task, tsk), TP_perf_arg(__count, delay)),
 
 	TP_STRUCT__entry(
 		__array( char,	comm,	TASK_COMM_LEN	)
@@ -325,10 +322,6 @@ DECLARE_EVENT_CLASS(sched_stat_template,
 		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
 		__entry->pid	= tsk->pid;
 		__entry->delay	= delay;
-	)
-	TP_perf_assign(
-		__perf_count(delay);
-		__perf_task(tsk);
 	),
 
 	TP_printk("comm=%s pid=%d delay=%Lu [ns]",
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -506,6 +506,9 @@ static inline notrace int ftrace_get_offsets_##call(			\
 #undef TP_perf_assign
 #define TP_perf_assign(args...)
 
+#undef TP_perf_arg
+#define TP_perf_arg(var, val)	(val)
+
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 									\
@@ -643,6 +646,9 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
 #undef TP_perf_assign
 #define TP_perf_assign(args...) args
 
+#undef TP_perf_arg
+#define TP_perf_arg(var, val)	(var = (val))
+
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 static notrace void							\
@@ -659,13 +665,12 @@ perf_trace_##call(void *__data, proto)					\
 	int __data_size;						\
 	int rctx;							\
 									\
-	perf_fetch_caller_regs(&__regs);				\
-									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
 	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
 			     sizeof(u64));				\
 	__entry_size -= sizeof(u32);					\
 									\
+	perf_fetch_caller_regs(&__regs);				\
 	entry = (struct ftrace_raw_##call *)perf_trace_buf_prepare(	\
 		__entry_size, event_call->event.type, &__regs, &rctx);	\
 	if (!entry)							\

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Steven Rostedt]

On Mon, 2013-06-17 at 22:18 +0200, Oleg Nesterov wrote:
> On 06/17, Oleg Nesterov wrote:
> >
> > DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because
> > of __perf_task()
> 
> Perhaps we can do something like below?

Did this actually compile for you?

> 
> Then we can
> 
> 	1. kill __perf_addr(), __perf_count(), __perf_task() and
> 	   TP_perf_assign()
> 
> 	2. Add the fast path check
> 
> 		if (builtin_constant(__task) && !__task &&
> 		    hlist_empty(head)))
> 			return;
> 
> 	   into perf_trace_call() right after ftrace_get_offsets_call().
> 
> Doesn't look very nice, it relies on the fact that
> ftrace_get_offsets_call(args) evaluates TP_perf_arg()'s hidden in
> TP_ARGS().
> 
> OTOH, the whole point of include/trace/ is to create the code which
> nobody except the maintainers can understand. At least I certainly
> can't ;) So perhaps this is fine.

It's the land of the MAGIC MACROs. You must be a MACRO WIZARD to
understand. I don't understand it unless I put on my MACRO WIZARD HAT.



> 
> Oleg.
> 
> --- a/include/trace/events/sched.h
> +++ b/include/trace/events/sched.h
> @@ -57,7 +57,7 @@ DECLARE_EVENT_CLASS(sched_wakeup_template,
>  
>  	TP_PROTO(struct task_struct *p, int success),
>  
> -	TP_ARGS(p, success),
> +	TP_ARGS(TP_perf_arg(__task, p), success),
>  
>  	TP_STRUCT__entry(
>  		__array(	char,	comm,	TASK_COMM_LEN	)
> @@ -73,9 +73,6 @@ DECLARE_EVENT_CLASS(sched_wakeup_template,
>  		__entry->prio		= p->prio;
>  		__entry->success	= success;
>  		__entry->target_cpu	= task_cpu(p);
> -	)
> -	TP_perf_assign(
> -		__perf_task(p);
>  	),
>  
>  	TP_printk("comm=%s pid=%d prio=%d success=%d target_cpu=%03d",
> @@ -313,7 +310,7 @@ DECLARE_EVENT_CLASS(sched_stat_template,
>  
>  	TP_PROTO(struct task_struct *tsk, u64 delay),
>  
> -	TP_ARGS(tsk, delay),
> +	TP_ARGS(TP_perf_arg(__task, tsk), TP_perf_arg(__count, delay)),
>  
>  	TP_STRUCT__entry(
>  		__array( char,	comm,	TASK_COMM_LEN	)
> @@ -325,10 +322,6 @@ DECLARE_EVENT_CLASS(sched_stat_template,
>  		memcpy(__entry->comm, tsk->comm, TASK_COMM_LEN);
>  		__entry->pid	= tsk->pid;
>  		__entry->delay	= delay;
> -	)
> -	TP_perf_assign(
> -		__perf_count(delay);
> -		__perf_task(tsk);
>  	),
>  
>  	TP_printk("comm=%s pid=%d delay=%Lu [ns]",
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -506,6 +506,9 @@ static inline notrace int ftrace_get_offsets_##call(			\
>  #undef TP_perf_assign
>  #define TP_perf_assign(args...)
>  
> +#undef TP_perf_arg
> +#define TP_perf_arg(var, val)	(val)
> +
>  #undef DECLARE_EVENT_CLASS
>  #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
>  									\
> @@ -643,6 +646,9 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
>  #undef TP_perf_assign
>  #define TP_perf_assign(args...) args
>  
> +#undef TP_perf_arg
> +#define TP_perf_arg(var, val)	(var = (val))
> +
>  #undef DECLARE_EVENT_CLASS
>  #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
>  static notrace void							\
> @@ -659,13 +665,12 @@ perf_trace_##call(void *__data, proto)					\
>  	int __data_size;						\
>  	int rctx;							\
>  									\
> -	perf_fetch_caller_regs(&__regs);				\
> -									\
>  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \

OK, so here the task gets assigned the val, and so does count.

This may not be a bad approach, but instead of having TP_perf_arg() in
events/sched.h, keep the TP_perf_task() and TP_perf_count(), and have
whatever is put there assigned. As "__task" and "__count" are hardcoded
into the macro. The you would have:

#undef TP_perf_task
#define TP_perf_task(val) (__task = (val))

#undef TP_perf_count
#define TP_perf_count(val) (__count = (val))

And that should work as well.

-- Steve


>  	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
>  			     sizeof(u64));				\
>  	__entry_size -= sizeof(u32);					\
>  									\
> +	perf_fetch_caller_regs(&__regs);				\
>  	entry = (struct ftrace_raw_##call *)perf_trace_buf_prepare(	\
>  		__entry_size, event_call->event.type, &__regs, &rctx);	\
>  	if (!entry)							\

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Oleg Nesterov]

On 06/17, Steven Rostedt wrote:
>
> On Mon, 2013-06-17 at 22:18 +0200, Oleg Nesterov wrote:
> > On 06/17, Oleg Nesterov wrote:
> > >
> > > DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because
> > > of __perf_task()
> >
> > Perhaps we can do something like below?
>
> Did this actually compile for you?

Why did you ask?

Perhaps you are trying to say that this patch needs more work...

Just because it can't be compiled? Pedant.

> > @@ -659,13 +665,12 @@ perf_trace_##call(void *__data, proto)					\
> >  	int __data_size;						\
> >  	int rctx;							\
> >  									\
> > -	perf_fetch_caller_regs(&__regs);				\
> > -									\
> >  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
>
> OK, so here the task gets assigned the val, and so does count.
>
> This may not be a bad approach, but instead of having TP_perf_arg() in
> events/sched.h, keep the TP_perf_task() and TP_perf_count(), and have
> whatever is put there assigned.

Or this, yes.

OK. Let me try to make something working. At least, something I believe
should work, I will mostly rely on your review anyway.

Oleg.

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Steven Rostedt]

On Tue, 2013-06-18 at 16:46 +0200, Oleg Nesterov wrote:
> On 06/17, Steven Rostedt wrote:
> >
> > On Mon, 2013-06-17 at 22:18 +0200, Oleg Nesterov wrote:
> > > On 06/17, Oleg Nesterov wrote:
> > > >
> > > > DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because
> > > > of __perf_task()
> > >
> > > Perhaps we can do something like below?
> >
> > Did this actually compile for you?
> 
> Why did you ask?
> 
> Perhaps you are trying to say that this patch needs more work...
> 
> Just because it can't be compiled? Pedant.

No, just because when I first looked at it, I didn't think it would, and
didn't delete this when I took a deeper look.

> 
> > > @@ -659,13 +665,12 @@ perf_trace_##call(void *__data, proto)					\
> > >  	int __data_size;						\
> > >  	int rctx;							\
> > >  									\
> > > -	perf_fetch_caller_regs(&__regs);				\
> > > -									\
> > >  	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> >
> > OK, so here the task gets assigned the val, and so does count.
> >
> > This may not be a bad approach, but instead of having TP_perf_arg() in
> > events/sched.h, keep the TP_perf_task() and TP_perf_count(), and have
> > whatever is put there assigned.
> 
> Or this, yes.
> 
> OK. Let me try to make something working. At least, something I believe
> should work, I will mostly rely on your review anyway.

Great,

-- Steve

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Oleg Nesterov]

On 06/18, Steven Rostedt wrote:
>
> On Tue, 2013-06-18 at 16:46 +0200, Oleg Nesterov wrote:
> >
> > Perhaps you are trying to say that this patch needs more work...
> >
> > Just because it can't be compiled? Pedant.
>
> No, just because when I first looked at it, I didn't think it would,

And your intuition was correct.

It can't be compiled if you also change TRACE_EVENT(sched_stat_runtime)
to use TP_perf_arg(__count, runtime). Hopefully this can be fixed by

	- TRACE_EVENT(...)
	+ DECLARE_EVENT_CLASS(...)
	+ DEFINE_EVENT(...)

Oleg.

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Steven Rostedt]

Peter,

These have been discussed, and they mostly live in the tracing
directory, but are perf related. Can you give me your Acked-by on them.

Thanks,

-- Steve


On Mon, 2013-06-17 at 19:01 +0200, Oleg Nesterov wrote:
> Hello.
> 
> Steven, we already discussed this a bit some time ago...
> 
> DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because
> of __perf_task(), but perhaps we can change other
> perf_trace_buf_submit(task => NULL) callers.
> 
> 
> And can't we factor out WARN_ONCE(size > PERF_MAX_TRACE_SIZE) ?
> See 3/3. I won't argue if you dislike it.
> 
> Oleg.
> 
>  include/trace/ftrace.h          |    4 ----
>  kernel/trace/trace_event_perf.c |   10 ++++++++--
>  kernel/trace/trace_kprobe.c     |    6 ------
>  kernel/trace/trace_syscalls.c   |   24 ++++++++----------------
>  kernel/trace/trace_uprobe.c     |    2 --
>  5 files changed, 16 insertions(+), 30 deletions(-)

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Peter Zijlstra]

On Wed, Jul 17, 2013 at 11:00:46PM -0400, Steven Rostedt wrote:
> Peter,
> 
> These have been discussed, and they mostly live in the tracing
> directory, but are perf related. Can you give me your Acked-by on them.

I haven't looked in detail, but I trust Oleg. The only thing I can
remember is that I objected to the changelogs in that it wasn't at all
clear why we were doing these patches (Oleg did clarify in a separate
email, not sure the changelogs were ammended).

Other than that sure, go ahead.

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Steven Rostedt]

On Thu, 2013-07-18 at 11:42 +0200, Peter Zijlstra wrote:
> On Wed, Jul 17, 2013 at 11:00:46PM -0400, Steven Rostedt wrote:
> > Peter,
> > 
> > These have been discussed, and they mostly live in the tracing
> > directory, but are perf related. Can you give me your Acked-by on them.
> 
> I haven't looked in detail, but I trust Oleg. The only thing I can
> remember is that I objected to the changelogs in that it wasn't at all
> clear why we were doing these patches (Oleg did clarify in a separate
> email, not sure the changelogs were ammended).
> 
> Other than that sure, go ahead.

Oleg, did you update the change logs? These patches look the same as
what was in your mbox. Or did Peter have an issues with the change log
of another patch?

-- Steve

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Oleg Nesterov]

On 07/18, Steven Rostedt wrote:
>
> Oleg, did you update the change logs? These patches look the same as
> what was in your mbox. Or did Peter have an issues with the change log
> of another patch?

No, I didn't change them in any way, just resended.

To remind. 0/3 says "Compile tested only, not for inclusion yet" but
I tried to test them after that, including the performance testing.
However I did this under KVM so I am not sure if we can trust these
numbers. But it seems that you got the similar results with
perf-bench-sched-pipe: http://marc.info/?l=linux-kernel&m=137176685731120


Steven, I am starting to think that it would be better to resend this
series (3-6 in mbox I sent) so that Peter and Frederic can take another
look. And I'll try to update the changelogs. Will do a bit later today.

OK?

Oleg.

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Steven Rostedt]

On Thu, 2013-07-18 at 17:44 +0200, Oleg Nesterov wrote:

> Steven, I am starting to think that it would be better to resend this
> series (3-6 in mbox I sent) so that Peter and Frederic can take another
> look. And I'll try to update the changelogs. Will do a bit later today.

Or do you mean 4-6? As I applied 1-3 already, and added Peter's
Acked-by.

I'm guessing 1-3's change logs were good enough.

-- Steve

Re: [PATCH 0/3] tracing: more list_empty(perf_events) checks

[Oleg Nesterov]

On 07/18, Steven Rostedt wrote:
>
> On Thu, 2013-07-18 at 17:44 +0200, Oleg Nesterov wrote:
>
> > Steven, I am starting to think that it would be better to resend this
> > series (3-6 in mbox I sent) so that Peter and Frederic can take another
> > look. And I'll try to update the changelogs. Will do a bit later today.
>
> Or do you mean 4-6?

Yeeess, sorry 4-6.

Oleg.