| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <18896.19227.471031.516118@cargo.ozlabs.ibm.com>
Date: Mon, 30 Mar 2009 15:31:23 +1100
From: Paul Mackerras <paulus@...ba.org>
To: Ingo Molnar <mingo@...e.hu>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
CC: linux-kernel@...r.kernel.org
Subject: [PATCH v2] perf_counter: make it possible for hw_perf_counter_init to return error codes
Impact: better error reporting
At present, if hw_perf_counter_init encounters an error, all it can do
is return NULL, which causes sys_perf_counter_open to return an EINVAL
error to userspace. This isn't very informative for userspace; it means
that userspace can't tell the difference between "sorry, oprofile is
already using the PMU" and "we don't support this CPU" and "this CPU
doesn't support the requested generic hardware event".
This commit uses the PTR_ERR/ERR_PTR/IS_ERR set of macros to let
hw_perf_counter_init return an error code on error rather than just NULL
if it wishes. If it does so, that error code will be returned from
sys_perf_counter_open to userspace. If it returns NULL, an EINVAL
error will be returned to userspace, as before.
This also adapts the powerpc hw_perf_counter_init to make use of this
to return ENXIO, EINVAL, EBUSY, or EOPNOTSUPP as appropriate. It would
be good to add extra error numbers in future to allow userspace to
distinguish the various errors that are currently reported as EINVAL,
i.e. irq_period < 0, too many events in a group, conflict between
exclude_* settings in a group, and PMU resource conflict in a group.
[v2: fix a bug pointed out by Corey Ashford where error returns from
hw_perf_counter_init were not handled correctly in the case of raw
hardware events.]
Signed-off-by: Paul Mackerras <paulus@...ba.org>
---
arch/powerpc/kernel/perf_counter.c | 14 +++++++-------
kernel/perf_counter.c | 35 ++++++++++++++++++++++-------------
2 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/perf_counter.c b/arch/powerpc/kernel/perf_counter.c
index 1efd252..3ac2a2d 100644
--- a/arch/powerpc/kernel/perf_counter.c
+++ b/arch/powerpc/kernel/perf_counter.c
@@ -624,13 +624,13 @@ hw_perf_counter_init(struct perf_counter *counter)
int err;
if (!ppmu)
- return NULL;
+ return ERR_PTR(-ENXIO);
if ((s64)counter->hw_event.irq_period < 0)
- return NULL;
+ return ERR_PTR(-EINVAL);
if (!perf_event_raw(&counter->hw_event)) {
ev = perf_event_id(&counter->hw_event);
if (ev >= ppmu->n_generic || ppmu->generic_events[ev] == 0)
- return NULL;
+ return ERR_PTR(-EOPNOTSUPP);
ev = ppmu->generic_events[ev];
} else {
ev = perf_event_config(&counter->hw_event);
@@ -656,14 +656,14 @@ hw_perf_counter_init(struct perf_counter *counter)
n = collect_events(counter->group_leader, ppmu->n_counter - 1,
ctrs, events);
if (n < 0)
- return NULL;
+ return ERR_PTR(-EINVAL);
}
events[n] = ev;
ctrs[n] = counter;
if (check_excludes(ctrs, n, 1))
- return NULL;
+ return ERR_PTR(-EINVAL);
if (power_check_constraints(events, n + 1))
- return NULL;
+ return ERR_PTR(-EINVAL);
counter->hw.config = events[n];
atomic64_set(&counter->hw.period_left, counter->hw_event.irq_period);
@@ -687,7 +687,7 @@ hw_perf_counter_init(struct perf_counter *counter)
counter->destroy = hw_perf_counter_destroy;
if (err)
- return NULL;
+ return ERR_PTR(err);
return &power_perf_ops;
}
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index 3b862a7..541c2ac 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -2187,10 +2187,11 @@ perf_counter_alloc(struct perf_counter_hw_event *hw_event,
{
const struct hw_perf_counter_ops *hw_ops;
struct perf_counter *counter;
+ long err;
counter = kzalloc(sizeof(*counter), gfpflags);
if (!counter)
- return NULL;
+ return ERR_PTR(-ENOMEM);
/*
* Single counters are their own group leaders, with an
@@ -2240,12 +2241,18 @@ perf_counter_alloc(struct perf_counter_hw_event *hw_event,
hw_ops = tp_perf_counter_init(counter);
break;
}
+done:
+ err = 0;
+ if (!hw_ops)
+ err = -EINVAL;
+ else if (IS_ERR(hw_ops))
+ err = PTR_ERR(hw_ops);
- if (!hw_ops) {
+ if (err) {
kfree(counter);
- return NULL;
+ return ERR_PTR(err);
}
-done:
+
counter->hw_ops = hw_ops;
return counter;
@@ -2318,10 +2325,10 @@ SYSCALL_DEFINE5(perf_counter_open,
goto err_put_context;
}
- ret = -EINVAL;
counter = perf_counter_alloc(&hw_event, cpu, ctx, group_leader,
GFP_KERNEL);
- if (!counter)
+ ret = PTR_ERR(counter);
+ if (IS_ERR(counter))
goto err_put_context;
ret = anon_inode_getfd("[perf_counter]", &perf_fops, counter, 0);
@@ -2393,8 +2400,8 @@ inherit_counter(struct perf_counter *parent_counter,
child_counter = perf_counter_alloc(&parent_counter->hw_event,
parent_counter->cpu, child_ctx,
group_leader, GFP_KERNEL);
- if (!child_counter)
- return NULL;
+ if (IS_ERR(child_counter))
+ return child_counter;
/*
* Link it up in the child's context:
@@ -2445,15 +2452,17 @@ static int inherit_group(struct perf_counter *parent_counter,
{
struct perf_counter *leader;
struct perf_counter *sub;
+ struct perf_counter *child_ctr;
leader = inherit_counter(parent_counter, parent, parent_ctx,
child, NULL, child_ctx);
- if (!leader)
- return -ENOMEM;
+ if (IS_ERR(leader))
+ return PTR_ERR(leader);
list_for_each_entry(sub, &parent_counter->sibling_list, list_entry) {
- if (!inherit_counter(sub, parent, parent_ctx,
- child, leader, child_ctx))
- return -ENOMEM;
+ child_ctr = inherit_counter(sub, parent, parent_ctx,
+ child, leader, child_ctx);
+ if (IS_ERR(child_ctr))
+ return PTR_ERR(child_ctr);
}
return 0;
}
--
1.5.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists