[PATCH] perf, pt, coresight: Clean up address filter structure
From: Alexander Shishkin
Date: Mon Jan 23 2017 - 10:29:20 EST
$WRITEME
Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
---
arch/x86/events/intel/pt.c | 13 ++++--
drivers/hwtracing/coresight/coresight-etm-perf.c | 59 +++++++++++-------------
include/linux/perf_event.h | 14 ++++--
kernel/events/core.c | 26 +++++++----
4 files changed, 63 insertions(+), 49 deletions(-)
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 1c1b9fe705..f693c8ae75 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1100,8 +1100,12 @@ static int pt_event_addr_filters_validate(struct list_head *filters)
int range = 0;
list_for_each_entry(filter, filters, entry) {
- /* PT doesn't support single address triggers */
- if (!filter->range || !filter->size)
+ /*
+ * PT doesn't support single address triggers and
+ * 'start' filters.
+ */
+ if (!filter->size ||
+ filter->action == PERF_ADDR_FILTER_ACTION_START)
return -EOPNOTSUPP;
if (!filter->inode) {
@@ -1141,7 +1145,10 @@ static void pt_event_addr_filters_sync(struct perf_event *event)
filters->filter[range].msr_a = msr_a;
filters->filter[range].msr_b = msr_b;
- filters->filter[range].config = filter->filter ? 1 : 2;
+ if (filter->action == PERF_ADDR_FILTER_ACTION_FILTER)
+ filters->filter[range].config = 1;
+ else
+ filters->filter[range].config = 2;
range++;
}
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 1774196902..70eaa74dc2 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -392,35 +392,26 @@ static int etm_addr_filters_validate(struct list_head *filters)
if (++index > ETM_ADDR_CMP_MAX)
return -EOPNOTSUPP;
+ /* filter::size==0 means single address trigger */
+ if (filter->size) {
+ /*
+ * The existing code relies on START/STOP filters
+ * being address filters.
+ */
+ if (filter->action == PERF_ADDR_FILTER_ACTION_START ||
+ filter->action == PERF_ADDR_FILTER_ACTION_STOP)
+ return -EOPNOTSUPP;
+
+ range = true;
+ } else
+ address = true;
+
/*
- * As taken from the struct perf_addr_filter documentation:
- * @range: 1: range, 0: address
- *
* At this time we don't allow range and start/stop filtering
* to cohabitate, they have to be mutually exclusive.
*/
- if ((filter->range == 1) && address)
+ if (range && address)
return -EOPNOTSUPP;
-
- if ((filter->range == 0) && range)
- return -EOPNOTSUPP;
-
- /*
- * For range filtering, the second address in the address
- * range comparator needs to be higher than the first.
- * Invalid otherwise.
- */
- if (filter->range && filter->size == 0)
- return -EINVAL;
-
- /*
- * Everything checks out with this filter, record what we've
- * received before moving on to the next one.
- */
- if (filter->range)
- range = true;
- else
- address = true;
}
return 0;
@@ -440,18 +431,20 @@ static void etm_addr_filters_sync(struct perf_event *event)
stop = start + filter->size;
etm_filter = &filters->etm_filter[i];
- if (filter->range == 1) {
+ switch (filter->action) {
+ case PERF_ADDR_FILTER_ACTION_FILTER:
etm_filter->start_addr = start;
etm_filter->stop_addr = stop;
etm_filter->type = ETM_ADDR_TYPE_RANGE;
- } else {
- if (filter->filter == 1) {
- etm_filter->start_addr = start;
- etm_filter->type = ETM_ADDR_TYPE_START;
- } else {
- etm_filter->stop_addr = stop;
- etm_filter->type = ETM_ADDR_TYPE_STOP;
- }
+ break;
+ case PERF_ADDR_FILTER_ACTION_START:
+ etm_filter->start_addr = start;
+ etm_filter->type = ETM_ADDR_TYPE_START;
+ break;
+ case PERF_ADDR_FILTER_ACTION_STOP:
+ etm_filter->stop_addr = stop;
+ etm_filter->type = ETM_ADDR_TYPE_STOP;
+ break;
}
i++;
}
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1432df5a82..c09aff84f7 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -457,14 +457,19 @@ struct pmu {
int (*filter_match) (struct perf_event *event); /* optional */
};
+enum perf_addr_filter_action_t {
+ PERF_ADDR_FILTER_ACTION_STOP = 0,
+ PERF_ADDR_FILTER_ACTION_START,
+ PERF_ADDR_FILTER_ACTION_FILTER,
+};
+
/**
* struct perf_addr_filter - address range filter definition
* @entry: event's filter list linkage
* @inode: object file's inode for file-based filters
* @offset: filter range offset
- * @size: filter range size
- * @range: 1: range, 0: address
- * @filter: 1: filter/start, 0: stop
+ * @size: filter range size (size==0 means single address trigger)
+ * @action: filter/start/stop
*
* This is a hardware-agnostic filter configuration as specified by the user.
*/
@@ -473,8 +478,7 @@ struct perf_addr_filter {
struct inode *inode;
unsigned long offset;
unsigned long size;
- unsigned int range : 1,
- filter : 1;
+ enum perf_addr_filter_action_t action;
};
/**
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 39106ae61b..d7a11faac1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8194,7 +8194,8 @@ static void perf_event_addr_filters_apply(struct perf_event *event)
* * for kernel addresses: <start address>[/<size>]
* * for object files: <start address>[/<size>]@</path/to/object/file>
*
- * if <size> is not specified, the range is treated as a single address.
+ * if <size> is not specified or is zero, the range is treated as a single
+ * address; not valid for ACTION=="filter".
*/
enum {
IF_ACT_NONE = -1,
@@ -8244,6 +8245,11 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
return -ENOMEM;
while ((start = strsep(&fstr, " ,\n")) != NULL) {
+ static const enum perf_addr_filter_action_t actions[] = {
+ [IF_ACT_FILTER] = PERF_ADDR_FILTER_ACTION_FILTER,
+ [IF_ACT_START] = PERF_ADDR_FILTER_ACTION_START,
+ [IF_ACT_STOP] = PERF_ADDR_FILTER_ACTION_STOP,
+ };
ret = -EINVAL;
if (!*start)
@@ -8260,12 +8266,11 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
switch (token) {
case IF_ACT_FILTER:
case IF_ACT_START:
- filter->filter = 1;
-
case IF_ACT_STOP:
if (state != IF_STATE_ACTION)
goto fail;
+ filter->action = actions[token];
state = IF_STATE_SOURCE;
break;
@@ -8278,15 +8283,12 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
if (state != IF_STATE_SOURCE)
goto fail;
- if (token == IF_SRC_FILE || token == IF_SRC_KERNEL)
- filter->range = 1;
-
*args[0].to = 0;
ret = kstrtoul(args[0].from, 0, &filter->offset);
if (ret)
goto fail;
- if (filter->range) {
+ if (token == IF_SRC_KERNEL || token == IF_SRC_FILE) {
*args[1].to = 0;
ret = kstrtoul(args[1].from, 0, &filter->size);
if (ret)
@@ -8294,7 +8296,7 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
}
if (token == IF_SRC_FILE || token == IF_SRC_FILEADDR) {
- int fpos = filter->range ? 2 : 1;
+ int fpos = token == IF_SRC_FILE ? 2 : 1;
filename = match_strdup(&args[fpos]);
if (!filename) {
@@ -8319,6 +8321,14 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
if (kernel && event->attr.exclude_kernel)
goto fail;
+ /*
+ * ACTION "filter" must have a non-zero length region
+ * specified.
+ */
+ if (filter->action == PERF_ADDR_FILTER_ACTION_FILTER &&
+ !filter->size)
+ goto fail;
+
if (!kernel) {
if (!filename)
goto fail;
--
2.11.0