linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] perf fixes
@ 2010-05-20  9:44 Frederic Weisbecker
  2010-05-20  9:44 ` [PATCH 1/3] perf: Use read() instead of lseek() in trace_event_read.c:skip() Frederic Weisbecker
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2010-05-20  9:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Paul Mackerras

Ingo,

Please pull the perf/urgent branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	perf/urgent

Thanks,
	Frederic
---

Frederic Weisbecker (2):
      perf: Comply with new rcu checks API
      perf: Fix unaligned accesses while fetching trace values

Tom Zanussi (1):
      perf: Use read() instead of lseek() in trace_event_read.c:skip()


 kernel/perf_event.c                |   58 ++++++++++++++++++++++++++++-------
 tools/perf/util/trace-event-read.c |   19 ++++++++----
 tools/perf/util/trace-event.h      |    7 ++++-
 3 files changed, 65 insertions(+), 19 deletions(-)

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

* [PATCH 1/3] perf: Use read() instead of lseek() in trace_event_read.c:skip()
  2010-05-20  9:44 [GIT PULL] perf fixes Frederic Weisbecker
@ 2010-05-20  9:44 ` Frederic Weisbecker
  2010-05-20  9:44 ` [PATCH 2/3] perf: Comply with new rcu checks API Frederic Weisbecker
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2010-05-20  9:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Tom Zanussi, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Paul Mackerras, Frederic Weisbecker

From: Tom Zanussi <tzanussi@gmail.com>

This is a small fix for a problem affecting live-mode, introduced
recently:

root@tropicana:~# perf trace rwtop
perf trace started with Perl
script /root/libexec/perf-core/scripts/perl/rwtop.pl

  Fatal: did not read header event

commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
function to skip over e.g. header_page, but this doesn't work for
live mode.  This patch re-implements skip() to use read() instead of
lseek() to fix that.

Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1273032130.6383.28.camel@tropicana>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 tools/perf/util/trace-event-read.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index cb54cd0..f55cc3a 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -53,12 +53,6 @@ static unsigned long	page_size;
 static ssize_t calc_data_size;
 static bool repipe;
 
-/* If it fails, the next read will report it */
-static void skip(int size)
-{
-	lseek(input_fd, size, SEEK_CUR);
-}
-
 static int do_read(int fd, void *buf, int size)
 {
 	int rsize = size;
@@ -98,6 +92,19 @@ static int read_or_die(void *data, int size)
 	return r;
 }
 
+/* If it fails, the next read will report it */
+static void skip(int size)
+{
+	char buf[BUFSIZ];
+	int r;
+
+	while (size) {
+		r = size > BUFSIZ ? BUFSIZ : size;
+		read_or_die(buf, r);
+		size -= r;
+	};
+}
+
 static unsigned int read4(void)
 {
 	unsigned int data;
-- 
1.6.2.3


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

* [PATCH 2/3] perf: Comply with new rcu checks API
  2010-05-20  9:44 [GIT PULL] perf fixes Frederic Weisbecker
  2010-05-20  9:44 ` [PATCH 1/3] perf: Use read() instead of lseek() in trace_event_read.c:skip() Frederic Weisbecker
