Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH 4/9] perf: Generic intel uncore support

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


zheng.z.yan at intel

May 1, 2012, 7:07 PM

Post #1 of 10 (126 views)
Permalink
[PATCH 4/9] perf: Generic intel uncore support

From: "Yan, Zheng" <zheng.z.yan [at] intel>

This patch adds the generic intel uncore pmu support, including helper
functions that add/delete uncore events, a hrtimer that periodically
polls the counters to avoid overflow and code that places all events
for a particular socket onto a single cpu. The code design is based on
the structure of Sandy Bridge-EP's uncore subsystem, which consists of
a variety of components, each component contain one or more boxes.

Signed-off-by: Zheng Yan <zheng.z.yan [at] intel>
---
arch/x86/kernel/cpu/Makefile | 2 +-
arch/x86/kernel/cpu/perf_event_intel_uncore.c | 880 +++++++++++++++++++++++++
arch/x86/kernel/cpu/perf_event_intel_uncore.h | 205 ++++++
3 files changed, 1086 insertions(+), 1 deletions(-)
create mode 100644 arch/x86/kernel/cpu/perf_event_intel_uncore.c
create mode 100644 arch/x86/kernel/cpu/perf_event_intel_uncore.h

diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 6ab6aa2..9dfa9e9 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_PERF_EVENTS) += perf_event.o

ifdef CONFIG_PERF_EVENTS
obj-$(CONFIG_CPU_SUP_AMD) += perf_event_amd.o
-obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o
+obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o perf_event_intel_uncore.o
endif

