From: markus.t.metzger@intel.com
To: mingo@elte.hu, tglx@linutronix.de, hpa@zytor.com
Cc: markus.t.metzger@gmail.com, roland@redhat.com,
eranian@googlemail.com, oleg@redhat.com, juan.villacis@intel.com,
ak@linux.jf.intel.com, linux-kernel@vger.kernel.org,
Markus Metzger <markus.t.metzger@intel.com>
Subject: [patch 05/20] x86, bts: fix race between per-task and per-cpu branch tracing
Date: Fri, 03 Apr 2009 16:43:37 +0200 [thread overview]
Message-ID: <20090403144552.856000000@intel.com> (raw)
In-Reply-To: 20090403144332.799740000@intel.com
[-- Attachment #1: tip.master.ds.get_tracer.patch --]
[-- Type: text/plain, Size: 4925 bytes --]
Per-task branch tracing installs a debug store context with the traced
task. This immediately results in the branch trace control bits to be
cleared for the next context switch of that task, if not set before.
Either per-cpu or per-task tracing are allowed at the same time.
An active per-cpu tracing would be disabled even if the per-task tracing
request is rejected and the task debug store context removed.
Check the tracing type (per-cpu or per-task) before installing a task
debug store context.
Signed-off-by: Markus Metzger <markus.t.metzger@intel.com>
---
arch/x86/kernel/ds.c | 72 41 + 31 - 0 !
1 file changed, 41 insertions(+), 31 deletions(-)
Index: b/arch/x86/kernel/ds.c
===================================================================
--- a/arch/x86/kernel/ds.c
+++ b/arch/x86/kernel/ds.c
@@ -193,12 +193,28 @@ static DEFINE_SPINLOCK(ds_lock);
*/
static atomic_t tracers = ATOMIC_INIT(0);
-static inline void get_tracer(struct task_struct *task)
+static inline int get_tracer(struct task_struct *task)
{
- if (task)
+ int error;
+
+ spin_lock_irq(&ds_lock);
+
+ if (task) {
+ error = -EPERM;
+ if (atomic_read(&tracers) < 0)
+ goto out;
atomic_inc(&tracers);
- else
+ } else {
+ error = -EPERM;
+ if (atomic_read(&tracers) > 0)
+ goto out;
atomic_dec(&tracers);
+ }
+
+ error = 0;
+out:
+ spin_unlock_irq(&ds_lock);
+ return error;
}
static inline void put_tracer(struct task_struct *task)
@@ -209,14 +225,6 @@ static inline void put_tracer(struct tas
atomic_inc(&tracers);
}
-static inline int check_tracer(struct task_struct *task)
-{
- return task ?
- (atomic_read(&tracers) >= 0) :
- (atomic_read(&tracers) <= 0);
-}
-
-
/*
* The DS context is either attached to a thread or to a cpu:
* - in the former case, the thread_struct contains a pointer to the
@@ -677,6 +685,10 @@ struct bts_tracer *ds_request_bts(struct
if (ovfl)
goto out;
+ error = get_tracer(task);
+ if (error < 0)
+ goto out;
+
/*
* Per-cpu tracing is typically requested using smp_call_function().
* We must not sleep.
@@ -684,7 +696,7 @@ struct bts_tracer *ds_request_bts(struct
error = -ENOMEM;
tracer = kzalloc(sizeof(*tracer), GFP_ATOMIC);
if (!tracer)
- goto out;
+ goto out_put_tracer;
tracer->ovfl = ovfl;
error = ds_request(&tracer->ds, &tracer->trace.ds,
@@ -696,13 +708,8 @@ struct bts_tracer *ds_request_bts(struct
spin_lock_irqsave(&ds_lock, irq);
error = -EPERM;
- if (!check_tracer(task))
- goto out_unlock;
- get_tracer(task);
-
- error = -EPERM;
if (tracer->ds.context->bts_master)
- goto out_put_tracer;
+ goto out_unlock;
tracer->ds.context->bts_master = tracer;
spin_unlock_irqrestore(&ds_lock, irq);
@@ -716,13 +723,13 @@ struct bts_tracer *ds_request_bts(struct
return tracer;
- out_put_tracer:
- put_tracer(task);
out_unlock:
spin_unlock_irqrestore(&ds_lock, irq);
ds_put_context(tracer->ds.context);
out_tracer:
kfree(tracer);
+ out_put_tracer:
+ put_tracer(task);
out:
return ERR_PTR(error);
}
@@ -741,6 +748,10 @@ struct pebs_tracer *ds_request_pebs(stru
if (ovfl)
goto out;
+ error = get_tracer(task);
+ if (error < 0)
+ goto out;
+
/*
* Per-cpu tracing is typically requested using smp_call_function().
* We must not sleep.
@@ -748,7 +759,7 @@ struct pebs_tracer *ds_request_pebs(stru
error = -ENOMEM;
tracer = kzalloc(sizeof(*tracer), GFP_ATOMIC);
if (!tracer)
- goto out;
+ goto out_put_tracer;
tracer->ovfl = ovfl;
error = ds_request(&tracer->ds, &tracer->trace.ds,
@@ -759,13 +770,8 @@ struct pebs_tracer *ds_request_pebs(stru
spin_lock_irqsave(&ds_lock, irq);
error = -EPERM;
- if (!check_tracer(task))
- goto out_unlock;
- get_tracer(task);
-
- error = -EPERM;
if (tracer->ds.context->pebs_master)
- goto out_put_tracer;
+ goto out_unlock;
tracer->ds.context->pebs_master = tracer;
spin_unlock_irqrestore(&ds_lock, irq);
@@ -775,13 +781,13 @@ struct pebs_tracer *ds_request_pebs(stru
return tracer;
- out_put_tracer:
- put_tracer(task);
out_unlock:
spin_unlock_irqrestore(&ds_lock, irq);
ds_put_context(tracer->ds.context);
out_tracer:
kfree(tracer);
+ out_put_tracer:
+ put_tracer(task);
out:
return ERR_PTR(error);
}
@@ -804,8 +810,8 @@ void ds_release_bts(struct bts_tracer *t
if (task && (task != current))
wait_task_context_switch(task);
- put_tracer(task);
ds_put_context(tracer->ds.context);
+ put_tracer(task);
kfree(tracer);
}
@@ -861,16 +867,20 @@ void ds_resume_bts(struct bts_tracer *tr
void ds_release_pebs(struct pebs_tracer *tracer)
{
+ struct task_struct *task;
+
if (!tracer)
return;
+ task = tracer->ds.context->task;
+
ds_suspend_pebs(tracer);
WARN_ON_ONCE(tracer->ds.context->pebs_master != tracer);
tracer->ds.context->pebs_master = NULL;
- put_tracer(tracer->ds.context->task);
ds_put_context(tracer->ds.context);
+ put_tracer(task);
kfree(tracer);
}
--
[-- Attachment #2: Type: text/plain, Size: 656 bytes --]
---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
next prev parent reply other threads:[~2009-04-03 14:48 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-03 14:43 [patch 00/20] x86, ptrace, bts, hw-branch-tracer: fixes and cleanups markus.t.metzger
2009-04-03 14:43 ` [patch 01/20] x86, bts: fix race when bts tracer is removed markus.t.metzger
2009-05-07 9:21 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 02/20] sched: add wait_task_context_switch() fucntion to sched.h markus.t.metzger
2009-05-07 9:21 ` [tip:tracing/core] sched, hw-branch-tracer: add wait_task_context_switch() function " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 03/20] x86, ptrace, bts: defer branch trace stopping markus.t.metzger
2009-04-03 15:00 ` Peter Zijlstra
2009-04-04 7:17 ` Metzger, Markus T
2009-04-04 11:12 ` Peter Zijlstra
2009-04-07 8:12 ` Metzger, Markus T
2009-04-07 8:29 ` Peter Zijlstra
2009-04-07 9:09 ` Metzger, Markus T
2009-05-07 9:21 ` [tip:tracing/core] mm, " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 04/20] x86, bts: wait until traced task has been scheduled out markus.t.metzger
2009-05-07 9:22 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` markus.t.metzger [this message]
2009-05-07 9:22 ` [tip:tracing/core] x86, bts: fix race between per-task and per-cpu branch tracing tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 06/20] x86, bts: use trace_clock_global() for timestamps markus.t.metzger
2009-05-07 9:22 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 07/20] x86, debugctlmsr: add _on_cpu variants to debugctlmsr functions markus.t.metzger
2009-05-07 9:22 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 08/20] x86, bts, hw-branch-tracer: add _noirq variants to the debug store interface markus.t.metzger
2009-05-07 9:22 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 09/20] x86, hw-branch-tracer: allocate selftest iterator on heap markus.t.metzger
2009-05-07 9:23 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 10/20] x86, ds: fix compiler warning markus.t.metzger
2009-05-07 9:23 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 11/20] x86, ds: fix bounds check in ds selftest markus.t.metzger
2009-05-07 9:23 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 12/20] x86, ds: selftest each cpu markus.t.metzger
2009-05-07 9:23 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 13/20] x86, ds: add task tracing selftest markus.t.metzger
2009-05-07 9:23 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 14/20] x86, ds: add leakage warning markus.t.metzger
2009-05-07 9:24 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 15/20] x86, ds: use single debug store cpu configuration markus.t.metzger
2009-05-07 9:24 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 16/20] x86, ptrace: add bts context unconditionally markus.t.metzger
2009-05-07 9:24 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 17/20] x86, ds: dont use TIF_DEBUGCTLMSR markus.t.metzger
2009-05-07 9:24 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 18/20] x86, ds: fix bad ds_reset_pebs() markus.t.metzger
2009-05-07 9:24 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 19/20] x86, ds: allow small debug store buffers markus.t.metzger
2009-05-07 9:24 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 14:43 ` [patch 20/20] x86, ds: support Core i7 markus.t.metzger
2009-05-07 9:25 ` [tip:tracing/core] " tip-bot for Markus Metzger
2009-04-03 15:36 ` [patch 00/20] x86, ptrace, bts, hw-branch-tracer: fixes and cleanups Ingo Molnar
2009-04-03 17:44 ` Markus Metzger
2009-04-03 17:48 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090403144552.856000000@intel.com \
--to=markus.t.metzger@intel.com \
--cc=ak@linux.jf.intel.com \
--cc=eranian@googlemail.com \
--cc=hpa@zytor.com \
--cc=juan.villacis@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=markus.t.metzger@gmail.com \
--cc=mingo@elte.hu \
--cc=oleg@redhat.com \
--cc=roland@redhat.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox