This patchset provides a proposed infrastructure to allow for the automatic selection of a sink during CoreSight tracing operations.
Currently starting tracing using perf requires a sink selection on the command line:-
sudo ./perf record -e cs_etm/@tmc_etr0/ --per-thread uname -a
After this set the infrastructure will be able to select a default sink:-
sudo ./perf record -e cs_etm// --per-thread uname -a
This matches with the default operation provided with perf and intelpt.
Where no sink is specified at the start of a trace session, the CoreSight system will walk the connection graph from the source ETM, to find a suitable sink using the first encountered highest priority device.
The CoreSight infrastructure is updated to define sink sub_types to differentiate between sinks with built in buffers (ETB / ETF) - BUFFER type , and those that use system memory (ETR) - SYSMEM - types.
SYSMEM types are considered higher priority.
When two sinks are found of equal priority, then the closest sink to the source in terms of connection nodes is chosen.
The automatic sink selection will also operate if an ETM is enabled using sysfs commands, and no sink is currently enabled.
Applies to Linux coresight/next branch
Changes since v2: 1) changed from explicit set priority value to using the sink_subtype in the coresight_device as the selection priority value.
2) Added in search depth to the find algorithm to ensure that the closest sink to the source with the highest priority is chosen.
3) A default sink is cached with the source when the search is first undertaken, reducing the need for repeat searches for a given source.
Changes since v1: 1) Dropped the device-tree attribute labelling of sinks for selection and implemented the priority schema preferring first encountered ETR, after mailing list discussions. 2) Added in sysfs support for auto sink selection. Mike Leach (6): coresight: Fix comment in main header file. coresight: Add default sink selection to CoreSight base coresight: tmc: Update sink types for default selection. coresight: etm: perf: Add default sink selection to etm perf coresight: sysfs: Allow select default sink on source enable. perf: cs-etm: Allow no CoreSight sink to be specified on command line
.../hwtracing/coresight/coresight-etm-perf.c | 17 +- drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight-tmc.c | 3 +- drivers/hwtracing/coresight/coresight.c | 148 +++++++++++++++++- include/linux/coresight.h | 6 +- tools/perf/arch/arm/util/cs-etm.c | 6 +- 6 files changed, 172 insertions(+), 10 deletions(-)
Comment for an elemnt in the coresight_device structure appears to have been corrupted & makes no sense. Fix this before making further changes.
Signed-off-by: Mike Leach mike.leach@linaro.org --- include/linux/coresight.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index e3e9f0e3a878..84dc695e87d4 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -179,7 +179,8 @@ struct coresight_sysfs_link { * @enable: 'true' if component is currently part of an active path. * @activated: 'true' only if a _sink_ has been activated. A sink can be * activated but not yet enabled. Enabling for a _sink_ - * appens when a source has been selected for that it. + * happens when a source has been selected and a path is enabled + * from source to that sink. * @ea: Device attribute for sink representation under PMU directory. * @ect_dev: Associated cross trigger device. Not part of the trace data * path or connections.
On 05/19/2020 06:31 PM, Mike Leach wrote:
Comment for an elemnt in the coresight_device structure appears to have been corrupted & makes no sense. Fix this before making further changes.
Signed-off-by: Mike Leach mike.leach@linaro.org
include/linux/coresight.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index e3e9f0e3a878..84dc695e87d4 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -179,7 +179,8 @@ struct coresight_sysfs_link {
- @enable: 'true' if component is currently part of an active path.
- @activated: 'true' only if a _sink_ has been activated. A sink can be
activated but not yet enabled. Enabling for a _sink_
appens when a source has been selected for that it.
happens when a source has been selected and a path is enabled
from source to that sink.
- @ea: Device attribute for sink representation under PMU directory.
- @ect_dev: Associated cross trigger device. Not part of the trace data
path or connections.
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Adds a method to select a suitable sink connected to a given source.
In cases where no sink is defined, the coresight_find_default_sink routine can search from a given source, through the child connections until a suitable sink is found.
The suitability is defined in by the sink coresight_dev_subtype on the CoreSight device, and the distance from the source by counting connections.
Higher value subtype is preferred - where these are equal, shorter distance from source is used as a tie-break.
This allows for default sink to be discovered were none is specified (e.g. perf command line)
Signed-off-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight.c | 137 +++++++++++++++++++ include/linux/coresight.h | 3 + 3 files changed, 142 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 36c943ae94d5..f2dc625ea585 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -150,6 +150,8 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data); struct coresight_device *coresight_get_sink(struct list_head *path); struct coresight_device *coresight_get_enabled_sink(bool reset); struct coresight_device *coresight_get_sink_by_id(u32 id); +struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev); struct list_head *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct list_head *path); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index f3efbb3b2b4d..96b9ad249c62 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -769,6 +769,143 @@ void coresight_release_path(struct list_head *path) path = NULL; }
+/* return true if the device is a suitable type for a default sink */ +static inline bool coresight_is_def_sink_type(struct coresight_device *csdev) +{ + /* sink & correct subtype */ + if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) || + (csdev->type == CORESIGHT_DEV_TYPE_LINKSINK)) && + (csdev->subtype.sink_subtype >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER)) + return true; + return false; +} + +/** + * coresight_select_best_sink - return the best sink for use as default from + * the two provided. + * + * @sink: current best sink. + * @depth: search depth where current sink was found. + * @new_sink: new sink for comparison with current sink. + * @new_depth: search depth where new sink was found. + * + * Sinks prioritised according to coresight_dev_subtype_sink, with only + * subtypes CORESIGHT_DEV_SUBTYPE_SINK_BUFFER or higher being used. + * + * Where two sinks of equal priority are found, the sink closest to the + * source is used (smallest search depth). + * + * return @new_sink & update @depth if better than @sink, else return @sink. + */ +static struct coresight_device * +coresight_select_best_sink(struct coresight_device *sink, int *depth, + struct coresight_device *new_sink, int new_depth) +{ + bool update = false; + + if (!sink) { + /* first found at this level */ + update = true; + } else if (new_sink->subtype.sink_subtype > + sink->subtype.sink_subtype) { + /* found better sink */ + update = true; + } else if ((new_sink->subtype.sink_subtype == + sink->subtype.sink_subtype) && + (*depth > new_depth)) { + /* found same but closer sink */ + update = true; + } + + if (update) + *depth = new_depth; + return update ? new_sink : sink; +} + +/** + * coresight_find_sink - recursive function to walk trace connections from + * source to find a suitable default sink. + * + * @csdev: source / current device to check. + * @depth: [in] search depth of calling dev, [out] depth of found sink. + * + * This will walk the connection path from a source (ETM) till a suitable + * sink is encountered and return that sink to the original caller. + * + * If current device is a plain sink return that & depth, otherwise recursively + * call child connections looking for a sink. Select best possible using + * coresight_select_best_sink. + * + * return best sink found, or NULL if not found at this node or child nodes. + */ +static struct coresight_device * +coresight_find_sink(struct coresight_device *csdev, + int *depth) +{ + int i, curr_depth = *depth+1, found_depth = 0; + struct coresight_device *found_sink = NULL; + + if (coresight_is_def_sink_type(csdev)) { + found_depth = curr_depth; + found_sink = csdev; + if (csdev->type == CORESIGHT_DEV_TYPE_SINK) + goto coresight_return_sink; + /* look past LINKSINK for something better */ + } + + /* + * Not a sink we want - or possible child sink may be better. + * recursively explore each port found on this element. + */ + for (i = 0; i < csdev->pdata->nr_outport; i++) { + struct coresight_device *child_dev, *sink = NULL; + int child_depth = curr_depth; + + child_dev = csdev->pdata->conns[i].child_dev; + if (child_dev) + sink = coresight_find_sink(child_dev, &child_depth); + + + if (sink) + found_sink = coresight_select_best_sink(found_sink, + &found_depth, + sink, + child_depth); + } + +coresight_return_sink: + /* return found sink and depth */ + if (found_sink) + *depth = found_depth; + return found_sink; +} + +/** + * coresight_find_default_sink: Find a sink suitable for use as a + * default sink. + * + * @csdev: starting source to find a connected sink. + * + * Walks connections graph looking for a suitable sink to enable for the + * supplied source. Uses csdev->priority values. + * + * If a sink is found, then the default sink for this device is set and + * will be automatically used in future. + * + * Used in cases where the CoreSight user (perf / sysfs) has not selected a + * sink. + */ +struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev) +{ + int depth = 0; + + /* look for a default sink if we have not found for this device */ + if (!csdev->def_sink) + csdev->def_sink = coresight_find_sink(csdev, &depth); + return csdev->def_sink; +} + /** coresight_validate_source - make sure a source has the right credentials * @csdev: the device structure for a source. * @function: the function this was called from. diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 84dc695e87d4..58fffdecdbfd 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -48,6 +48,7 @@ enum coresight_dev_subtype_sink { CORESIGHT_DEV_SUBTYPE_SINK_NONE, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER, + CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM, };
enum coresight_dev_subtype_link { @@ -182,6 +183,7 @@ struct coresight_sysfs_link { * happens when a source has been selected and a path is enabled * from source to that sink. * @ea: Device attribute for sink representation under PMU directory. + * @def_sink: cached reference to default sink found for this device. * @ect_dev: Associated cross trigger device. Not part of the trace data * path or connections. * @nr_links: number of sysfs links created to other components from this @@ -200,6 +202,7 @@ struct coresight_device { /* sink specific fields */ bool activated; /* true only if a sink is part of a path */ struct dev_ext_attribute *ea; + struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev; /* sysfs links between components */
On Tue, May 19, 2020 at 06:31:24PM +0100, Mike Leach wrote:
Adds a method to select a suitable sink connected to a given source.
In cases where no sink is defined, the coresight_find_default_sink routine can search from a given source, through the child connections until a suitable sink is found.
The suitability is defined in by the sink coresight_dev_subtype on the CoreSight device, and the distance from the source by counting connections.
Higher value subtype is preferred - where these are equal, shorter distance from source is used as a tie-break.
This allows for default sink to be discovered were none is specified (e.g. perf command line)
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight.c | 137 +++++++++++++++++++ include/linux/coresight.h | 3 + 3 files changed, 142 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 36c943ae94d5..f2dc625ea585 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -150,6 +150,8 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data); struct coresight_device *coresight_get_sink(struct list_head *path); struct coresight_device *coresight_get_enabled_sink(bool reset); struct coresight_device *coresight_get_sink_by_id(u32 id); +struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev); struct list_head *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct list_head *path); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index f3efbb3b2b4d..96b9ad249c62 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -769,6 +769,143 @@ void coresight_release_path(struct list_head *path) path = NULL; } +/* return true if the device is a suitable type for a default sink */ +static inline bool coresight_is_def_sink_type(struct coresight_device *csdev) +{
- /* sink & correct subtype */
- if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) ||
(csdev->type == CORESIGHT_DEV_TYPE_LINKSINK)) &&
(csdev->subtype.sink_subtype >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER))
return true;
- return false;
+}
+/**
- coresight_select_best_sink - return the best sink for use as default from
- the two provided.
- @sink: current best sink.
- @depth: search depth where current sink was found.
- @new_sink: new sink for comparison with current sink.
- @new_depth: search depth where new sink was found.
- Sinks prioritised according to coresight_dev_subtype_sink, with only
- subtypes CORESIGHT_DEV_SUBTYPE_SINK_BUFFER or higher being used.
- Where two sinks of equal priority are found, the sink closest to the
- source is used (smallest search depth).
- return @new_sink & update @depth if better than @sink, else return @sink.
- */
+static struct coresight_device * +coresight_select_best_sink(struct coresight_device *sink, int *depth,
struct coresight_device *new_sink, int new_depth)
+{
- bool update = false;
- if (!sink) {
/* first found at this level */
update = true;
- } else if (new_sink->subtype.sink_subtype >
sink->subtype.sink_subtype) {
/* found better sink */
update = true;
- } else if ((new_sink->subtype.sink_subtype ==
sink->subtype.sink_subtype) &&
(*depth > new_depth)) {
/* found same but closer sink */
update = true;
- }
- if (update)
*depth = new_depth;
- return update ? new_sink : sink;
+}
+/**
- coresight_find_sink - recursive function to walk trace connections from
- source to find a suitable default sink.
- @csdev: source / current device to check.
- @depth: [in] search depth of calling dev, [out] depth of found sink.
- This will walk the connection path from a source (ETM) till a suitable
- sink is encountered and return that sink to the original caller.
- If current device is a plain sink return that & depth, otherwise recursively
- call child connections looking for a sink. Select best possible using
- coresight_select_best_sink.
- return best sink found, or NULL if not found at this node or child nodes.
- */
+static struct coresight_device * +coresight_find_sink(struct coresight_device *csdev,
int *depth)
This doesn't have to be on a separate line.
+{
- int i, curr_depth = *depth+1, found_depth = 0;
s/*depth+1/*depth + 1 or *depth++
- struct coresight_device *found_sink = NULL;
- if (coresight_is_def_sink_type(csdev)) {
found_depth = curr_depth;
found_sink = csdev;
if (csdev->type == CORESIGHT_DEV_TYPE_SINK)
goto coresight_return_sink;
/* look past LINKSINK for something better */
- }
- /*
* Not a sink we want - or possible child sink may be better.
* recursively explore each port found on this element.
*/
- for (i = 0; i < csdev->pdata->nr_outport; i++) {
struct coresight_device *child_dev, *sink = NULL;
int child_depth = curr_depth;
child_dev = csdev->pdata->conns[i].child_dev;
if (child_dev)
sink = coresight_find_sink(child_dev, &child_depth);
Exta new line.
if (sink)
found_sink = coresight_select_best_sink(found_sink,
&found_depth,
sink,
child_depth);
- }
+coresight_return_sink:
- /* return found sink and depth */
- if (found_sink)
*depth = found_depth;
- return found_sink;
I spent half an hour trying to poke holes in the above algorithm without success. If we stick to the CS-BSA, this should work.
+}
+/**
- coresight_find_default_sink: Find a sink suitable for use as a
- default sink.
- @csdev: starting source to find a connected sink.
- Walks connections graph looking for a suitable sink to enable for the
- supplied source. Uses csdev->priority values.
The last sentence no longer applies.
- If a sink is found, then the default sink for this device is set and
- will be automatically used in future.
- Used in cases where the CoreSight user (perf / sysfs) has not selected a
- sink.
- */
+struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev) +{
- int depth = 0;
- /* look for a default sink if we have not found for this device */
- if (!csdev->def_sink)
csdev->def_sink = coresight_find_sink(csdev, &depth);
- return csdev->def_sink;
+}
/** coresight_validate_source - make sure a source has the right credentials
- @csdev: the device structure for a source.
- @function: the function this was called from.
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 84dc695e87d4..58fffdecdbfd 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -48,6 +48,7 @@ enum coresight_dev_subtype_sink { CORESIGHT_DEV_SUBTYPE_SINK_NONE, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
- CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM,
}; enum coresight_dev_subtype_link { @@ -182,6 +183,7 @@ struct coresight_sysfs_link {
happens when a source has been selected and a path is enabled
from source to that sink.
- @ea: Device attribute for sink representation under PMU directory.
- @def_sink: cached reference to default sink found for this device.
- @ect_dev: Associated cross trigger device. Not part of the trace data
path or connections.
- @nr_links: number of sysfs links created to other components from this
@@ -200,6 +202,7 @@ struct coresight_device { /* sink specific fields */ bool activated; /* true only if a sink is part of a path */ struct dev_ext_attribute *ea;
- struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev; /* sysfs links between components */
-- 2.17.1
Hi Mike,
Several minor comments.
On Tue, May 19, 2020 at 06:31:24PM +0100, Mike Leach wrote:
Adds a method to select a suitable sink connected to a given source.
In cases where no sink is defined, the coresight_find_default_sink routine can search from a given source, through the child connections until a suitable sink is found.
The suitability is defined in by the sink coresight_dev_subtype on the CoreSight device, and the distance from the source by counting connections.
Higher value subtype is preferred - where these are equal, shorter distance from source is used as a tie-break.
This allows for default sink to be discovered were none is specified (e.g. perf command line)
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight.c | 137 +++++++++++++++++++ include/linux/coresight.h | 3 + 3 files changed, 142 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 36c943ae94d5..f2dc625ea585 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -150,6 +150,8 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data); struct coresight_device *coresight_get_sink(struct list_head *path); struct coresight_device *coresight_get_enabled_sink(bool reset); struct coresight_device *coresight_get_sink_by_id(u32 id); +struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev); struct list_head *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct list_head *path); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index f3efbb3b2b4d..96b9ad249c62 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -769,6 +769,143 @@ void coresight_release_path(struct list_head *path) path = NULL; } +/* return true if the device is a suitable type for a default sink */ +static inline bool coresight_is_def_sink_type(struct coresight_device *csdev) +{
- /* sink & correct subtype */
- if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) ||
(csdev->type == CORESIGHT_DEV_TYPE_LINKSINK)) &&
(csdev->subtype.sink_subtype >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER))
return true;
Seems to me, we don't ned to check 'csdev->type' thus can simplify the condition checking:
if (csdev->subtype.sink_subtype == CORESIGHT_DEV_SUBTYPE_SINK_BUFFER || csdev->subtype.sink_subtype == CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM) return true;
- return false;
+}
+/**
- coresight_select_best_sink - return the best sink for use as default from
- the two provided.
- @sink: current best sink.
- @depth: search depth where current sink was found.
- @new_sink: new sink for comparison with current sink.
- @new_depth: search depth where new sink was found.
- Sinks prioritised according to coresight_dev_subtype_sink, with only
- subtypes CORESIGHT_DEV_SUBTYPE_SINK_BUFFER or higher being used.
- Where two sinks of equal priority are found, the sink closest to the
- source is used (smallest search depth).
- return @new_sink & update @depth if better than @sink, else return @sink.
- */
+static struct coresight_device * +coresight_select_best_sink(struct coresight_device *sink, int *depth,
struct coresight_device *new_sink, int new_depth)
+{
- bool update = false;
- if (!sink) {
/* first found at this level */
update = true;
- } else if (new_sink->subtype.sink_subtype >
sink->subtype.sink_subtype) {
/* found better sink */
update = true;
- } else if ((new_sink->subtype.sink_subtype ==
sink->subtype.sink_subtype) &&
(*depth > new_depth)) {
/* found same but closer sink */
update = true;
- }
- if (update)
*depth = new_depth;
- return update ? new_sink : sink;
+}
+/**
- coresight_find_sink - recursive function to walk trace connections from
- source to find a suitable default sink.
- @csdev: source / current device to check.
- @depth: [in] search depth of calling dev, [out] depth of found sink.
- This will walk the connection path from a source (ETM) till a suitable
- sink is encountered and return that sink to the original caller.
- If current device is a plain sink return that & depth, otherwise recursively
- call child connections looking for a sink. Select best possible using
- coresight_select_best_sink.
- return best sink found, or NULL if not found at this node or child nodes.
- */
+static struct coresight_device * +coresight_find_sink(struct coresight_device *csdev,
int *depth)
+{
- int i, curr_depth = *depth+1, found_depth = 0;
- struct coresight_device *found_sink = NULL;
- if (coresight_is_def_sink_type(csdev)) {
found_depth = curr_depth;
found_sink = csdev;
if (csdev->type == CORESIGHT_DEV_TYPE_SINK)
goto coresight_return_sink;
/* look past LINKSINK for something better */
- }
- /*
* Not a sink we want - or possible child sink may be better.
* recursively explore each port found on this element.
*/
- for (i = 0; i < csdev->pdata->nr_outport; i++) {
struct coresight_device *child_dev, *sink = NULL;
int child_depth = curr_depth;
child_dev = csdev->pdata->conns[i].child_dev;
if (child_dev)
sink = coresight_find_sink(child_dev, &child_depth);
if (sink)
found_sink = coresight_select_best_sink(found_sink,
&found_depth,
sink,
child_depth);
- }
+coresight_return_sink:
suggest to use short jump tag: s/coresight_return_sink/return_sink
- /* return found sink and depth */
- if (found_sink)
*depth = found_depth;
- return found_sink;
+}
As Mathieu mentioned, actually I also carefully read up code for the iteration all CoreSight device nodes to find best sink, but I don't find any issue I can say. FWIW:
Reviewed-by: Leo Yan leo.yan@linaro.org
+/**
- coresight_find_default_sink: Find a sink suitable for use as a
- default sink.
- @csdev: starting source to find a connected sink.
- Walks connections graph looking for a suitable sink to enable for the
- supplied source. Uses csdev->priority values.
- If a sink is found, then the default sink for this device is set and
- will be automatically used in future.
- Used in cases where the CoreSight user (perf / sysfs) has not selected a
- sink.
- */
+struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev) +{
- int depth = 0;
- /* look for a default sink if we have not found for this device */
- if (!csdev->def_sink)
csdev->def_sink = coresight_find_sink(csdev, &depth);
- return csdev->def_sink;
+}
/** coresight_validate_source - make sure a source has the right credentials
- @csdev: the device structure for a source.
- @function: the function this was called from.
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 84dc695e87d4..58fffdecdbfd 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -48,6 +48,7 @@ enum coresight_dev_subtype_sink { CORESIGHT_DEV_SUBTYPE_SINK_NONE, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
- CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM,
}; enum coresight_dev_subtype_link { @@ -182,6 +183,7 @@ struct coresight_sysfs_link {
happens when a source has been selected and a path is enabled
from source to that sink.
- @ea: Device attribute for sink representation under PMU directory.
- @def_sink: cached reference to default sink found for this device.
- @ect_dev: Associated cross trigger device. Not part of the trace data
path or connections.
- @nr_links: number of sysfs links created to other components from this
@@ -200,6 +202,7 @@ struct coresight_device { /* sink specific fields */ bool activated; /* true only if a sink is part of a path */ struct dev_ext_attribute *ea;
- struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev; /* sysfs links between components */
-- 2.17.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
Hi Leo,
On Sat, 23 May 2020 at 08:18, Leo Yan leo.yan@linaro.org wrote:
Hi Mike,
Several minor comments.
On Tue, May 19, 2020 at 06:31:24PM +0100, Mike Leach wrote:
Adds a method to select a suitable sink connected to a given source.
In cases where no sink is defined, the coresight_find_default_sink routine can search from a given source, through the child connections until a suitable sink is found.
The suitability is defined in by the sink coresight_dev_subtype on the CoreSight device, and the distance from the source by counting connections.
Higher value subtype is preferred - where these are equal, shorter distance from source is used as a tie-break.
This allows for default sink to be discovered were none is specified (e.g. perf command line)
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight.c | 137 +++++++++++++++++++ include/linux/coresight.h | 3 + 3 files changed, 142 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 36c943ae94d5..f2dc625ea585 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -150,6 +150,8 @@ int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data); struct coresight_device *coresight_get_sink(struct list_head *path); struct coresight_device *coresight_get_enabled_sink(bool reset); struct coresight_device *coresight_get_sink_by_id(u32 id); +struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev); struct list_head *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct list_head *path); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index f3efbb3b2b4d..96b9ad249c62 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -769,6 +769,143 @@ void coresight_release_path(struct list_head *path) path = NULL; }
+/* return true if the device is a suitable type for a default sink */ +static inline bool coresight_is_def_sink_type(struct coresight_device *csdev) +{
/* sink & correct subtype */
if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) ||
(csdev->type == CORESIGHT_DEV_TYPE_LINKSINK)) &&
(csdev->subtype.sink_subtype >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER))
return true;
Seems to me, we don't ned to check 'csdev->type' thus can simplify the condition checking:
if (csdev->subtype.sink_subtype == CORESIGHT_DEV_SUBTYPE_SINK_BUFFER || csdev->subtype.sink_subtype == CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM) return true;
csdev->subtype is a union of all subtypes - so the explicit check for type is required otherwise we could match on source subtypes for example.
Regards
Mike
return false;
+}
+/**
- coresight_select_best_sink - return the best sink for use as default from
- the two provided.
- @sink: current best sink.
- @depth: search depth where current sink was found.
- @new_sink: new sink for comparison with current sink.
- @new_depth: search depth where new sink was found.
- Sinks prioritised according to coresight_dev_subtype_sink, with only
- subtypes CORESIGHT_DEV_SUBTYPE_SINK_BUFFER or higher being used.
- Where two sinks of equal priority are found, the sink closest to the
- source is used (smallest search depth).
- return @new_sink & update @depth if better than @sink, else return @sink.
- */
+static struct coresight_device * +coresight_select_best_sink(struct coresight_device *sink, int *depth,
struct coresight_device *new_sink, int new_depth)
+{
bool update = false;
if (!sink) {
/* first found at this level */
update = true;
} else if (new_sink->subtype.sink_subtype >
sink->subtype.sink_subtype) {
/* found better sink */
update = true;
} else if ((new_sink->subtype.sink_subtype ==
sink->subtype.sink_subtype) &&
(*depth > new_depth)) {
/* found same but closer sink */
update = true;
}
if (update)
*depth = new_depth;
return update ? new_sink : sink;
+}
+/**
- coresight_find_sink - recursive function to walk trace connections from
- source to find a suitable default sink.
- @csdev: source / current device to check.
- @depth: [in] search depth of calling dev, [out] depth of found sink.
- This will walk the connection path from a source (ETM) till a suitable
- sink is encountered and return that sink to the original caller.
- If current device is a plain sink return that & depth, otherwise recursively
- call child connections looking for a sink. Select best possible using
- coresight_select_best_sink.
- return best sink found, or NULL if not found at this node or child nodes.
- */
+static struct coresight_device * +coresight_find_sink(struct coresight_device *csdev,
int *depth)
+{
int i, curr_depth = *depth+1, found_depth = 0;
struct coresight_device *found_sink = NULL;
if (coresight_is_def_sink_type(csdev)) {
found_depth = curr_depth;
found_sink = csdev;
if (csdev->type == CORESIGHT_DEV_TYPE_SINK)
goto coresight_return_sink;
/* look past LINKSINK for something better */
}
/*
* Not a sink we want - or possible child sink may be better.
* recursively explore each port found on this element.
*/
for (i = 0; i < csdev->pdata->nr_outport; i++) {
struct coresight_device *child_dev, *sink = NULL;
int child_depth = curr_depth;
child_dev = csdev->pdata->conns[i].child_dev;
if (child_dev)
sink = coresight_find_sink(child_dev, &child_depth);
if (sink)
found_sink = coresight_select_best_sink(found_sink,
&found_depth,
sink,
child_depth);
}
+coresight_return_sink:
suggest to use short jump tag: s/coresight_return_sink/return_sink
/* return found sink and depth */
if (found_sink)
*depth = found_depth;
return found_sink;
+}
As Mathieu mentioned, actually I also carefully read up code for the iteration all CoreSight device nodes to find best sink, but I don't find any issue I can say. FWIW:
Reviewed-by: Leo Yan leo.yan@linaro.org
+/**
- coresight_find_default_sink: Find a sink suitable for use as a
- default sink.
- @csdev: starting source to find a connected sink.
- Walks connections graph looking for a suitable sink to enable for the
- supplied source. Uses csdev->priority values.
- If a sink is found, then the default sink for this device is set and
- will be automatically used in future.
- Used in cases where the CoreSight user (perf / sysfs) has not selected a
- sink.
- */
+struct coresight_device * +coresight_find_default_sink(struct coresight_device *csdev) +{
int depth = 0;
/* look for a default sink if we have not found for this device */
if (!csdev->def_sink)
csdev->def_sink = coresight_find_sink(csdev, &depth);
return csdev->def_sink;
+}
/** coresight_validate_source - make sure a source has the right credentials
- @csdev: the device structure for a source.
- @function: the function this was called from.
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 84dc695e87d4..58fffdecdbfd 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -48,6 +48,7 @@ enum coresight_dev_subtype_sink { CORESIGHT_DEV_SUBTYPE_SINK_NONE, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER,
CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM,
};
enum coresight_dev_subtype_link { @@ -182,6 +183,7 @@ struct coresight_sysfs_link {
happens when a source has been selected and a path is enabled
from source to that sink.
- @ea: Device attribute for sink representation under PMU directory.
- @def_sink: cached reference to default sink found for this device.
- @ect_dev: Associated cross trigger device. Not part of the trace data
path or connections.
- @nr_links: number of sysfs links created to other components from this
@@ -200,6 +202,7 @@ struct coresight_device { /* sink specific fields */ bool activated; /* true only if a sink is part of a path */ struct dev_ext_attribute *ea;
struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev; /* sysfs links between components */
-- 2.17.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
An additional sink subtype is added to differentiate ETB/ETF buffer sinks and ETR type system memory sinks.
This allows the prioritised selection of default sinks.
Signed-off-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-tmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 39fba1d16e6e..0d2eb7e0e1bb 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -484,7 +484,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) break; case TMC_CONFIG_TYPE_ETR: desc.type = CORESIGHT_DEV_TYPE_SINK; - desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; + desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM; desc.ops = &tmc_etr_cs_ops; ret = tmc_etr_setup_caps(dev, devid, coresight_get_uci_data(id)); @@ -496,6 +496,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) break; case TMC_CONFIG_TYPE_ETF: desc.type = CORESIGHT_DEV_TYPE_LINKSINK; + desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO; desc.ops = &tmc_etf_cs_ops; dev_list = &etf_devs;
Add default sink selection to the perf trace handling in the etm driver. Uses the select default sink infrastructure to select a sink for the perf session, if no other sink is specified.
Signed-off-by: Mike Leach mike.leach@linaro.org --- .../hwtracing/coresight/coresight-etm-perf.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 84f1dcb69827..1a3169e69bb1 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -226,9 +226,6 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, sink = coresight_get_enabled_sink(true); }
- if (!sink) - goto err; - mask = &event_data->mask;
/* @@ -253,6 +250,16 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, continue; }
+ /* + * No sink provided - look for a default sink for one of the + * devices. At present we only support topology where all CPUs + * use the same sink [N:1], so only need to find one sink. The + * coresight_build_path later will remove any CPU that does not + * attach to the sink, or if we have not found a sink. + */ + if (!sink) + sink = coresight_find_default_sink(csdev); + /* * Building a path doesn't enable it, it simply builds a * list of devices from source to sink that can be @@ -267,6 +274,10 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, *etm_event_cpu_path_ptr(event_data, cpu) = path; }
+ /* no sink found for any CPU - cannot trace */ + if (!sink) + goto err; + /* If we don't have any CPUs ready for tracing, abort */ cpu = cpumask_first(mask); if (cpu >= nr_cpu_ids)
When enabling a trace source using sysfs, allow the CoreSight system to auto-select a default sink if none has been enabled by the user.
Uses the sink select algorithm that uses the default select priorities set when sinks are registered with the system. At present this will prefer ETR over ETB / ETF.
Signed-off-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 96b9ad249c62..6729b92c7b6a 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -966,8 +966,15 @@ int coresight_enable(struct coresight_device *csdev) */ sink = coresight_get_enabled_sink(false); if (!sink) { - ret = -EINVAL; - goto out; + /* look for a default sink if nothing enabled */ + sink = coresight_find_default_sink(csdev); + if (!sink) { + ret = -EINVAL; + goto out; + } + /* mark the default as enabled */ + sink->activated = true; + dev_info(&sink->dev, "Enabled default sink."); }
path = coresight_build_path(csdev, sink);
Adjust the handling of the session sink selection to allow no sink to be selected on the command line. This then forwards the sink selection to the CoreSight infrastructure which will attempt to select a sink based on the default sink select priorities.
Signed-off-by: Mike Leach mike.leach@linaro.org --- tools/perf/arch/arm/util/cs-etm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index 941f814820b8..ed9ea2c60f27 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -242,10 +242,10 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu, }
/* - * No sink was provided on the command line - for _now_ treat - * this as an error. + * No sink was provided on the command line - allow the CoreSight + * system to look for a default */ - return ret; + return 0; }
static int cs_etm_recording_options(struct auxtrace_record *itr,
On Tue, May 19, 2020 at 06:31:28PM +0100, Mike Leach wrote:
Adjust the handling of the session sink selection to allow no sink to be selected on the command line. This then forwards the sink selection to the CoreSight infrastructure which will attempt to select a sink based on the default sink select priorities.
Signed-off-by: Mike Leach mike.leach@linaro.org
tools/perf/arch/arm/util/cs-etm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index 941f814820b8..ed9ea2c60f27 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -242,10 +242,10 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu, } /*
* No sink was provided on the command line - for _now_ treat
* this as an error.
* No sink was provided on the command line - allow the CoreSight
*/* system to look for a default
- return ret;
- return 0;
Tested on my Juno-r2 board and works as expected. FWIW:
Tested-by: Leo Yan leo.yan@linaro.org
} static int cs_etm_recording_options(struct auxtrace_record *itr, -- 2.17.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
Hi Mike,
On Tue, May 19, 2020 at 06:31:22PM +0100, Mike Leach wrote:
This patchset provides a proposed infrastructure to allow for the automatic selection of a sink during CoreSight tracing operations.
Currently starting tracing using perf requires a sink selection on the command line:-
sudo ./perf record -e cs_etm/@tmc_etr0/ --per-thread uname -a
After this set the infrastructure will be able to select a default sink:-
sudo ./perf record -e cs_etm// --per-thread uname -a
This matches with the default operation provided with perf and intelpt.
Where no sink is specified at the start of a trace session, the CoreSight system will walk the connection graph from the source ETM, to find a suitable sink using the first encountered highest priority device.
The CoreSight infrastructure is updated to define sink sub_types to differentiate between sinks with built in buffers (ETB / ETF) - BUFFER type , and those that use system memory (ETR) - SYSMEM - types.
SYSMEM types are considered higher priority.
When two sinks are found of equal priority, then the closest sink to the source in terms of connection nodes is chosen.
The automatic sink selection will also operate if an ETM is enabled using sysfs commands, and no sink is currently enabled.
Applies to Linux coresight/next branch
Despite the minor cosmetic issues I think this set is ready for mainline action. We might even be able to squeeze it in for the upcoming merge window _if_ we get an rc8.
For the next revision please send the user space portion as a separate patch to Arnaldo and CC me (and lakml). It is important that Arnaldo be on the "To:" field, otherwiser he won't see it.
Thanks, Mathieu
Changes since v2:
- changed from explicit set priority value to using the sink_subtype in the
coresight_device as the selection priority value.
- Added in search depth to the find algorithm to ensure that the closest sink
to the source with the highest priority is chosen.
- A default sink is cached with the source when the search is first
undertaken, reducing the need for repeat searches for a given source.
Changes since v1:
- Dropped the device-tree attribute labelling of sinks for selection and
implemented the priority schema preferring first encountered ETR, after mailing list discussions. 2) Added in sysfs support for auto sink selection. Mike Leach (6): coresight: Fix comment in main header file. coresight: Add default sink selection to CoreSight base coresight: tmc: Update sink types for default selection. coresight: etm: perf: Add default sink selection to etm perf coresight: sysfs: Allow select default sink on source enable. perf: cs-etm: Allow no CoreSight sink to be specified on command line
.../hwtracing/coresight/coresight-etm-perf.c | 17 +- drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight-tmc.c | 3 +- drivers/hwtracing/coresight/coresight.c | 148 +++++++++++++++++- include/linux/coresight.h | 6 +- tools/perf/arch/arm/util/cs-etm.c | 6 +- 6 files changed, 172 insertions(+), 10 deletions(-)
-- 2.17.1