obj-$(CONFIG_X86_MCE) += mcheck/
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
new file mode 100644
index 0000000..0dda34e
--- /dev/null
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -0,0 +1,880 @@
+#include "perf_event_intel_uncore.h"
+
+static struct intel_uncore_type *empty_uncore[] = { NULL, };
+static struct intel_uncore_type **msr_uncores = empty_uncore;
+
+/* mask of cpus that collect uncore events */
+static cpumask_t uncore_cpu_mask;
+
+/* constraint for the fixed countesr */
+static struct event_constraint constraint_fixed =
+ EVENT_CONSTRAINT((u64)-1, 1 << UNCORE_PMC_IDX_FIXED, (u64)-1);
+
+static void uncore_assign_hw_event(struct intel_uncore_box *box,
+ struct perf_event *event, int idx)
+{
+ struct hw_perf_event *hwc = &event->hw;
+
+ hwc->idx = idx;
+ hwc->last_tag = ++box->tags[idx];
+
+ if (hwc->idx == UNCORE_PMC_IDX_FIXED) {
+ hwc->event_base = uncore_msr_fixed_ctr(box);
+ hwc->config_base = uncore_msr_fixed_ctl(box);
+ return;
+ }
+
+ hwc->config_base = uncore_msr_event_ctl(box, hwc->idx);
+ hwc->event_base = uncore_msr_perf_ctr(box, hwc->idx);
+}
+
+static void uncore_perf_event_update(struct intel_uncore_box *box,
+ struct perf_event *event)
+{
+ u64 prev_count, new_count, delta;
+ int shift;
+
+ if (event->hw.idx >= UNCORE_PMC_IDX_FIXED)
+ shift = 64 - uncore_fixed_ctr_bits(box);
+ else
+ shift = 64 - uncore_perf_ctr_bits(box);
+
+ /* the hrtimer might modify the previous event value */
+again:
+ prev_count = local64_read(&event->hw.prev_count);
+ new_count = uncore_read_counter(box, event);
+ if (local64_xchg(&event->hw.prev_count, new_count) != prev_count)
+ goto again;
+
+ delta = (new_count << shift) - (prev_count << shift);
+ delta >>= shift;
+
+ local64_add(delta, &event->count);
+}
+
+/*
+ * The overflow interrupt is unavailable for SandyBridge-EP, is broken
+ * for SandyBridge. So we use hrtimer to periodically poll the counter
+ * to avoid overflow.
+ */
+static enum hrtimer_restart uncore_pmu_hrtimer(struct hrtimer *hrtimer)
+{
+ struct intel_uncore_box *box;
+ unsigned long flags;
+ int bit;
+
+ box = container_of(hrtimer, struct intel_uncore_box, hrtimer);
+ if (!box->n_active || box->cpu != smp_processor_id())
+ return HRTIMER_NORESTART;
+ /*
+ * disable local interrupt to prevent uncore_pmu_event_start/stop
+ * to interrupt the update process
+ */
+ local_irq_save(flags);
+
+ for_each_set_bit(bit, box->active_mask, UNCORE_PMC_IDX_MAX)
+ uncore_perf_event_update(box, box->events[bit]);
+
+ local_irq_restore(flags);
+
+ hrtimer_forward_now(hrtimer, ns_to_ktime(UNCORE_PMU_HRTIMER_INTERVAL));
+ return HRTIMER_RESTART;
+}
+
+static void uncore_pmu_start_hrtimer(struct intel_uncore_box *box)
+{
+ __hrtimer_start_range_ns(&box->hrtimer,
+ ns_to_ktime(UNCORE_PMU_HRTIMER_INTERVAL), 0,
+ HRTIMER_MODE_REL_PINNED, 0);
+}
+
+static void uncore_pmu_cancel_hrtimer(struct intel_uncore_box *box)
+{
+ hrtimer_cancel(&box->hrtimer);
+}
+
+static void uncore_pmu_init_hrtimer(struct intel_uncore_box *box)
+{
+ hrtimer_init(&box->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ box->hrtimer.function = uncore_pmu_hrtimer;
+}
+
+struct intel_uncore_box *uncore_alloc_box(int cpu)
+{
+ struct intel_uncore_box *box;
+
+ box = kmalloc_node(sizeof(*box), GFP_KERNEL | __GFP_ZERO,
+ cpu_to_node(cpu));
+ if (!box)
+ return NULL;
+
+ uncore_pmu_init_hrtimer(box);
+ box->cpu = -1;
+ box->refcnt = 1;
+
+ return box;
+}
+
+static struct intel_uncore_box *
+__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
+{
+ struct intel_uncore_box *box;
+ struct hlist_head *head;
+ struct hlist_node *node;
+
+ head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
+ hlist_for_each_entry_rcu(box, node, head, hlist) {
+ if (box->phy_id == phyid)
+ return box;
+ }
+
+ return NULL;
+}
+
+static struct intel_uncore_box *
+uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
+{
+ struct intel_uncore_box *box;
+
+ rcu_read_lock();
+ box = __uncore_pmu_find_box(pmu, phyid);
+ rcu_read_unlock();
+
+ return box;
+}
+
+static void uncore_pmu_add_box(struct intel_uncore_pmu *pmu,
+ struct intel_uncore_box *box)
+{
+ struct hlist_head *head;
+
+ head = &pmu->box_hash[box->phy_id % UNCORE_BOX_HASH_SIZE];
+ hlist_add_head_rcu(&box->hlist, head);
+}
+
+static struct intel_uncore_pmu *uncore_event_to_pmu(struct perf_event *event)
+{
+ return container_of(event->pmu, struct intel_uncore_pmu, pmu);
+}
+
+static struct intel_uncore_box *uncore_event_to_box(struct perf_event *event)
+{
+ /*
+ * perf core schedules event on the basis of cpu, uncore events are
+ * collected by one of the cpus inside a physical package.
+ */
+ int phyid = topology_physical_package_id(smp_processor_id());
+ return uncore_pmu_find_box(uncore_event_to_pmu(event), phyid);
+}
+
+static int uncore_collect_events(struct intel_uncore_box *box,
+ struct perf_event *leader, bool dogrp)
+{
+ struct perf_event *event;
+ int n, max_count;
+
+ max_count = box->pmu->type->num_counters;
+ if (box->pmu->type->fixed_ctl)
+ max_count++;
+
+ if (box->n_events >= max_count)
+ return -EINVAL;
+
+ n = box->n_events;
+ box->event_list[n] = leader;
+ n++;
+ if (!dogrp)
+ return n;
+
+ list_for_each_entry(event, &leader->sibling_list, group_entry) {
+ if (event->state <= PERF_EVENT_STATE_OFF)
+ continue;
+
+ if (n >= max_count)
+ return -EINVAL;
+
+ box->event_list[n] = event;
+ n++;
+ }
+ return n;
+}
+
+static struct event_constraint *
+uncore_event_constraint(struct intel_uncore_type *type,
+ struct perf_event *event)
+{
+ struct event_constraint *c;
+
+ if (event->hw.config == (u64)-1)
+ return &constraint_fixed;
+
+ if (type->constraints) {
+ for_each_event_constraint(c, type->constraints) {
+ if ((event->hw.config & c->cmask) == c->code)
+ return c;
+ }
+ }
+
+ return &type->unconstrainted;
+}
+
+static int uncore_assign_events(struct intel_uncore_box *box,
+ int assign[], int n)
+{
+ unsigned long used_mask[BITS_TO_LONGS(UNCORE_PMC_IDX_MAX)];
+ struct event_constraint *c, *constraints[UNCORE_PMC_IDX_MAX];
+ int i, ret, wmin, wmax;
+ struct hw_perf_event *hwc;
+
+ bitmap_zero(used_mask, UNCORE_PMC_IDX_MAX);
+
+ for (i = 0, wmin = UNCORE_PMC_IDX_MAX, wmax = 0; i < n; i++) {
+ c = uncore_event_constraint(box->pmu->type,
+ box->event_list[i]);
+ constraints[i] = c;
+ wmin = min(wmin, c->weight);
+ wmax = max(wmax, c->weight);
+ }
+
+ /* fastpath, try to reuse previous register */
+ for (i = 0; i < n; i++) {
+ hwc = &box->event_list[i]->hw;
+ c = constraints[i];
+
+ /* never assigned */
+ if (hwc->idx == -1)
+ break;
+
+ /* constraint still honored */
+ if (!test_bit(hwc->idx, c->idxmsk))
+ break;
+
+ /* not already used */
+ if (test_bit(hwc->idx, used_mask))
+ break;
+
+ __set_bit(hwc->idx, used_mask);
+ assign[i] = hwc->idx;
+ }
+ if (i == n)
+ return 0;
+
+ /* slow path */
+ ret = perf_assign_events(constraints, n, wmin, wmax, assign);
+ return ret ? -EINVAL : 0;
+}
+
+static void uncore_pmu_event_start(struct perf_event *event, int flags)
+{
+ struct intel_uncore_box *box = uncore_event_to_box(event);
+ int idx = event->hw.idx;
+
+ if (WARN_ON_ONCE(!(event->hw.state & PERF_HES_STOPPED)))
+ return;
+
+ if (WARN_ON_ONCE(idx == -1 || idx >= UNCORE_PMC_IDX_MAX))
+ return;
+
+ event->hw.state = 0;
+ box->events[idx] = event;
+ box->n_active++;
+ __set_bit(idx, box->active_mask);
+
+ local64_set(&event->hw.prev_count, uncore_read_counter(box, event));
+ uncore_enable_event(box, event);
+
+ if (box->n_active == 1) {
+ uncore_enable_box(box);
+ uncore_pmu_start_hrtimer(box);
+ }
+}
+
+static void uncore_pmu_event_stop(struct perf_event *event, int flags)
+{
+ struct intel_uncore_box *box = uncore_event_to_box(event);
+ struct hw_perf_event *hwc = &event->hw;
+
+ if (__test_and_clear_bit(hwc->idx, box->active_mask)) {
+ uncore_disable_event(box, event);
+ box->n_active--;
+ box->events[hwc->idx] = NULL;
+ WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+ hwc->state |= PERF_HES_STOPPED;
+
+ if (box->n_active == 0) {
+ uncore_disable_box(box);
+ uncore_pmu_cancel_hrtimer(box);
+ }
+ }
+
+ if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
+ /*
+ * Drain the remaining delta count out of a event
+ * that we are disabling:
+ */
+ uncore_perf_event_update(box, event);
+ hwc->state |= PERF_HES_UPTODATE;
+ }
+}
+
+static int uncore_pmu_event_add(struct perf_event *event, int flags)
+{
+ struct intel_uncore_box *box = uncore_event_to_box(event);
+ struct hw_perf_event *hwc = &event->hw;
+ int assign[UNCORE_PMC_IDX_MAX];
+ int i, n, ret;
+
+ if (!box)
+ return -ENODEV;
+
+ ret = n = uncore_collect_events(box, event, false);
+ if (ret < 0)
+ return ret;
+
+ hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
+ if (!(flags & PERF_EF_START))
+ hwc->state |= PERF_HES_ARCH;
+
+ ret = uncore_assign_events(box, assign, n);
+ if (ret)
+ return ret;
+
+ /* save events moving to new counters */
+ for (i = 0; i < box->n_events; i++) {
+ event = box->event_list[i];
+ hwc = &event->hw;
+
+ if (hwc->idx == assign[i] &&
+ hwc->last_tag == box->tags[assign[i]])
+ continue;
+ /*
+ * Ensure we don't accidentally enable a stopped
+ * counter simply because we rescheduled.
+ */
+ if (hwc->state & PERF_HES_STOPPED)
+ hwc->state |= PERF_HES_ARCH;
+
+ uncore_pmu_event_stop(event, PERF_EF_UPDATE);
+ }
+
+ /* reprogram moved events into new counters */
+ for (i = 0; i < n; i++) {
+ event = box->event_list[i];
+ hwc = &event->hw;
+
+ if (hwc->idx != assign[i] ||
+ hwc->last_tag != box->tags[assign[i]])
+ uncore_assign_hw_event(box, event, assign[i]);
+ else if (i < box->n_events)
+ continue;
+
+ if (hwc->state & PERF_HES_ARCH)
+ continue;
+
+ uncore_pmu_event_start(event, 0);
+ }
+ box->n_events = n;
+
+ return 0;
+}
+
+static void uncore_pmu_event_del(struct perf_event *event, int flags)
+{
+ struct intel_uncore_box *box = uncore_event_to_box(event);
+ int i;
+
+ uncore_pmu_event_stop(event, PERF_EF_UPDATE);
+
+ for (i = 0; i < box->n_events; i++) {
+ if (event == box->event_list[i]) {
+ while (++i < box->n_events)
+ box->event_list[i - 1] = box->event_list[i];
+
+ --box->n_events;
+ break;
+ }
+ }
+
+ event->hw.idx = -1;
+ event->hw.last_tag = ~0ULL;
+}
+
+static void uncore_pmu_event_read(struct perf_event *event)
+{
+ struct intel_uncore_box *box = uncore_event_to_box(event);
+ uncore_perf_event_update(box, event);
+}
+
+/*
+ * validation ensures the group can be loaded onto the
+ * PMU if it was the only group available.
+ */
+static int uncore_validate_group(struct intel_uncore_pmu *pmu,
+ struct perf_event *event)
+{
+ struct perf_event *leader = event->group_leader;
+ struct intel_uncore_box *fake_box;
+ int assign[UNCORE_PMC_IDX_MAX];
+ int ret = -EINVAL, n;
+
+ fake_box = uncore_alloc_box(smp_processor_id());
+ if (!fake_box)
+ return -ENOMEM;
+
+ fake_box->pmu = pmu;
+ /*
+ * the event is not yet connected with its
+ * siblings therefore we must first collect
+ * existing siblings, then add the new event
+ * before we can simulate the scheduling
+ */
+ n = uncore_collect_events(fake_box, leader, true);
+ if (n < 0)
+ goto out;
+
+ fake_box->n_events = n;
+ n = uncore_collect_events(fake_box, event, false);
+ if (n < 0)
+ goto out;
+
+ fake_box->n_events = n;
+
+ ret = uncore_assign_events(fake_box, assign, n);
+out:
+ kfree(fake_box);
+ return ret;
+}
+
+int uncore_pmu_event_init(struct perf_event *event)
+{
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_box *box;
+ struct hw_perf_event *hwc = &event->hw;
+ int ret;
+
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ pmu = uncore_event_to_pmu(event);
+ /* no device found for this pmu */
+ if (pmu->func_id < 0)
+ return -ENOENT;
+
+ /*
+ * Uncore PMU does measure at all privilege level all the time.
+ * So it doesn't make sense to specify any exclude bits.
+ */
+ if (event->attr.exclude_user || event->attr.exclude_kernel ||
+ event->attr.exclude_hv || event->attr.exclude_idle)
+ return -EINVAL;
+
+ /* Sampling not supported yet */
+ if (hwc->sample_period)
+ return -EINVAL;
+
+ /*
+ * Place all uncore events for a particular physical package
+ * onto a single cpu
+ */
+ if (event->cpu < 0)
+ return -EINVAL;
+ box = uncore_pmu_find_box(pmu,
+ topology_physical_package_id(event->cpu));
+ if (!box || box->cpu < 0)
+ return -EINVAL;
+ event->cpu = box->cpu;
+
+ if (event->attr.config == UNCORE_FIXED_EVENT) {
+ /* no fixed counter */
+ if (!pmu->type->fixed_ctl)
+ return -EINVAL;
+ /*
+ * if there is only one fixed counter, only the first pmu
+ * can access the fixed counter
+ */
+ if (pmu->type->single_fixed && pmu->pmu_idx > 0)
+ return -EINVAL;
+ hwc->config = (u64)-1;
+ } else {
+ hwc->config = event->attr.config & pmu->type->event_mask;
+ }
+
+ event->hw.idx = -1;
+ event->hw.last_tag = ~0ULL;
+
+ if (event->group_leader != event)
+ ret = uncore_validate_group(pmu, event);
+ else
+ ret = 0;
+
+ return ret;
+}
+
+static int __init uncore_pmu_register(struct intel_uncore_pmu *pmu)
+{
+ int ret;
+
+ pmu->pmu = (struct pmu) {
+ .attr_groups = pmu->type->attr_groups,
+ .task_ctx_nr = perf_invalid_context,
+ .event_init = uncore_pmu_event_init,
+ .add = uncore_pmu_event_add,
+ .del = uncore_pmu_event_del,
+ .start = uncore_pmu_event_start,
+ .stop = uncore_pmu_event_stop,
+ .read = uncore_pmu_event_read,
+ };
+
+ if (pmu->type->num_boxes == 1) {
+ if (strlen(pmu->type->name) > 0)
+ sprintf(pmu->name, "uncore_%s", pmu->type->name);
+ else
+ sprintf(pmu->name, "uncore");
+ } else {
+ sprintf(pmu->name, "uncore_%s%d", pmu->type->name,
+ pmu->pmu_idx);
+ }
+
+ ret = perf_pmu_register(&pmu->pmu, pmu->name, -1);
+ return ret;
+}
+
+static void __init uncore_type_exit(struct intel_uncore_type *type)
+{
+ kfree(type->attr_groups[1]);
+ kfree(type->pmus);
+ type->attr_groups[1] = NULL;
+ type->pmus = NULL;
+}
+
+static int __init uncore_type_init(struct intel_uncore_type *type)
+{
+ struct intel_uncore_pmu *pmus;
+ struct attribute_group *events_group;
+ struct attribute **attrs;
+ int i, j;
+
+ pmus = kzalloc(sizeof(*pmus) * type->num_boxes, GFP_KERNEL);
+ if (!pmus)
+ return -ENOMEM;
+
+ type->unconstrainted = (struct event_constraint)
+ __EVENT_CONSTRAINT(0, (1ULL << type->num_counters) - 1,
+ 0, type->num_counters, 0);
+
+ for (i = 0; i < type->num_boxes; i++) {
+ pmus[i].func_id = -1;
+ pmus[i].pmu_idx = i;
+ pmus[i].type = type;
+
+ for (j = 0; j < ARRAY_SIZE(pmus[0].box_hash); j++)
+ INIT_HLIST_HEAD(&pmus[i].box_hash[j]);
+ }
+
+ if (type->event_descs) {
+ for (i = 0; type->event_descs[i].attr.attr.name; i++);
+
+ events_group = kzalloc(sizeof(struct attribute *) * (i + 1) +
+ sizeof(*events_group), GFP_KERNEL);
+ if (!events_group)
+ goto fail;
+
+ attrs = (struct attribute **)(events_group + 1);
+ events_group->name = "events";
+ events_group->attrs = attrs;
+
+ for (j = 0; j < i; j++)
+ attrs[j] = &type->event_descs[j].attr.attr;
+
+ type->attr_groups[1] = events_group;
+ }
+
+ type->pmus = pmus;
+ return 0;
+fail:
+ uncore_type_exit(type);
+ return -ENOMEM;
+}
+
+static int __init uncore_types_init(struct intel_uncore_type **types)
+{
+ int i, ret;
+
+ for (i = 0; types[i]; i++) {
+ ret = uncore_type_init(types[i]);
+ if (ret)
+ goto fail;
+ }
+ return 0;
+fail:
+ while (--i >= 0)
+ uncore_type_exit(types[i]);
+ return ret;
+}
+
+static void __cpuinit uncore_cpu_dying(int cpu)
+{
+ struct intel_uncore_type *type;
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_box *box;
+ int i, j, phyid;
+
+ phyid = topology_physical_package_id(cpu);
+
+ for (i = 0; msr_uncores[i]; i++) {
+ type = msr_uncores[i];
+ for (j = 0; j < type->num_boxes; j++) {
+ pmu = &type->pmus[j];
+ box = uncore_pmu_find_box(pmu, phyid);
+ if (box && --box->refcnt == 0) {
+ hlist_del_rcu(&box->hlist);
+ kfree_rcu(box, rcu_head);
+ }
+ }
+ }
+}
+
+static int __cpuinit uncore_cpu_starting(int cpu)
+{
+ struct intel_uncore_type *type;
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_box *box;
+ int i, j, phyid;
+
+ phyid = topology_physical_package_id(cpu);
+
+ for (i = 0; msr_uncores[i]; i++) {
+ type = msr_uncores[i];
+ for (j = 0; j < type->num_boxes; j++) {
+ pmu = &type->pmus[j];
+ box = uncore_pmu_find_box(pmu, phyid);
+ if (box)
+ uncore_box_init(box);
+ }
+ }
+ return 0;
+}
+
+static int __cpuinit uncore_cpu_prepare(int cpu)
+{
+ struct intel_uncore_type *type;
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_box *exist, *box;
+ int i, j, phyid;
+
+ phyid = topology_physical_package_id(cpu);
+
+ /* allocate the box data structure */
+ for (i = 0; msr_uncores[i]; i++) {
+ type = msr_uncores[i];
+ for (j = 0; j < type->num_boxes; j++) {
+ exist = NULL;
+ pmu = &type->pmus[j];
+
+ if (pmu->func_id < 0)
+ pmu->func_id = j;
+ exist = uncore_pmu_find_box(pmu, phyid);
+ if (exist)
+ exist->refcnt++;
+ if (exist)
+ continue;
+
+ box = uncore_alloc_box(cpu);
+ if (!box)
+ return -ENOMEM;
+
+ box->pmu = pmu;
+ box->phy_id = phyid;
+ uncore_pmu_add_box(pmu, box);
+ }
+ }
+ return 0;
+}
+
+static void __cpuinit uncore_event_exit_cpu(int cpu)
+{
+ struct intel_uncore_type *type;
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_box *box;
+ int i, j, phyid, target;
+
+ /* if exiting cpu is used for collecting uncore events */
+ if (!cpumask_test_and_clear_cpu(cpu, &uncore_cpu_mask))
+ return;
+
+ /* find a new cpu to collect uncore events */
+ phyid = topology_physical_package_id(cpu);
+ target = -1;
+ for_each_online_cpu(i) {
+ if (i == cpu)
+ continue;
+ if (phyid == topology_physical_package_id(i)) {
+ target = i;
+ break;
+ }
+ }
+
+ /* migrate uncore events to the new cpu */
+ if (target >= 0)
+ cpumask_set_cpu(target, &uncore_cpu_mask);
+
+ for (i = 0; msr_uncores[i]; i++) {
+ type = msr_uncores[i];
+ for (j = 0; j < type->num_boxes; j++) {
+ pmu = &type->pmus[j];
+ box = uncore_pmu_find_box(pmu, phyid);
+ WARN_ON_ONCE(box->cpu != cpu);
+
+ if (target >= 0) {
+ uncore_pmu_cancel_hrtimer(box);
+ perf_pmu_migrate_context(&pmu->pmu,
+ cpu, target);
+ box->cpu = target;
+ } else {
+ box->cpu = -1;
+ }
+ }
+ }
+}
+
+static void __cpuinit uncore_event_init_cpu(int cpu)
+{
+ struct intel_uncore_type *type;
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_box *box;
+ int i, j, phyid;
+
+ phyid = topology_physical_package_id(cpu);
+ for_each_cpu(i, &uncore_cpu_mask) {
+ if (phyid == topology_physical_package_id(i))
+ return;
+ }
+
+ cpumask_set_cpu(cpu, &uncore_cpu_mask);
+
+ for (i = 0; msr_uncores[i]; i++) {
+ type = msr_uncores[i];
+ for (j = 0; j < type->num_boxes; j++) {
+ pmu = &type->pmus[j];
+ box = uncore_pmu_find_box(pmu, phyid);
+ WARN_ON_ONCE(box->cpu != -1);
+ box->cpu = cpu;
+ }
+ }
+}
+
+static int __cpuinit uncore_cpu_notifier(struct notifier_block *self,
+ unsigned long action, void *hcpu)
+{
+ unsigned int cpu = (long)hcpu;
+
+ /* allocate/free data structure for uncore box */
+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_UP_PREPARE:
+ uncore_cpu_prepare(cpu);
+ break;
+ case CPU_STARTING:
+ uncore_cpu_starting(cpu);
+ break;
+ case CPU_UP_CANCELED:
+ case CPU_DYING:
+ uncore_cpu_dying(cpu);
+ break;
+ default:
+ break;
+ }
+
+ /* select the cpu that collects uncore events */
+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_UP_PREPARE:
+ case CPU_DOWN_FAILED:
+ uncore_event_init_cpu(cpu);
+ break;
+
+ case CPU_UP_CANCELED:
+ case CPU_DOWN_PREPARE:
+ uncore_event_exit_cpu(cpu);
+ break;
+ default:
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block uncore_cpu_nb __cpuinitdata = {
+ .notifier_call = uncore_cpu_notifier,
+ /*
+ * to migrate uncore events, our notifier should be executed
+ * before perf core's notifier.
+ */
+ .priority = CPU_PRI_PERF + 1,
+};
+
+static void __init uncore_cpu_setup(void *dummy)
+{
+ uncore_cpu_starting(smp_processor_id());
+}
+
+static int __init uncore_cpu_init(void)
+{
+ int ret, cpu;
+
+ switch (boot_cpu_data.x86_model) {
+ default:
+ return 0;
+ }
+
+ ret = uncore_types_init(msr_uncores);
+ if (ret)
+ return ret;
+
+ get_online_cpus();
+
+ for_each_online_cpu(cpu) {
+ uncore_cpu_prepare(cpu);
+ uncore_event_init_cpu(cpu);
+ }
+ on_each_cpu(uncore_cpu_setup, NULL, 1);
+
+ register_cpu_notifier(&uncore_cpu_nb);
+
+ put_online_cpus();
+
+ return 0;
+}
+
+static int __init uncore_pmus_register(void)
+{
+ struct intel_uncore_pmu *pmu;
+ struct intel_uncore_type *type;
+ int i, j;
+
+ for (i = 0; msr_uncores[i]; i++) {
+ type = msr_uncores[i];
+ for (j = 0; j < type->num_boxes; j++) {
+ pmu = &type->pmus[j];
+ uncore_pmu_register(pmu);
+ }
+ }
+
+ return 0;
+}
+
+static int __init intel_uncore_init(void)
+{
+ int ret;
+
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ return -ENODEV;
+
+ ret = uncore_cpu_init();
+ if (ret)
+ goto fail;
+
+ uncore_pmus_register();
+ return 0;
+fail:
+ return ret;
+}
+device_initcall(intel_uncore_init);
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
new file mode 100644
index 0000000..ab648f3
--- /dev/null
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -0,0 +1,205 @@
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/perf_event.h>
+#include "perf_event.h"
+
+#define UNCORE_PMU_NAME_LEN 32
+#define UNCORE_BOX_HASH_SIZE 8
+
+#define UNCORE_PMU_HRTIMER_INTERVAL (60 * NSEC_PER_SEC)
+
+#define UNCORE_FIXED_EVENT 0xffff
+#define UNCORE_PMC_IDX_MAX_GENERIC 8
+#define UNCORE_PMC_IDX_FIXED UNCORE_PMC_IDX_MAX_GENERIC
+#define UNCORE_PMC_IDX_MAX (UNCORE_PMC_IDX_FIXED + 1)
+
+#define UNCORE_EVENT_CONSTRAINT(c, n) EVENT_CONSTRAINT(c, n, 0xff)
+
+struct intel_uncore_ops;
+struct intel_uncore_pmu;
+struct intel_uncore_box;
+struct uncore_event_desc;
+
+struct intel_uncore_type {
+ const char *name;
+ int num_counters;
+ int num_boxes;
+ int perf_ctr_bits;
+ int fixed_ctr_bits;
+ int single_fixed;
+ unsigned perf_ctr;
+ unsigned event_ctl;
+ unsigned event_mask;
+ unsigned fixed_ctr;
+ unsigned fixed_ctl;
+ unsigned box_ctl;
+ unsigned msr_offset;
+ struct event_constraint unconstrainted;
+ struct event_constraint *constraints;
+ struct intel_uncore_pmu *pmus;
+ struct intel_uncore_ops *ops;
+ struct uncore_event_desc *event_descs;
+ const struct attribute_group *attr_groups[3];
+};
+
+#define format_group attr_groups[0]
+
+struct intel_uncore_ops {
+ void (*init_box)(struct intel_uncore_box *);
+ void (*disable_box)(struct intel_uncore_box *);
+ void (*enable_box)(struct intel_uncore_box *);
+ void (*disable_event)(struct intel_uncore_box *, struct perf_event *);
+ void (*enable_event)(struct intel_uncore_box *, struct perf_event *);
+ u64 (*read_counter)(struct intel_uncore_box *, struct perf_event *);
+};
+
+struct intel_uncore_pmu {
+ struct pmu pmu;
+ char name[UNCORE_PMU_NAME_LEN];
+ int pmu_idx;
+ int func_id;
+ struct intel_uncore_type *type;
+ struct hlist_head box_hash[UNCORE_BOX_HASH_SIZE];
+};
+
+struct intel_uncore_box {
+ struct hlist_node hlist;
+ int phy_id;
+ int refcnt;
+ int n_active; /* number of active events */
+ int n_events;
+ int cpu; /* cpu to collect events */
+ unsigned long flags;
+ struct perf_event *events[UNCORE_PMC_IDX_MAX];
+ struct perf_event *event_list[UNCORE_PMC_IDX_MAX];
+ unsigned long active_mask[BITS_TO_LONGS(UNCORE_PMC_IDX_MAX)];
+ u64 tags[UNCORE_PMC_IDX_MAX];
+ struct intel_uncore_pmu *pmu;
+ struct hrtimer hrtimer;
+ struct rcu_head rcu_head;
+};
+
+#define UNCORE_BOX_FLAG_INITIATED 0
+
+struct uncore_event_desc {
+ struct kobj_attribute attr;
+ const char *config;
+};
+
+#define INTEL_UNCORE_EVENT_DESC(_name, _config) \
+{ \
+ .attr = __ATTR(_name, 0444, uncore_event_show, NULL), \
+ .config = _config, \
+}
+
+#define DEFINE_UNCORE_FORMAT_ATTR(_var, _name, _format) \
+static ssize_t __uncore_##_var##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, \
+ char *page) \
+{ \
+ BUILD_BUG_ON(sizeof(_format) >= PAGE_SIZE); \
+ return sprintf(page, _format "\n"); \
+} \
+static struct kobj_attribute format_attr_##_var = \
+ __ATTR(_name, 0444, __uncore_##_var##_show, NULL)
+
+
+static ssize_t uncore_event_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct uncore_event_desc *event =
+ container_of(attr, struct uncore_event_desc, attr);
+ return sprintf(buf, "%s", event->config);
+}
+
+static inline
+unsigned uncore_msr_box_ctl(struct intel_uncore_box *box)
+{
+ if (!box->pmu->type->box_ctl)
+ return 0;
+ return box->pmu->type->box_ctl +
+ box->pmu->type->msr_offset * box->pmu->pmu_idx;
+}
+
+static inline
+unsigned uncore_msr_fixed_ctl(struct intel_uncore_box *box)
+{
+ if (!box->pmu->type->fixed_ctl)
+ return 0;
+ return box->pmu->type->fixed_ctl +
+ box->pmu->type->msr_offset * box->pmu->pmu_idx;
+}
+
+static inline
+unsigned uncore_msr_fixed_ctr(struct intel_uncore_box *box)
+{
+ return box->pmu->type->fixed_ctr +
+ box->pmu->type->msr_offset * box->pmu->pmu_idx;
+}
+
+static inline
+unsigned uncore_msr_event_ctl(struct intel_uncore_box *box, int idx)
+{
+ return idx + box->pmu->type->event_ctl +
+ box->pmu->type->msr_offset * box->pmu->pmu_idx;
+}
+
+static inline
+unsigned uncore_msr_perf_ctr(struct intel_uncore_box *box, int idx)
+{
+ return idx + box->pmu->type->perf_ctr +
+ box->pmu->type->msr_offset * box->pmu->pmu_idx;
+}
+
+static inline int uncore_perf_ctr_bits(struct intel_uncore_box *box)
+{
+ return box->pmu->type->perf_ctr_bits;
+}
+
+static inline int uncore_fixed_ctr_bits(struct intel_uncore_box *box)
+{
+ return box->pmu->type->fixed_ctr_bits;
+}
+
+static inline int uncore_num_counters(struct intel_uncore_box *box)
+{
+ return box->pmu->type->num_counters;
+}
+
+static inline void uncore_disable_box(struct intel_uncore_box *box)
+{
+ if (box->pmu->type->ops->disable_box)
+ box->pmu->type->ops->disable_box(box);
+}
+
+static inline void uncore_enable_box(struct intel_uncore_box *box)
+{
+ if (box->pmu->type->ops->enable_box)
+ box->pmu->type->ops->enable_box(box);
+}
+
+static inline void uncore_disable_event(struct intel_uncore_box *box,
+ struct perf_event *event)
+{
+ box->pmu->type->ops->disable_event(box, event);
+}
+
+static inline void uncore_enable_event(struct intel_uncore_box *box,
+ struct perf_event *event)
+{
+ box->pmu->type->ops->enable_event(box, event);
+}
+
+static inline u64 uncore_read_counter(struct intel_uncore_box *box,
+ struct perf_event *event)
+{
+ return box->pmu->type->ops->read_counter(box, event);
+}
+
+static inline void uncore_box_init(struct intel_uncore_box *box)
+{
+ if (!test_and_set_bit(UNCORE_BOX_FLAG_INITIATED, &box->flags)) {
+ if (box->pmu->type->ops->init_box)
+ box->pmu->type->ops->init_box(box);
+ }
+}
--
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


