| Submitter | Frederic Weisbecker |
|---|---|
| Date | 2009-11-03 19:11:13 |
| Message ID | <1257275474-5285-6-git-send-email-fweisbec@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/57359/ |
| State | New |
| Headers | show |
Comments
Frederic Weisbecker writes: > Allow or refuse to build a counter using the breakpoints pmu following > given constraints. As far as I can see, you assume each CPU has HBP_NUM breakpoint registers which are all interchangeable and can all be used either for data breakpoints or instruction breakpoints. Is that accurate? If so, we'll need to extend it a bit for Power since we have some CPUs that have one data breakpoint register and one instruction breakpoint register. In general on powerpc the instruction and data breakpoint facilities are separate, i.e. we have no registers that can be used for either. > +static void toggle_bp_slot(struct perf_event *bp, bool enable) > +{ > + int cpu = bp->cpu; > + unsigned int *nr; > + struct task_struct *tsk = bp->ctx->task; > + > + /* Flexible */ > + if (!bp->attr.pinned) { > + if (cpu >= 0) { > + nr = &per_cpu(nr_bp_flexible, cpu); > + goto toggle; > + } > + > + for_each_online_cpu(cpu) { > + nr = &per_cpu(nr_bp_flexible, cpu); > + goto toggle; ... > +toggle: > + *nr = enable ? *nr + 1 : *nr - 1; > +} This won't do what I think you want. In the case where !bp->attr.pinned and cpu == -1, it will only update the count for the first online cpu, not all of them. Paul. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Thu, Nov 05, 2009 at 09:58:30PM +1100, Paul Mackerras wrote: > Frederic Weisbecker writes: > > > Allow or refuse to build a counter using the breakpoints pmu following > > given constraints. > > As far as I can see, you assume each CPU has HBP_NUM breakpoint > registers which are all interchangeable and can all be used either for > data breakpoints or instruction breakpoints. Is that accurate? Yes, they are interchangeable at runtime while calling enable/disable callbacks of the pmu. I'm not sure instruction breakpoints are supported though. > If so, we'll need to extend it a bit for Power since we have some CPUs > that have one data breakpoint register and one instruction breakpoint > register. In general on powerpc the instruction and data breakpoint > facilities are separate, i.e. we have no registers that can be used > for either. Sure. I would be glad to help in that area. That said I won't be able to test anything as I don't have a PowerPc box. > > +static void toggle_bp_slot(struct perf_event *bp, bool enable) > > +{ > > + int cpu = bp->cpu; > > + unsigned int *nr; > > + struct task_struct *tsk = bp->ctx->task; > > + > > + /* Flexible */ > > + if (!bp->attr.pinned) { > > + if (cpu >= 0) { > > + nr = &per_cpu(nr_bp_flexible, cpu); > > + goto toggle; > > + } > > + > > + for_each_online_cpu(cpu) { > > + nr = &per_cpu(nr_bp_flexible, cpu); > > + goto toggle; > > ... > > > +toggle: > > + *nr = enable ? *nr + 1 : *nr - 1; > > +} > > This won't do what I think you want. In the case where > !bp->attr.pinned and cpu == -1, it will only update the count for the > first online cpu, not all of them. > > Paul. Oh right! That's really idiotic. Will fix. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Thu, 2009-11-05 at 21:58 +1100, Paul Mackerras wrote: > Frederic Weisbecker writes: > > > Allow or refuse to build a counter using the breakpoints pmu following > > given constraints. > > As far as I can see, you assume each CPU has HBP_NUM breakpoint > registers which are all interchangeable and can all be used either for > data breakpoints or instruction breakpoints. Is that accurate? > > If so, we'll need to extend it a bit for Power since we have some CPUs > that have one data breakpoint register and one instruction breakpoint > register. In general on powerpc the instruction and data breakpoint > facilities are separate, i.e. we have no registers that can be used > for either. Additionally, we have more fancy facilities that I don't see exposed at all through this interface (we are building an ad-hoc ptrace based interface today so that gdb can make use of them) and we have one guy with crazy constraints that we don't know yet how to deal with: Among others features: - Pairing of two data or instruction breakpoints to create a ranges breakpoint - Data value compare option - Instruction value compare option And now the crazy constraints: - On one embedded core at least we have a case where the core has 4 threads, but the data (4) and instruction (2) breakpoint registers are shared. The 'enable' bits are split so a given data breakpoint can be enabled only on some HW threads but that's about it. I'm not sure if there's a realistic way to handle the later constraint though other than just not allowing use of the HW breakpoint function on those cores at all. Ben. > > +static void toggle_bp_slot(struct perf_event *bp, bool enable) > > +{ > > + int cpu = bp->cpu; > > + unsigned int *nr; > > + struct task_struct *tsk = bp->ctx->task; > > + > > + /* Flexible */ > > + if (!bp->attr.pinned) { > > + if (cpu >= 0) { > > + nr = &per_cpu(nr_bp_flexible, cpu); > > + goto toggle; > > + } > > + > > + for_each_online_cpu(cpu) { > > + nr = &per_cpu(nr_bp_flexible, cpu); > > + goto toggle; > > ... > > > +toggle: > > + *nr = enable ? *nr + 1 : *nr - 1; > > +} > > This won't do what I think you want. In the case where > !bp->attr.pinned and cpu == -1, it will only update the count for the > first online cpu, not all of them. > > Paul. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, Nov 09, 2009 at 07:56:21AM +1100, Benjamin Herrenschmidt wrote: > On Thu, 2009-11-05 at 21:58 +1100, Paul Mackerras wrote: > > Frederic Weisbecker writes: > > > > > Allow or refuse to build a counter using the breakpoints pmu following > > > given constraints. > > > > As far as I can see, you assume each CPU has HBP_NUM breakpoint > > registers which are all interchangeable and can all be used either for > > data breakpoints or instruction breakpoints. Is that accurate? > > > > If so, we'll need to extend it a bit for Power since we have some CPUs > > that have one data breakpoint register and one instruction breakpoint > > register. In general on powerpc the instruction and data breakpoint > > facilities are separate, i.e. we have no registers that can be used > > for either. > > Additionally, we have more fancy facilities that I don't see exposed at > all through this interface (we are building an ad-hoc ptrace based > interface today so that gdb can make use of them) and we have one guy > with crazy constraints that we don't know yet how to deal with: > > Among others features: > > - Pairing of two data or instruction breakpoints to create a ranges > breakpoint > - Data value compare option > - Instruction value compare option Yeah. The current generic interface is a draft. I'll try to write something generic enough to fit in every archs needs. This is needed before we expose its perf interface to userpace anyway. > And now the crazy constraints: > > - On one embedded core at least we have a case where the core has 4 > threads, but the data (4) and instruction (2) breakpoint registers are > shared. The 'enable' bits are split so a given data breakpoint can be > enabled only on some HW threads but that's about it. > > I'm not sure if there's a realistic way to handle the later constraint > though other than just not allowing use of the HW breakpoint function on > those cores at all. > > Ben. Yeah this latter one is tricky. Not sure how to handle it either. How are these hw-threads considered by the kernel core? As different cpu? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Thu, 2009-11-12 at 16:54 +0100, Frederic Weisbecker wrote: > > - On one embedded core at least we have a case where the core has 4 > > threads, but the data (4) and instruction (2) breakpoint registers are > > shared. The 'enable' bits are split so a given data breakpoint can be > > enabled only on some HW threads but that's about it. > > > > I'm not sure if there's a realistic way to handle the later constraint > > though other than just not allowing use of the HW breakpoint function on > > those cores at all. > > > > Ben. > > > Yeah this latter one is tricky. Not sure how to handle it either. > How are these hw-threads considered by the kernel core? As different > cpu? Yes. So it basically looks like you have 4 data and 2 HW instruction breakpoint registers shared by 4 CPUs in a group :-) Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, Nov 13, 2009 at 07:00:38AM +1100, Benjamin Herrenschmidt wrote: > On Thu, 2009-11-12 at 16:54 +0100, Frederic Weisbecker wrote: > > > - On one embedded core at least we have a case where the core has 4 > > > threads, but the data (4) and instruction (2) breakpoint registers are > > > shared. The 'enable' bits are split so a given data breakpoint can be > > > enabled only on some HW threads but that's about it. > > > > > > I'm not sure if there's a realistic way to handle the later constraint > > > though other than just not allowing use of the HW breakpoint function on > > > those cores at all. > > > > > > Ben. > > > > > > Yeah this latter one is tricky. Not sure how to handle it either. > > How are these hw-threads considered by the kernel core? As different > > cpu? > > Yes. > > So it basically looks like you have 4 data and 2 HW instruction breakpoint > registers shared by 4 CPUs in a group :-) > > Cheers, > Ben. > > That's not a simple situation :) I guess we'll need to let powerpc handle the constraints from the arch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Patch
diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c index 08f6d01..60ff182 100644 --- a/kernel/hw_breakpoint.c +++ b/kernel/hw_breakpoint.c @@ -16,6 +16,8 @@ * Copyright (C) 2007 Alan Stern * Copyright (C) IBM Corporation, 2009 * Copyright (C) 2009, Frederic Weisbecker <fweisbec@gmail.com> + * + * Thanks to Ingo Molnar for his many suggestions. */ /* @@ -44,24 +46,247 @@ #include <asm/debugreg.h> #endif -static atomic_t bp_slot; +/* + * Constraints data + */ + +/* Number of pinned cpu breakpoints in a cpu */ +static DEFINE_PER_CPU(unsigned int, nr_cpu_bp_pinned); -int reserve_bp_slot(struct perf_event *bp) +/* Number of pinned task breakpoints in a cpu */ +static DEFINE_PER_CPU(unsigned int, task_bp_pinned[HBP_NUM]); + +/* Number of non-pinned cpu/task breakpoints in a cpu */ +static DEFINE_PER_CPU(unsigned int, nr_bp_flexible); + +/* Gather the number of total pinned and un-pinned bp in a cpuset */ +struct bp_busy_slots { + unsigned int pinned; + unsigned int flexible; +}; + +/* Serialize accesses to the above constraints */ +static DEFINE_MUTEX(nr_bp_mutex); + +/* + * Report the maximum number of pinned breakpoints a task + * have in this cpu + */ +static unsigned int max_task_bp_pinned(int cpu) { - if (atomic_inc_return(&bp_slot) == HBP_NUM) { - atomic_dec(&bp_slot); + int i; + unsigned int *tsk_pinned = per_cpu(task_bp_pinned, cpu); - return -ENOSPC; + for (i = HBP_NUM -1; i >= 0; i--) { + if (tsk_pinned[i] > 0) + return i + 1; } return 0; } +/* + * Report the number of pinned/un-pinned breakpoints we have in + * a given cpu (cpu > -1) or in all of them (cpu = -1). + */ +static void fetch_bp_busy_slots(struct bp_busy_slots *slots, int cpu) +{ + if (cpu >= 0) { + slots->pinned = per_cpu(nr_cpu_bp_pinned, cpu); + slots->pinned += max_task_bp_pinned(cpu); + slots->flexible = per_cpu(nr_bp_flexible, cpu); + + return; + } + + for_each_online_cpu(cpu) { + unsigned int nr; + + nr = per_cpu(nr_cpu_bp_pinned, cpu); + nr += max_task_bp_pinned(cpu); + + if (nr > slots->pinned) + slots->pinned = nr; + + nr = per_cpu(nr_bp_flexible, cpu); + + if (nr > slots->flexible) + slots->flexible = nr; + } +} + +/* + * Add a pinned breakpoint for the given task in our constraint table + */ +static void toggle_bp_task_slot(struct task_struct *tsk, int cpu, bool enable) +{ + int count = 0; + struct perf_event *bp; + struct perf_event_context *ctx = tsk->perf_event_ctxp; + unsigned int *task_bp_pinned; + struct list_head *list; + unsigned long flags; + + if (WARN_ONCE(!ctx, "No perf context for this task")) + return; + + list = &ctx->event_list; + + spin_lock_irqsave(&ctx->lock, flags); + + /* + * The current breakpoint counter is not included in the list + * at the open() callback time + */ + list_for_each_entry(bp, list, event_entry) { + if (bp->attr.type == PERF_TYPE_BREAKPOINT) + count++; + } + + spin_unlock_irqrestore(&ctx->lock, flags); + + if (WARN_ONCE(count < 0, "No breakpoint counter found in the counter list")) + return; + + task_bp_pinned = per_cpu(task_bp_pinned, cpu); + if (enable) { + task_bp_pinned[count]++; + if (count > 0) + task_bp_pinned[count-1]--; + } else { + task_bp_pinned[count]--; + if (count > 0) + task_bp_pinned[count-1]++; + } +} + +/* + * Add/remove the given breakpoint in our constraint table + */ +static void toggle_bp_slot(struct perf_event *bp, bool enable) +{ + int cpu = bp->cpu; + unsigned int *nr; + struct task_struct *tsk = bp->ctx->task; + + /* Flexible */ + if (!bp->attr.pinned) { + if (cpu >= 0) { + nr = &per_cpu(nr_bp_flexible, cpu); + goto toggle; + } + + for_each_online_cpu(cpu) { + nr = &per_cpu(nr_bp_flexible, cpu); + goto toggle; + } + } + /* Pinned counter task profiling */ + if (tsk) { + if (cpu >= 0) { + toggle_bp_task_slot(tsk, cpu, enable); + return; + } + + for_each_online_cpu(cpu) + toggle_bp_task_slot(tsk, cpu, enable); + return; + } + + /* Pinned counter cpu profiling */ + nr = &per_cpu(nr_cpu_bp_pinned, bp->cpu); + +toggle: + *nr = enable ? *nr + 1 : *nr - 1; +} + +/* + * Contraints to check before allowing this new breakpoint counter: + * + * == Non-pinned counter == + * + * - If attached to a single cpu, check: + * + * (per_cpu(nr_bp_flexible, cpu) || (per_cpu(nr_cpu_bp_pinned, cpu) + * + max(per_cpu(task_bp_pinned, cpu)))) < HBP_NUM + * + * -> If there are already non-pinned counters in this cpu, it means + * there is already a free slot for them. + * Otherwise, we check that the maximum number of per task + * breakpoints (for this cpu) plus the number of per cpu breakpoint + * (for this cpu) doesn't cover every registers. + * + * - If attached to every cpus, check: + * + * (per_cpu(nr_bp_flexible, *) || (max(per_cpu(nr_cpu_bp_pinned, *)) + * + max(per_cpu(task_bp_pinned, *)))) < HBP_NUM + * + * -> This is roughly the same, except we check the number of per cpu + * bp for every cpu and we keep the max one. Same for the per tasks + * breakpoints. + * + * + * == Pinned counter == + * + * - If attached to a single cpu, check: + * + * ((per_cpu(nr_bp_flexible, cpu) > 1) + per_cpu(nr_cpu_bp_pinned, cpu) + * + max(per_cpu(task_bp_pinned, cpu))) < HBP_NUM + * + * -> Same checks as before. But now the nr_bp_flexible, if any, must keep + * one register at least (or they will never be fed). + * + * - If attached to every cpus, check: + * + * ((per_cpu(nr_bp_flexible, *) > 1) + max(per_cpu(nr_cpu_bp_pinned, *)) + * + max(per_cpu(task_bp_pinned, *))) < HBP_NUM + */ +int reserve_bp_slot(struct perf_event *bp) +{ + struct bp_busy_slots slots = {0}; + int ret = 0; + + mutex_lock(&nr_bp_mutex); + + fetch_bp_busy_slots(&slots, bp->cpu); + + if (!bp->attr.pinned) { + /* + * If there are already flexible counters here, + * there is at least one slot reserved for all + * of them. Just join the party. + * + * Otherwise, check there is at least one free slot + */ + if (!slots.flexible && slots.pinned == HBP_NUM) { + ret = -ENOSPC; + goto end; + } + + /* Flexible counters need to keep at least one slot */ + } else if (slots.pinned + (!!slots.flexible) == HBP_NUM) { + ret = -ENOSPC; + goto end; + } + + toggle_bp_slot(bp, true); + +end: + mutex_unlock(&nr_bp_mutex); + + return ret; +} + void release_bp_slot(struct perf_event *bp) { - atomic_dec(&bp_slot); + mutex_lock(&nr_bp_mutex); + + toggle_bp_slot(bp, false); + + mutex_unlock(&nr_bp_mutex); } + int __register_perf_hw_breakpoint(struct perf_event *bp) { int ret;
Allow or refuse to build a counter using the breakpoints pmu following given constraints. We keep track of the pmu users by using three per cpu variables: - nr_cpu_bp_pinned stores the number of pinned cpu breakpoints counters in the given cpu - nr_bp_flexible stores the number of non-pinned breakpoints counters in the given cpu. - task_bp_pinned stores the number of pinned task breakpoints in a cpu The latter is not a simple counter but gathers the number of tasks that have n pinned breakpoints. Considering HBP_NUM the number of available breakpoint address registers: task_bp_pinned[0] is the number of tasks having 1 breakpoint task_bp_pinned[1] is the number of tasks having 2 breakpoints [...] task_bp_pinned[HBP_NUM - 1] is the number of tasks having the maximum number of registers (HBP_NUM). When a breakpoint counter is created and wants an access to the pmu, we evaluate the following constraints: == Non-pinned counter == - If attached to a single cpu, check: (per_cpu(nr_bp_flexible, cpu) || (per_cpu(nr_cpu_bp_pinned, cpu) + max(per_cpu(task_bp_pinned, cpu)))) < HBP_NUM -> If there are already non-pinned counters in this cpu, it means there is already a free slot for them. Otherwise, we check that the maximum number of per task breakpoints (for this cpu) plus the number of per cpu breakpoint (for this cpu) doesn't cover every registers. - If attached to every cpus, check: (per_cpu(nr_bp_flexible, *) || (max(per_cpu(nr_cpu_bp_pinned, *)) + max(per_cpu(task_bp_pinned, *)))) < HBP_NUM -> This is roughly the same, except we check the number of per cpu bp for every cpu and we keep the max one. Same for the per tasks breakpoints. == Pinned counter == - If attached to a single cpu, check: ((per_cpu(nr_bp_flexible, cpu) > 1) + per_cpu(nr_cpu_bp_pinned, cpu) + max(per_cpu(task_bp_pinned, cpu))) < HBP_NUM -> Same checks as before. But now the nr_bp_flexible, if any, must keep one register at least (or flexible breakpoints will never be be fed). - If attached to every cpus, check: ((per_cpu(nr_bp_flexible, *) > 1) + max(per_cpu(nr_cpu_bp_pinned, *)) + max(per_cpu(task_bp_pinned, *))) < HBP_NUM Changes in v2: - Counter -> event rename Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com> Cc: Prasad <prasad@linux.vnet.ibm.com> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Ingo Molnar <mingo@elte.hu> Cc: Jan Kiszka <jan.kiszka@web.de> Cc: Jiri Slaby <jirislaby@gmail.com> Cc: Li Zefan <lizf@cn.fujitsu.com> Cc: Avi Kivity <avi@redhat.com> Cc: Paul Mackerras <paulus@samba.org> Cc: Mike Galbraith <efault@gmx.de> Cc: Masami Hiramatsu <mhiramat@redhat.com> Cc: Paul Mundt <lethal@linux-sh.org> --- kernel/hw_breakpoint.c | 237 ++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 231 insertions(+), 6 deletions(-)