tracing: more list_empty(perf_events) checks

PATCH Superseded Oleg Nesterov 3 participants 15 messages Jun 17, 2013 – Jul 18, 2013

Subsystems (2)

Tracing ftrace

lore

Share & tools

lore

Oleg Nesterov

author

2013-06-17 17:07 UTC #0 lore raw

cover tracing: more list_empty(perf_events) checks

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

Oleg Nesterov

author

2013-06-17 17:06 UTC #1 lore raw

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

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 <[email protected]>

---
 kernel/trace/trace_event_perf.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

kernel/trace/trace_event_perf.c +4 -2

index 84b1e04..12df557 100644
@@ -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);

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2013-06-17 17:07 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 kernel/trace/trace_syscalls.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

kernel/trace/trace_syscalls.c +8 -4

index 8f2ac73..4d1bd5d 100644
@@ -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);
 }

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2013-06-17 17:06 UTC #3 lore raw

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

In reply to 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 <[email protected]>

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

include/trace/ftrace.h +0 -4

index 19edd7f..c162a57 100644
@@ -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)							\

kernel/trace/trace_event_perf.c +4 -0

index 12df557..80c36bc 100644
@@ -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();

kernel/trace/trace_kprobe.c +0 -6

index b95f683..156a4d8 100644
@@ -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)

kernel/trace/trace_syscalls.c +0 ~~-12~~

index 4d1bd5d..d17246e 100644
@@ -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)

kernel/trace/trace_uprobe.c +0 -2

index 32494fb..4a32b52 100644
@@ -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);

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2013-06-17 20:22 UTC #4 lore raw

In reply to Oleg Nesterov

Quoted 3 lines · Oleg Nesterov

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.

include/trace/events/sched.h +2 -9

@@ -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]",

include/trace/ftrace.h +7 -2

@@ -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)							\

Steven Rostedt

maintainer

2013-06-17 21:27 UTC #5 lore raw

Quoted 6 lines · Oleg Nesterov

Quoted 3 lines · Oleg Nesterov
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?

Quoted 14 lines

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

Quoted 7 lines

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.

Quoted 67 lines

Oleg.

include/trace/events/sched.h +2 -9

@@ -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]",

include/trace/ftrace.h +6 -0

@@ -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							\

include/trace/ftrace.h perf_trace_##call(void *__data, proto) \ · line 668

Quoted 6 lines

include/trace/ftrace.h +0 -2

@@ -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

Quoted 8 lines

include/trace/ftrace.h +1 -0

 	__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)							\

Oleg Nesterov

author

2013-06-18 14:51 UTC #6 lore raw

Quoted 10 lines · Steven Rostedt

Quoted 6 lines · Oleg Nesterov
Quoted 3 lines · Oleg Nesterov
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.

Quoted 10 lines

include/trace/ftrace.h perf_trace_##call(void *__data, proto) \ · line 668
Quoted 7 lines
include/trace/ftrace.h +0 -2
@@ -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.

Quoted 3 lines

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.

Steven Rostedt

maintainer

2013-06-18 15:41 UTC #7 lore raw

Quoted 10 lines · Oleg Nesterov

Quoted 9 lines · Steven Rostedt
Quoted 6 lines · Oleg Nesterov
Quoted 3 lines · Oleg Nesterov
DECLARE_EVENT_CLASS()->perf_trace_##call() is not trivial because of __perf_task()
Perhaps we can do something like below?

Quoted 7 lines

Quoted 1 line
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.

Quoted 11 lines

Quoted 11 lines
include/trace/ftrace.h perf_trace_##call(void *__data, proto) \ · line 668
Quoted 8 lines
include/trace/ftrace.h +0 -2
@@ -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.

Quoted 8 lines

Quoted 3 lines
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

Oleg Nesterov

author

2013-06-18 16:30 UTC #8 lore raw

Quoted 8 lines · Steven Rostedt

Quoted 4 lines · Oleg Nesterov
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.

Steven Rostedt

maintainer

2013-07-18 03:00 UTC #9 lore raw

In reply to Oleg Nesterov

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

Quoted 20 lines · 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(-)

Peter Zijlstra

2013-07-18 09:42 UTC #10 lore raw

Quoted 4 lines · 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.

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.

Steven Rostedt

maintainer

2013-07-18 14:39 UTC #11 lore raw

Quoted 6 lines · Peter Zijlstra

Quoted 4 lines · 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.

Quoted 6 lines

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

Oleg Nesterov

author

2013-07-18 15:49 UTC #12 lore raw

Quoted 4 lines · Steven Rostedt

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.

Steven Rostedt

maintainer

2013-07-18 15:53 UTC #13 lore raw

Quoted 3 lines · Oleg Nesterov

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

Oleg Nesterov

author

2013-07-18 16:16 UTC #14 lore raw

Quoted 8 lines · Steven Rostedt

Quoted 3 lines · Oleg Nesterov
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.