a.p.zijlstra at chello

May 3, 2012, 10:12 AM

Post #2 of 10 (124 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On Wed, 2012-05-02 at 10:07 +0800, Yan, Zheng wrote:
> +static struct intel_uncore_box *
> +__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
> +{
> + struct intel_uncore_box *box;
> + struct hlist_head *head;
> + struct hlist_node *node;
> +
> + head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
> + hlist_for_each_entry_rcu(box, node, head, hlist) {
> + if (box->phy_id == phyid)
> + return box;
> + }
> +
> + return NULL;
> +}

I still don't get why something like:

static struct intel_uncore_box *
pmu_to_box(struct intel_uncore_pmu *pmu, int cpu)
{
return per_cpu_ptr(pmu->box, cpu);
}

doesn't work.

Last time you mumbled something about PCI devices, but afaict those are
in all respects identical to MSR devices except you talk to them using
PCI-mmio instead of MSR registers.

In fact, since its all local to the generic code there's nothing
different between pci/msr already.

So how about something like this:

---
Makefile | 4 +-
perf_event_intel_uncore.c | 92 ++++++++++++++++++----------------------------
perf_event_intel_uncore.h | 4 +-
3 files changed, 42 insertions(+), 58 deletions(-)

--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -32,7 +32,9 @@ obj-$(CONFIG_PERF_EVENTS) += perf_event