@ 2010-05-20  9:44 ` Frederic Weisbecker
  2010-05-20  9:44 ` [PATCH 3/3] perf: Fix unaligned accesses while fetching trace values Frederic Weisbecker
  2010-05-20 12:40 ` [GIT PULL] perf fixes Ingo Molnar
  3 siblings, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2010-05-20  9:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Paul Mackerras

The software events hlist doesn't fully comply with the new
rcu checks api.

We need to consider three different sides that access the hlist:

- the hlist allocation/release side. This side happens when an
  events is created or released, accesses to the hlist are
  serialized under the cpuctx mutex.

- the events insertion/removal in the hlist. This side is always
  serialized against the above one. The hlist is always present
  during such operations. This side happens when a software event
  is scheduled in/out. The serialization that ensures the software
  event is really attached to the context is made under the
  ctx->lock.

- events triggering. This is the read side, it can happen
  concurrently with any update side.

This patch deals with them one by one and anticipates with the
separate rcu mem space patches in preparation.

This patch fixes various annoying rcu warnings.

Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
---
 kernel/perf_event.c |   58 ++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index a4fa381..511677b 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -4066,19 +4066,46 @@ static inline u64 swevent_hash(u64 type, u32 event_id)
 	return hash_64(val, SWEVENT_HLIST_BITS);
 }
 
-static struct hlist_head *
-find_swevent_head(struct perf_cpu_context *ctx, u64 type, u32 event_id)
+static inline struct hlist_head *
+__find_swevent_head(struct swevent_hlist *hlist, u64 type, u32 event_id)
 {
-	u64 hash;
-	struct swevent_hlist *hlist;
+	u64 hash = swevent_hash(type, event_id);
+
+	return &hlist->heads[hash];
+}
 
-	hash = swevent_hash(type, event_id);
+/* For the read side: events when they trigger */
+static inline struct hlist_head *
+find_swevent_head_rcu(struct perf_cpu_context *ctx, u64 type, u32 event_id)
+{
+	struct swevent_hlist *hlist;
 
 	hlist = rcu_dereference(ctx->swevent_hlist);
 	if (!hlist)
 		return NULL;
 
-	return &hlist->heads[hash];
+	return __find_swevent_head(hlist, type, event_id);
+}
+
+/* For the event head insertion and removal in the hlist */
+static inline struct hlist_head *
+find_swevent_head(struct perf_cpu_context *ctx, struct perf_event *event)
+{
+	struct swevent_hlist *hlist;
+	u32 event_id = event->attr.config;
+	u64 type = event->attr.type;
+
+	/*
+	 * Event scheduling is always serialized against hlist allocation
+	 * and release. Which makes the protected version suitable here.
+	 * The context lock guarantees that.
+	 */
+	hlist = rcu_dereference_protected(ctx->swevent_hlist,
+					  lockdep_is_held(&event->ctx->lock));
+	if (!hlist)
+		return NULL;
+
+	return __find_swevent_head(hlist, type, event_id);
 }
 
 static void do_perf_sw_event(enum perf_type_id type, u32 event_id,
@@ -4095,7 +4122,7 @@ static void do_perf_sw_event(enum perf_type_id type, u32 event_id,
 
 	rcu_read_lock();
 
-	head = find_swevent_head(cpuctx, type, event_id);
+	head = find_swevent_head_rcu(cpuctx, type, event_id);
 
 	if (!head)
 		goto end;
@@ -4178,7 +4205,7 @@ static int perf_swevent_enable(struct perf_event *event)
 		perf_swevent_set_period(event);
 	}
 
-	head = find_swevent_head(cpuctx, event->attr.type, event->attr.config);
+	head = find_swevent_head(cpuctx, event);
 	if (WARN_ON_ONCE(!head))
 		return -EINVAL;
 
@@ -4366,6 +4393,14 @@ static const struct pmu perf_ops_task_clock = {
 	.read		= task_clock_perf_event_read,
 };
 
+/* Deref the hlist from the update side */
+static inline struct swevent_hlist *
+swevent_hlist_deref(struct perf_cpu_context *cpuctx)
+{
+	return rcu_dereference_protected(cpuctx->swevent_hlist,
+					 lockdep_is_held(&cpuctx->hlist_mutex));
+}
+
 static void swevent_hlist_release_rcu(struct rcu_head *rcu_head)
 {
 	struct swevent_hlist *hlist;
@@ -4376,12 +4411,11 @@ static void swevent_hlist_release_rcu(struct rcu_head *rcu_head)
 
 static void swevent_hlist_release(struct perf_cpu_context *cpuctx)
 {
-	struct swevent_hlist *hlist;
+	struct swevent_hlist *hlist = swevent_hlist_deref(cpuctx);
 
-	if (!cpuctx->swevent_hlist)
+	if (!hlist)
 		return;
 
-	hlist = cpuctx->swevent_hlist;
 	rcu_assign_pointer(cpuctx->swevent_hlist, NULL);
 	call_rcu(&hlist->rcu_head, swevent_hlist_release_rcu);
 }
@@ -4418,7 +4452,7 @@ static int swevent_hlist_get_cpu(struct perf_event *event, int cpu)
 
 	mutex_lock(&cpuctx->hlist_mutex);
 
-	if (!cpuctx->swevent_hlist && cpu_online(cpu)) {
+	if (!swevent_hlist_deref(cpuctx) && cpu_online(cpu)) {
 		struct swevent_hlist *hlist;
 
 		hlist = kzalloc(sizeof(*hlist), GFP_KERNEL);
-- 
1.6.2.3


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

* [PATCH 3/3] perf: Fix unaligned accesses while fetching trace values
  2010-05-20  9:44 [GIT PULL] perf fixes Frederic Weisbecker
  2010-05-20  9:44 ` [PATCH 1/3] perf: Use read() instead of lseek() in trace_event_read.c:skip() Frederic Weisbecker
  2010-05-20  9:44 ` [PATCH 2/3] perf: Comply with new rcu checks API Frederic Weisbecker
@ 2010-05-20  9:44 ` Frederic Weisbecker
  2010-05-20 12:40 ` [GIT PULL] perf fixes Ingo Molnar
  3 siblings, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2010-05-20  9:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Paul Mackerras, Steven Rostedt,
	Tom Zanussi, Masami Hiramatsu, David Miller

Accessing trace values of an 8 size may end up in a segfault
on archs that can't deal with misaligned access, which is the
case for sparc 64. This is because PERF_SAMPLE_RAW are aligned
to 4 and not to 8.

Fix this on the macros that get the values of 8 size.

This fixes segfaults on perf tools in sparc 64.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
Cc: David Miller <davem@davemloft.net>
---
 tools/perf/util/trace-event.h |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 406d452..b3e86b1 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -233,7 +233,12 @@ static inline unsigned long long __data2host8(unsigned long long data)
 
 #define data2host2(ptr)		__data2host2(*(unsigned short *)ptr)
 #define data2host4(ptr)		__data2host4(*(unsigned int *)ptr)
-#define data2host8(ptr)		__data2host8(*(unsigned long long *)ptr)
+#define data2host8(ptr)		({				\
+	unsigned long long __val;				\
+								\
+	memcpy(&__val, (ptr), sizeof(unsigned long long));	\
+	__data2host8(__val);					\
+})
 
 extern int header_page_ts_offset;
 extern int header_page_ts_size;
-- 
1.6.2.3


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

* Re: [GIT PULL] perf fixes
  2010-05-20  9:44 [GIT PULL] perf fixes Frederic Weisbecker
                   ` (2 preceding siblings ...)
  2010-05-20  9:44 ` [PATCH 3/3] perf: Fix unaligned accesses while fetching trace values Frederic Weisbecker
@ 2010-05-20 12:40 ` Ingo Molnar
  3 siblings, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2010-05-20 12:40 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Peter Zijlstra, Arnaldo Carvalho de Melo, Paul Mackerras


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo,
> 
> Please pull the perf/urgent branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/urgent
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (2):
>       perf: Comply with new rcu checks API
>       perf: Fix unaligned accesses while fetching trace values
> 
> Tom Zanussi (1):
>       perf: Use read() instead of lseek() in trace_event_read.c:skip()
> 
> 
>  kernel/perf_event.c                |   58 ++++++++++++++++++++++++++++-------
>  tools/perf/util/trace-event-read.c |   19 ++++++++----
>  tools/perf/util/trace-event.h      |    7 ++++-
>  3 files changed, 65 insertions(+), 19 deletions(-)

Pulled, thanks Frederic!

	Ingo

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

end of thread, other threads:[~2010-05-20 12:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-20  9:44 [GIT PULL] perf fixes Frederic Weisbecker
2010-05-20  9:44 ` [PATCH 1/3] perf: Use read() instead of lseek() in trace_event_read.c:skip() Frederic Weisbecker
2010-05-20  9:44 ` [PATCH 2/3] perf: Comply with new rcu checks API Frederic Weisbecker
2010-05-20  9:44 ` [PATCH 3/3] perf: Fix unaligned accesses while fetching trace values Frederic Weisbecker
2010-05-20 12:40 ` [GIT PULL] perf fixes Ingo Molnar

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