ifdef CONFIG_PERF_EVENTS
obj-$(CONFIG_CPU_SUP_AMD) += perf_event_amd.o
-obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o perf_event_intel_uncore.o
+obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o
+obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o
+obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_uncore.o
endif

obj-$(CONFIG_X86_MCE) += mcheck/
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -116,40 +116,21 @@ struct intel_uncore_box *uncore_alloc_bo
}

static struct intel_uncore_box *
-__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
+uncore_pmu_to_box(struct intel_uncore_pmu *pmu, int cpu)
{
- struct intel_uncore_box *box;
- struct hlist_head *head;
- struct hlist_node *node;
-
- head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
- hlist_for_each_entry_rcu(box, node, head, hlist) {
- if (box->phy_id == phyid)
- return box;
- }
-
- return NULL;
-}
-
-static struct intel_uncore_box *
-uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
-{
- struct intel_uncore_box *box;
-
- rcu_read_lock();
- box = __uncore_pmu_find_box(pmu, phyid);
- rcu_read_unlock();
-
- return box;
+ return per_cpu_ptr(pmu->box, cpu);
}

static void uncore_pmu_add_box(struct intel_uncore_pmu *pmu,
struct intel_uncore_box *box)
{
- struct hlist_head *head;
+ int cpu;

- head = &pmu->box_hash[box->phy_id % UNCORE_BOX_HASH_SIZE];
- hlist_add_head_rcu(&box->hlist, head);
+ for_each_cpu(cpu) {
+ if (box->phys_id != topology_physical_package_id(cpu))
+ continue;
+ per_cpu_ptr(pmu->box, cpu) = box;
+ }
}

static struct intel_uncore_pmu *uncore_event_to_pmu(struct perf_event *event)
@@ -163,8 +144,7 @@ static struct intel_uncore_box *uncore_e
* perf core schedules event on the basis of cpu, uncore events are
* collected by one of the cpus inside a physical package.
*/
- int phyid = topology_physical_package_id(smp_processor_id());
- return uncore_pmu_find_box(uncore_event_to_pmu(event), phyid);
+ return uncore_pmu_to_box(uncore_event_to_pmu(event), smp_processor_id());
}

static int uncore_collect_events(struct intel_uncore_box *box,
@@ -478,8 +458,7 @@ int uncore_pmu_event_init(struct perf_ev
*/
if (event->cpu < 0)
return -EINVAL;
- box = uncore_pmu_find_box(pmu,
- topology_physical_package_id(event->cpu));
+ box = uncore_pmu_to_box(pmu, event->cpu);
if (!box || box->cpu < 0)
return -EINVAL;
event->cpu = box->cpu;
@@ -541,7 +520,11 @@ static int __init uncore_pmu_register(st

static void __init uncore_type_exit(struct intel_uncore_type *type)
{
+ int i;
+
kfree(type->attr_groups[1]);
+ for (i = 0; i < type->num_boxes; i++)
+ free_percpu(type->pmus[i].box);
kfree(type->pmus);
type->attr_groups[1] = NULL;
type->pmus = NULL;
@@ -566,9 +549,9 @@ static int __init uncore_type_init(struc
pmus[i].func_id = -1;
pmus[i].pmu_idx = i;
pmus[i].type = type;
-
- for (j = 0; j < ARRAY_SIZE(pmus[0].box_hash); j++)
- INIT_HLIST_HEAD(&pmus[i].box_hash[j]);
+ pmus[i].box = alloc_percpu(struct intel_uncore_box *);
+ if (!pmus[i].box)
+ goto fail_percpu;
}

if (type->event_descs) {
@@ -591,6 +574,11 @@ static int __init uncore_type_init(struc

type->pmus = pmus;
return 0;
+
+fail_percpu:
+ for (i = 0; i < type->num_boxes; i++)
+ free_percpu(pmus[i].box);
+
fail:
uncore_type_exit(type);
return -ENOMEM;
@@ -617,15 +605,13 @@ static void __cpuinit uncore_cpu_dying(i
struct intel_uncore_type *type;
struct intel_uncore_pmu *pmu;
struct intel_uncore_box *box;
- int i, j, phyid;
-
- phyid = topology_physical_package_id(cpu);
+ int i, j;

for (i = 0; msr_uncores[i]; i++) {
type = msr_uncores[i];
for (j = 0; j < type->num_boxes; j++) {
pmu = &type->pmus[j];
- box = uncore_pmu_find_box(pmu, phyid);
+ box = uncore_pmu_to_box(pmu, cpu);
if (box && --box->refcnt == 0) {
hlist_del_rcu(&box->hlist);
kfree_rcu(box, rcu_head);
@@ -639,15 +625,13 @@ static int __cpuinit uncore_cpu_starting
struct intel_uncore_type *type;
struct intel_uncore_pmu *pmu;
struct intel_uncore_box *box;
- int i, j, phyid;
-
- phyid = topology_physical_package_id(cpu);
+ int i, j;

for (i = 0; msr_uncores[i]; i++) {
type = msr_uncores[i];
for (j = 0; j < type->num_boxes; j++) {
pmu = &type->pmus[j];
- box = uncore_pmu_find_box(pmu, phyid);
+ box = uncore_pmu_to_box(pmu, cpu);
if (box)
uncore_box_init(box);
}
@@ -660,9 +644,7 @@ static int __cpuinit uncore_cpu_prepare(
struct intel_uncore_type *type;
struct intel_uncore_pmu *pmu;
struct intel_uncore_box *exist, *box;
- int i, j, phyid;
-
- phyid = topology_physical_package_id(cpu);
+ int i, j;

/* allocate the box data structure */
for (i = 0; msr_uncores[i]; i++) {
@@ -673,7 +655,7 @@ static int __cpuinit uncore_cpu_prepare(

if (pmu->func_id < 0)
pmu->func_id = j;
- exist = uncore_pmu_find_box(pmu, phyid);
+ exist = uncore_pmu_to_box(pmu, cpu);
if (exist)
exist->refcnt++;
if (exist)
@@ -684,7 +666,7 @@ static int __cpuinit uncore_cpu_prepare(
return -ENOMEM;

box->pmu = pmu;
- box->phy_id = phyid;
+ box->phys_id = topology_physical_package_id(cpu);
uncore_pmu_add_box(pmu, box);
}
}
@@ -696,19 +678,19 @@ static void __cpuinit uncore_event_exit_
struct intel_uncore_type *type;
struct intel_uncore_pmu *pmu;
struct intel_uncore_box *box;
- int i, j, phyid, target;
+ int i, j, phys_id, target;

/* if exiting cpu is used for collecting uncore events */
if (!cpumask_test_and_clear_cpu(cpu, &uncore_cpu_mask))
return;

/* find a new cpu to collect uncore events */
- phyid = topology_physical_package_id(cpu);
+ phys_id = topology_physical_package_id(cpu);
target = -1;
for_each_online_cpu(i) {
if (i == cpu)
continue;
- if (phyid == topology_physical_package_id(i)) {
+ if (phys_id == topology_physical_package_id(i)) {
target = i;
break;
}
@@ -722,7 +704,7 @@ static void __cpuinit uncore_event_exit_
type = msr_uncores[i];
for (j = 0; j < type->num_boxes; j++) {
pmu = &type->pmus[j];
- box = uncore_pmu_find_box(pmu, phyid);
+ box = uncore_pmu_to_box(pmu, phys_id);
WARN_ON_ONCE(box->cpu != cpu);

if (target >= 0) {
@@ -742,11 +724,11 @@ static void __cpuinit uncore_event_init_
struct intel_uncore_type *type;
struct intel_uncore_pmu *pmu;
struct intel_uncore_box *box;
- int i, j, phyid;
+ int i, j, phys_id;

- phyid = topology_physical_package_id(cpu);
+ phys_id = topology_physical_package_id(cpu);
for_each_cpu(i, &uncore_cpu_mask) {
- if (phyid == topology_physical_package_id(i))
+ if (phys_id == topology_physical_package_id(i))
return;
}

@@ -756,7 +738,7 @@ static void __cpuinit uncore_event_init_
type = msr_uncores[i];
for (j = 0; j < type->num_boxes; j++) {
pmu = &type->pmus[j];
- box = uncore_pmu_find_box(pmu, phyid);
+ box = uncore_pmu_to_box(pmu, cpu);
WARN_ON_ONCE(box->cpu != -1);
box->cpu = cpu;
}
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -59,12 +59,12 @@ struct intel_uncore_pmu {
int pmu_idx;
int func_id;
struct intel_uncore_type *type;
- struct hlist_head box_hash[UNCORE_BOX_HASH_SIZE];
+ struct intel_uncore_box * __per_cpu box;
};

struct intel_uncore_box {
struct hlist_node hlist;
- int phy_id;
+ int phys_id;
int refcnt;
int n_active; /* number of active events */
int n_events;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


a.p.zijlstra at chello

May 3, 2012, 2:49 PM

Post #3 of 10 (119 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On Wed, 2012-05-02 at 10:07 +0800, Yan, Zheng wrote:
> + sprintf(pmu->name, "uncore_%s%d", pmu->type->name,
> + pmu->pmu_idx);

That probably wants to be: "uncore_%s_%d" or somesuch.

"uncore_C-Box3 vs "uncore_C-Box_3"

Looking at it like that, it might do to call it "Uncore*", dunno..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


zheng.z.yan at intel

May 4, 2012, 12:33 AM

Post #4 of 10 (125 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On 05/04/2012 01:12 AM, Peter Zijlstra wrote:
> On Wed, 2012-05-02 at 10:07 +0800, Yan, Zheng wrote:
>> +static struct intel_uncore_box *
>> +__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
>> +{
>> + struct intel_uncore_box *box;
>> + struct hlist_head *head;
>> + struct hlist_node *node;
>> +
>> + head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
>> + hlist_for_each_entry_rcu(box, node, head, hlist) {
>> + if (box->phy_id == phyid)
>> + return box;
>> + }
>> +
>> + return NULL;
>> +}
>
> I still don't get why something like:
>
> static struct intel_uncore_box *
> pmu_to_box(struct intel_uncore_pmu *pmu, int cpu)
> {
> return per_cpu_ptr(pmu->box, cpu);
> }
>
> doesn't work.
>
> Last time you mumbled something about PCI devices, but afaict those are
> in all respects identical to MSR devices except you talk to them using
> PCI-mmio instead of MSR registers.
>
> In fact, since its all local to the generic code there's nothing
> different between pci/msr already.
>
> So how about something like this:
>
> ---
> Makefile | 4 +-
> perf_event_intel_uncore.c | 92 ++++++++++++++++++----------------------------
> perf_event_intel_uncore.h | 4 +-
> 3 files changed, 42 insertions(+), 58 deletions(-)
>
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -32,7 +32,9 @@ obj-$(CONFIG_PERF_EVENTS) += perf_event
>
> ifdef CONFIG_PERF_EVENTS
> obj-$(CONFIG_CPU_SUP_AMD) += perf_event_amd.o
> -obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o perf_event_intel_uncore.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_uncore.o
> endif
>
> obj-$(CONFIG_X86_MCE) += mcheck/
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -116,40 +116,21 @@ struct intel_uncore_box *uncore_alloc_bo
> }
>
> static struct intel_uncore_box *
> -__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
> +uncore_pmu_to_box(struct intel_uncore_pmu *pmu, int cpu)
> {
> - struct intel_uncore_box *box;
> - struct hlist_head *head;
> - struct hlist_node *node;
> -
> - head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
> - hlist_for_each_entry_rcu(box, node, head, hlist) {
> - if (box->phy_id == phyid)
> - return box;
> - }
> -
> - return NULL;
> -}
> -
> -static struct intel_uncore_box *
> -uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
> -{
> - struct intel_uncore_box *box;
> -
> - rcu_read_lock();
> - box = __uncore_pmu_find_box(pmu, phyid);
> - rcu_read_unlock();
> -
> - return box;
> + return per_cpu_ptr(pmu->box, cpu);
> }
>
> static void uncore_pmu_add_box(struct intel_uncore_pmu *pmu,
> struct intel_uncore_box *box)
> {
> - struct hlist_head *head;
> + int cpu;
>
> - head = &pmu->box_hash[box->phy_id % UNCORE_BOX_HASH_SIZE];
> - hlist_add_head_rcu(&box->hlist, head);
> + for_each_cpu(cpu) {
> + if (box->phys_id != topology_physical_package_id(cpu))
> + continue;
> + per_cpu_ptr(pmu->box, cpu) = box;
> + }
> }
Thank you for your suggestion. One reason I choose hash table is that I'm uncertain
the system behavior when hot-plug a CPU, the PCI uncore device is probed first or
the CPU data is initialized first? If the PCI device is probed first, I think your
code won't work because topology_physical_package_id may return incorrect id.

Regards
Yan, Zheng

>
> static struct intel_uncore_pmu *uncore_event_to_pmu(struct perf_event *event)
> @@ -163,8 +144,7 @@ static struct intel_uncore_box *uncore_e
> * perf core schedules event on the basis of cpu, uncore events are
> * collected by one of the cpus inside a physical package.
> */
> - int phyid = topology_physical_package_id(smp_processor_id());
> - return uncore_pmu_find_box(uncore_event_to_pmu(event), phyid);
> + return uncore_pmu_to_box(uncore_event_to_pmu(event), smp_processor_id());
> }
>
> static int uncore_collect_events(struct intel_uncore_box *box,
> @@ -478,8 +458,7 @@ int uncore_pmu_event_init(struct perf_ev
> */
> if (event->cpu < 0)
> return -EINVAL;
> - box = uncore_pmu_find_box(pmu,
> - topology_physical_package_id(event->cpu));
> + box = uncore_pmu_to_box(pmu, event->cpu);
> if (!box || box->cpu < 0)
> return -EINVAL;
> event->cpu = box->cpu;
> @@ -541,7 +520,11 @@ static int __init uncore_pmu_register(st
>
> static void __init uncore_type_exit(struct intel_uncore_type *type)
> {
> + int i;
> +
> kfree(type->attr_groups[1]);
> + for (i = 0; i < type->num_boxes; i++)
> + free_percpu(type->pmus[i].box);
> kfree(type->pmus);
> type->attr_groups[1] = NULL;
> type->pmus = NULL;
> @@ -566,9 +549,9 @@ static int __init uncore_type_init(struc
> pmus[i].func_id = -1;
> pmus[i].pmu_idx = i;
> pmus[i].type = type;
> -
> - for (j = 0; j < ARRAY_SIZE(pmus[0].box_hash); j++)
> - INIT_HLIST_HEAD(&pmus[i].box_hash[j]);
> + pmus[i].box = alloc_percpu(struct intel_uncore_box *);
> + if (!pmus[i].box)
> + goto fail_percpu;
> }
>
> if (type->event_descs) {
> @@ -591,6 +574,11 @@ static int __init uncore_type_init(struc
>
> type->pmus = pmus;
> return 0;
> +
> +fail_percpu:
> + for (i = 0; i < type->num_boxes; i++)
> + free_percpu(pmus[i].box);
> +
> fail:
> uncore_type_exit(type);
> return -ENOMEM;
> @@ -617,15 +605,13 @@ static void __cpuinit uncore_cpu_dying(i
> struct intel_uncore_type *type;
> struct intel_uncore_pmu *pmu;
> struct intel_uncore_box *box;
> - int i, j, phyid;
> -
> - phyid = topology_physical_package_id(cpu);
> + int i, j;
>
> for (i = 0; msr_uncores[i]; i++) {
> type = msr_uncores[i];
> for (j = 0; j < type->num_boxes; j++) {
> pmu = &type->pmus[j];
> - box = uncore_pmu_find_box(pmu, phyid);
> + box = uncore_pmu_to_box(pmu, cpu);
> if (box && --box->refcnt == 0) {
> hlist_del_rcu(&box->hlist);
> kfree_rcu(box, rcu_head);
> @@ -639,15 +625,13 @@ static int __cpuinit uncore_cpu_starting
> struct intel_uncore_type *type;
> struct intel_uncore_pmu *pmu;
> struct intel_uncore_box *box;
> - int i, j, phyid;
> -
> - phyid = topology_physical_package_id(cpu);
> + int i, j;
>
> for (i = 0; msr_uncores[i]; i++) {
> type = msr_uncores[i];
> for (j = 0; j < type->num_boxes; j++) {
> pmu = &type->pmus[j];
> - box = uncore_pmu_find_box(pmu, phyid);
> + box = uncore_pmu_to_box(pmu, cpu);
> if (box)
> uncore_box_init(box);
> }
> @@ -660,9 +644,7 @@ static int __cpuinit uncore_cpu_prepare(
> struct intel_uncore_type *type;
> struct intel_uncore_pmu *pmu;
> struct intel_uncore_box *exist, *box;
> - int i, j, phyid;
> -
> - phyid = topology_physical_package_id(cpu);
> + int i, j;
>
> /* allocate the box data structure */
> for (i = 0; msr_uncores[i]; i++) {
> @@ -673,7 +655,7 @@ static int __cpuinit uncore_cpu_prepare(
>
> if (pmu->func_id < 0)
> pmu->func_id = j;
> - exist = uncore_pmu_find_box(pmu, phyid);
> + exist = uncore_pmu_to_box(pmu, cpu);
> if (exist)
> exist->refcnt++;
> if (exist)
> @@ -684,7 +666,7 @@ static int __cpuinit uncore_cpu_prepare(
> return -ENOMEM;
>
> box->pmu = pmu;
> - box->phy_id = phyid;
> + box->phys_id = topology_physical_package_id(cpu);
> uncore_pmu_add_box(pmu, box);
> }
> }
> @@ -696,19 +678,19 @@ static void __cpuinit uncore_event_exit_
> struct intel_uncore_type *type;
> struct intel_uncore_pmu *pmu;
> struct intel_uncore_box *box;
> - int i, j, phyid, target;
> + int i, j, phys_id, target;
>
> /* if exiting cpu is used for collecting uncore events */
> if (!cpumask_test_and_clear_cpu(cpu, &uncore_cpu_mask))
> return;
>
> /* find a new cpu to collect uncore events */
> - phyid = topology_physical_package_id(cpu);
> + phys_id = topology_physical_package_id(cpu);
> target = -1;
> for_each_online_cpu(i) {
> if (i == cpu)
> continue;
> - if (phyid == topology_physical_package_id(i)) {
> + if (phys_id == topology_physical_package_id(i)) {
> target = i;
> break;
> }
> @@ -722,7 +704,7 @@ static void __cpuinit uncore_event_exit_
> type = msr_uncores[i];
> for (j = 0; j < type->num_boxes; j++) {
> pmu = &type->pmus[j];
> - box = uncore_pmu_find_box(pmu, phyid);
> + box = uncore_pmu_to_box(pmu, phys_id);
> WARN_ON_ONCE(box->cpu != cpu);
>
> if (target >= 0) {
> @@ -742,11 +724,11 @@ static void __cpuinit uncore_event_init_
> struct intel_uncore_type *type;
> struct intel_uncore_pmu *pmu;
> struct intel_uncore_box *box;
> - int i, j, phyid;
> + int i, j, phys_id;
>
> - phyid = topology_physical_package_id(cpu);
> + phys_id = topology_physical_package_id(cpu);
> for_each_cpu(i, &uncore_cpu_mask) {
> - if (phyid == topology_physical_package_id(i))
> + if (phys_id == topology_physical_package_id(i))
> return;
> }
>
> @@ -756,7 +738,7 @@ static void __cpuinit uncore_event_init_
> type = msr_uncores[i];
> for (j = 0; j < type->num_boxes; j++) {
> pmu = &type->pmus[j];
> - box = uncore_pmu_find_box(pmu, phyid);
> + box = uncore_pmu_to_box(pmu, cpu);
> WARN_ON_ONCE(box->cpu != -1);
> box->cpu = cpu;
> }
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> @@ -59,12 +59,12 @@ struct intel_uncore_pmu {
> int pmu_idx;
> int func_id;
> struct intel_uncore_type *type;
> - struct hlist_head box_hash[UNCORE_BOX_HASH_SIZE];
> + struct intel_uncore_box * __per_cpu box;
> };
>
> struct intel_uncore_box {
> struct hlist_node hlist;
> - int phy_id;
> + int phys_id;
> int refcnt;
> int n_active; /* number of active events */
> int n_events;
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


a.p.zijlstra at chello

May 4, 2012, 10:57 AM

Post #5 of 10 (121 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On Fri, 2012-05-04 at 15:33 +0800, Yan, Zheng wrote:
> Thank you for your suggestion. One reason I choose hash table is that I'm uncertain
> the system behavior when hot-plug a CPU, the PCI uncore device is probed first or
> the CPU data is initialized first? If the PCI device is probed first, I think your
> code won't work because topology_physical_package_id may return incorrect id.

It had better first initialize the cpu data before it starts probing its
devices. Doing it the other way around just doesn't make any sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


zheng.z.yan at intel

May 10, 2012, 12:34 AM

Post #6 of 10 (112 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On 05/04/2012 01:12 AM, Peter Zijlstra wrote:
> On Wed, 2012-05-02 at 10:07 +0800, Yan, Zheng wrote:
>> +static struct intel_uncore_box *
>> +__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
>> +{
>> + struct intel_uncore_box *box;
>> + struct hlist_head *head;
>> + struct hlist_node *node;
>> +
>> + head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
>> + hlist_for_each_entry_rcu(box, node, head, hlist) {
>> + if (box->phy_id == phyid)
>> + return box;
>> + }
>> +
>> + return NULL;
>> +}
>
> I still don't get why something like:
>
> static struct intel_uncore_box *
> pmu_to_box(struct intel_uncore_pmu *pmu, int cpu)
> {
> return per_cpu_ptr(pmu->box, cpu);
> }
>
> doesn't work.
>
> Last time you mumbled something about PCI devices, but afaict those are
> in all respects identical to MSR devices except you talk to them using
> PCI-mmio instead of MSR registers.
>
> In fact, since its all local to the generic code there's nothing
> different between pci/msr already.
>
> So how about something like this:
>
> ---
> Makefile | 4 +-
> perf_event_intel_uncore.c | 92 ++++++++++++++++++----------------------------
> perf_event_intel_uncore.h | 4 +-
> 3 files changed, 42 insertions(+), 58 deletions(-)
>
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -32,7 +32,9 @@ obj-$(CONFIG_PERF_EVENTS) += perf_event
>
> ifdef CONFIG_PERF_EVENTS
> obj-$(CONFIG_CPU_SUP_AMD) += perf_event_amd.o
> -obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o perf_event_intel_uncore.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_p6.o perf_event_p4.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_lbr.o perf_event_intel_ds.o perf_event_intel.o
> +obj-$(CONFIG_CPU_SUP_INTEL) += perf_event_intel_uncore.o
> endif
>
> obj-$(CONFIG_X86_MCE) += mcheck/
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -116,40 +116,21 @@ struct intel_uncore_box *uncore_alloc_bo
> }
>
> static struct intel_uncore_box *
> -__uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
> +uncore_pmu_to_box(struct intel_uncore_pmu *pmu, int cpu)
> {
> - struct intel_uncore_box *box;
> - struct hlist_head *head;
> - struct hlist_node *node;
> -
> - head = &pmu->box_hash[phyid % UNCORE_BOX_HASH_SIZE];
> - hlist_for_each_entry_rcu(box, node, head, hlist) {
> - if (box->phy_id == phyid)
> - return box;
> - }
> -
> - return NULL;
> -}
> -
> -static struct intel_uncore_box *
> -uncore_pmu_find_box(struct intel_uncore_pmu *pmu, int phyid)
> -{
> - struct intel_uncore_box *box;
> -
> - rcu_read_lock();
> - box = __uncore_pmu_find_box(pmu, phyid);
> - rcu_read_unlock();
> -
> - return box;
> + return per_cpu_ptr(pmu->box, cpu);
> }
>
> static void uncore_pmu_add_box(struct intel_uncore_pmu *pmu,
> struct intel_uncore_box *box)
> {
> - struct hlist_head *head;
> + int cpu;
>
> - head = &pmu->box_hash[box->phy_id % UNCORE_BOX_HASH_SIZE];
> - hlist_add_head_rcu(&box->hlist, head);
> + for_each_cpu(cpu) {
> + if (box->phys_id != topology_physical_package_id(cpu))
> + continue;
> + per_cpu_ptr(pmu->box, cpu) = box;
> + }
> }

This code doesn't work for PCI uncore device if there are offline CPUs,
because topology_physical_package_id() always return 0 for offline CPUs.
So besides the per CPU variable, we still need another data structure
to track the uncore boxes. Do you still want to use per CPU variable?

Regards
Yan, Zheng

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


a.p.zijlstra at chello

May 10, 2012, 3:05 AM

Post #7 of 10 (113 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On Thu, 2012-05-10 at 15:34 +0800, Yan, Zheng wrote:
>
> This code doesn't work for PCI uncore device if there are offline CPUs,
> because topology_physical_package_id() always return 0 for offline CPUs.

Hmm, that sounds wrong.. one would expect something like BAD_APICID, -1
or the correct number. hpa should we fix that?

Anyway,

> So besides the per CPU variable, we still need another data structure
> to track the uncore boxes. Do you still want to use per CPU variable?

Well you don't really need the value for offline CPUs, until they're
online right? So all you need to do is iterate all the box muck in a
hotplug handler and set the right pointer, no? (I suspect this would
need to be done in CPU_STARTING).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


zheng.z.yan at intel

May 10, 2012, 6:54 PM

Post #8 of 10 (114 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On 05/10/2012 06:05 PM, Peter Zijlstra wrote:
> On Thu, 2012-05-10 at 15:34 +0800, Yan, Zheng wrote:
>>
>> This code doesn't work for PCI uncore device if there are offline CPUs,
>> because topology_physical_package_id() always return 0 for offline CPUs.
>
> Hmm, that sounds wrong.. one would expect something like BAD_APICID, -1
> or the correct number. hpa should we fix that?
>
> Anyway,
>
>> So besides the per CPU variable, we still need another data structure
>> to track the uncore boxes. Do you still want to use per CPU variable?
>
> Well you don't really need the value for offline CPUs, until they're
> online right?
Yes

> So all you need to do is iterate all the box muck in a
> hotplug handler and set the right pointer, no? (I suspect this would
> need to be done in CPU_STARTING).
The problem is that CPUs on a particular socket can all be offline. So
we need an extra data structure to track the uncore boxes. This means
that the memory addresses of uncore boxes are stored in two places.
It's a little tricky to keep the two places in sync. For example, In the
case of hot-plugging CPU, the per CPU pointers can only be set after the
PCI uncore devices are probed. How to handle the case that cpu hotplug
handler is executed before PCI uncore devices are probed? For the reason
listed above I still prefer not to use per CPU pointer.

Regards
Yan, Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


khandual at linux

May 10, 2012, 11:31 PM

Post #9 of 10 (114 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On Wednesday 02 May 2012 07:37 AM, Yan, Zheng wrote:

> +static int __cpuinit uncore_cpu_prepare(int cpu)
> +{
> + struct intel_uncore_type *type;
> + struct intel_uncore_pmu *pmu;
> + struct intel_uncore_box *exist, *box;
> + int i, j, phyid;
> +
> + phyid = topology_physical_package_id(cpu);
> +
> + /* allocate the box data structure */
> + for (i = 0; msr_uncores[i]; i++) {
> + type = msr_uncores[i];
> + for (j = 0; j < type->num_boxes; j++) {
> + exist = NULL;
> + pmu = &type->pmus[j];
> +
> + if (pmu->func_id < 0)
> + pmu->func_id = j;
> + exist = uncore_pmu_find_box(pmu, phyid);
> + if (exist)
> + exist->refcnt++;
> + if (exist)
> + continue;



May be a redundant condition checking ^ ?

> +
> + box = uncore_alloc_box(cpu);
> + if (!box)
> + return -ENOMEM;
> +
> + box->pmu = pmu;
> + box->phy_id = phyid;
> + uncore_pmu_add_box(pmu, box);
> + }
> + }
> + return 0;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


zheng.z.yan at intel

May 10, 2012, 11:41 PM

Post #10 of 10 (112 views)
Permalink
Re: [PATCH 4/9] perf: Generic intel uncore support [In reply to]

On 05/11/2012 02:31 PM, Anshuman Khandual wrote:
> On Wednesday 02 May 2012 07:37 AM, Yan, Zheng wrote:
>
>> +static int __cpuinit uncore_cpu_prepare(int cpu)
>> +{
>> + struct intel_uncore_type *type;
>> + struct intel_uncore_pmu *pmu;
>> + struct intel_uncore_box *exist, *box;
>> + int i, j, phyid;
>> +
>> + phyid = topology_physical_package_id(cpu);
>> +
>> + /* allocate the box data structure */
>> + for (i = 0; msr_uncores[i]; i++) {
>> + type = msr_uncores[i];
>> + for (j = 0; j < type->num_boxes; j++) {
>> + exist = NULL;
>> + pmu = &type->pmus[j];
>> +
>> + if (pmu->func_id < 0)
>> + pmu->func_id = j;
>> + exist = uncore_pmu_find_box(pmu, phyid);
>> + if (exist)
>> + exist->refcnt++;
>> + if (exist)
>> + continue;
>
>
>
> May be a redundant condition checking ^ ?

Yes, it's redundant. I will remove it in later patches.

Regards
Yan, Zheng
>
>> +
>> + box = uncore_alloc_box(cpu);
>> + if (!box)
>> + return -ENOMEM;
>> +
>> + box->pmu = pmu;
>> + box->phy_id = phyid;
>> + uncore_pmu_add_box(pmu, box);
>> + }
>> + }
>> + return 0;
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.