| Message ID | 70f672b39780ba7387d15fd6485f94b75d47b1ec.1573692109.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] interconnect: Add interconnect_graph file to debugfs | expand |
On Thu, Nov 14, 2019 at 02:50:49AM +0200, Leonard Crestez wrote: > The interconnect graphs can be difficult to understand and the current > "interconnect_summary" file doesn't even display links in any way. > > Add a new "interconnect_graph" file to debugfs in the graphviz "dot" > format which describes interconnect providers, nodes and links. > > The file is human-readable and can be visualized by piping through > graphviz. Example: > > ssh $TARGET cat /sys/kernel/debug/interconnect/interconnect_graph \ > | dot -Tsvg > interconnect_graph.svg You might want to document this somewhere so we don't all have to go dig it out of the changelog every time we want to look at this file. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > drivers/interconnect/core.c | 66 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > Example output as a github gist: > https://gist.github.com/cdleonard/2f74a7efe74587e3d4b57cf7983b46a8 > > The qcs404 driver was hacked to probe on imx, the links to "0" seem to > from incorrect trailing 0s on DEFINE_QNODE. Possibly fallout from > switching to ARRAY_SIZE(__VA_ARGS__)? > > I'm not sure that "graphviz" is allowed as an output format even in > debugfs. Why not! :) This is great, I love it, nice job, no objection from me. Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Wed 13 Nov 16:50 PST 2019, Leonard Crestez wrote: > The interconnect graphs can be difficult to understand and the current > "interconnect_summary" file doesn't even display links in any way. > > Add a new "interconnect_graph" file to debugfs in the graphviz "dot" > format which describes interconnect providers, nodes and links. > > The file is human-readable and can be visualized by piping through > graphviz. Example: > > ssh $TARGET cat /sys/kernel/debug/interconnect/interconnect_graph \ > | dot -Tsvg > interconnect_graph.svg > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> Nice, I like it! Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > --- > drivers/interconnect/core.c | 66 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > Example output as a github gist: > https://gist.github.com/cdleonard/2f74a7efe74587e3d4b57cf7983b46a8 > > The qcs404 driver was hacked to probe on imx, the links to "0" seem to > from incorrect trailing 0s on DEFINE_QNODE. Possibly fallout from > switching to ARRAY_SIZE(__VA_ARGS__)? > > I'm not sure that "graphviz" is allowed as an output format even in > debugfs. > > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c > index c498796adc07..07e91288c7f4 100644 > --- a/drivers/interconnect/core.c > +++ b/drivers/interconnect/core.c > @@ -92,10 +92,74 @@ static int icc_summary_show(struct seq_file *s, void *data) > > return 0; > } > DEFINE_SHOW_ATTRIBUTE(icc_summary); > > +static void icc_graph_show_link(struct seq_file *s, int level, > + struct icc_node *n, struct icc_node *m) > +{ > + seq_printf(s, "%s\"%d:%s\" -> \"%d:%s\"\n", > + level == 2 ? "\t\t" : "\t", > + n->id, n->name, m->id, m->name); > +} > + > +static void icc_graph_show_node(struct seq_file *s, struct icc_node *n) > +{ > + seq_printf(s, "\t\t\"%d:%s\" [label=\"%d:%s", > + n->id, n->name, n->id, n->name); > + seq_printf(s, "\n\t\t\t|avg_bw=%ukBps", n->avg_bw); > + seq_printf(s, "\n\t\t\t|peak_bw=%ukBps", n->peak_bw); > + seq_puts(s, "\"]\n"); > +} > + > +static int icc_graph_show(struct seq_file *s, void *data) > +{ > + struct icc_provider *provider; > + struct icc_node *n; > + int cluster_index = 0; > + int i; > + > + seq_puts(s, "digraph {\n\trankdir = LR\n\tnode [shape = record]\n"); > + mutex_lock(&icc_lock); > + > + /* draw providers as cluster subgraphs */ > + cluster_index = 0; > + list_for_each_entry(provider, &icc_providers, provider_list) { > + seq_printf(s, "\tsubgraph cluster_%d {\n", ++cluster_index); > + if (provider->dev) > + seq_printf(s, "\t\tlabel = \"%s\"\n", > + dev_name(provider->dev)); > + > + /* draw nodes */ > + list_for_each_entry(n, &provider->nodes, node_list) > + icc_graph_show_node(s, n); > + > + /* draw internal links */ > + list_for_each_entry(n, &provider->nodes, node_list) > + for (i = 0; i < n->num_links; ++i) > + if (n->provider == n->links[i]->provider) > + icc_graph_show_link(s, 2, n, > + n->links[i]); > + > + seq_puts(s, "\t}\n"); > + } > + > + /* draw external links */ > + list_for_each_entry(provider, &icc_providers, provider_list) > + list_for_each_entry(n, &provider->nodes, node_list) > + for (i = 0; i < n->num_links; ++i) > + if (n->provider != n->links[i]->provider) > + icc_graph_show_link(s, 1, n, > + n->links[i]); > + > + mutex_unlock(&icc_lock); > + seq_puts(s, "}"); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(icc_graph); > + > static struct icc_node *node_find(const int id) > { > return idr_find(&icc_idr, id); > } > > @@ -800,10 +864,12 @@ EXPORT_SYMBOL_GPL(icc_provider_del); > static int __init icc_init(void) > { > icc_debugfs_dir = debugfs_create_dir("interconnect", NULL); > debugfs_create_file("interconnect_summary", 0444, > icc_debugfs_dir, NULL, &icc_summary_fops); > + debugfs_create_file("interconnect_graph", 0444, > + icc_debugfs_dir, NULL, &icc_graph_fops); > return 0; > } > > static void __exit icc_exit(void) > { > -- > 2.17.1 >
On 14.11.2019 04:41, Greg Kroah-Hartman wrote: > On Thu, Nov 14, 2019 at 02:50:49AM +0200, Leonard Crestez wrote: >> The interconnect graphs can be difficult to understand and the current >> "interconnect_summary" file doesn't even display links in any way. >> >> Add a new "interconnect_graph" file to debugfs in the graphviz "dot" >> format which describes interconnect providers, nodes and links. >> >> The file is human-readable and can be visualized by piping through >> graphviz. Example: >> >> ssh $TARGET cat /sys/kernel/debug/interconnect/interconnect_graph \ >> | dot -Tsvg > interconnect_graph.svg > > You might want to document this somewhere so we don't all have to go dig > it out of the changelog every time we want to look at this file. Files from sysfs are all described under Documentation/ABI but there's nothing similar for debugfs (and this should definitely not be considered ABI). Maybe Documentation/driver-api/interconnect.rst should have a "debugfs interfaces" paragraph? >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >> --- >> drivers/interconnect/core.c | 66 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 66 insertions(+) >> >> Example output as a github gist: >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgist.github.com%2Fcdleonard%2F2f74a7efe74587e3d4b57cf7983b46a8&data=02%7C01%7Cleonard.crestez%40nxp.com%7C946b54955bda47a2c7a308d768ac2d23%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637092961007878684&sdata=Uk7QI%2FOo70H4H5N3ZZl2IMXMHMvP3vov%2FqSMnPuNWg8%3D&reserved=0 >> >> The qcs404 driver was hacked to probe on imx, the links to "0" seem to >> from incorrect trailing 0s on DEFINE_QNODE. Possibly fallout from >> switching to ARRAY_SIZE(__VA_ARGS__)? >> >> I'm not sure that "graphviz" is allowed as an output format even in >> debugfs. > > Why not! :) > > This is great, I love it, nice job, no objection from me. > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index c498796adc07..07e91288c7f4 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -92,10 +92,74 @@ static int icc_summary_show(struct seq_file *s, void *data) return 0; } DEFINE_SHOW_ATTRIBUTE(icc_summary); +static void icc_graph_show_link(struct seq_file *s, int level, + struct icc_node *n, struct icc_node *m) +{ + seq_printf(s, "%s\"%d:%s\" -> \"%d:%s\"\n", + level == 2 ? "\t\t" : "\t", + n->id, n->name, m->id, m->name); +} + +static void icc_graph_show_node(struct seq_file *s, struct icc_node *n) +{ + seq_printf(s, "\t\t\"%d:%s\" [label=\"%d:%s", + n->id, n->name, n->id, n->name); + seq_printf(s, "\n\t\t\t|avg_bw=%ukBps", n->avg_bw); + seq_printf(s, "\n\t\t\t|peak_bw=%ukBps", n->peak_bw); + seq_puts(s, "\"]\n"); +} + +static int icc_graph_show(struct seq_file *s, void *data) +{ + struct icc_provider *provider; + struct icc_node *n; + int cluster_index = 0; + int i; + + seq_puts(s, "digraph {\n\trankdir = LR\n\tnode [shape = record]\n"); + mutex_lock(&icc_lock); + + /* draw providers as cluster subgraphs */ + cluster_index = 0; + list_for_each_entry(provider, &icc_providers, provider_list) { + seq_printf(s, "\tsubgraph cluster_%d {\n", ++cluster_index); + if (provider->dev) + seq_printf(s, "\t\tlabel = \"%s\"\n", + dev_name(provider->dev)); + + /* draw nodes */ + list_for_each_entry(n, &provider->nodes, node_list) + icc_graph_show_node(s, n); + + /* draw internal links */ + list_for_each_entry(n, &provider->nodes, node_list) + for (i = 0; i < n->num_links; ++i) + if (n->provider == n->links[i]->provider) + icc_graph_show_link(s, 2, n, + n->links[i]); + + seq_puts(s, "\t}\n"); + } + + /* draw external links */ + list_for_each_entry(provider, &icc_providers, provider_list) + list_for_each_entry(n, &provider->nodes, node_list) + for (i = 0; i < n->num_links; ++i) + if (n->provider != n->links[i]->provider) + icc_graph_show_link(s, 1, n, + n->links[i]); + + mutex_unlock(&icc_lock); + seq_puts(s, "}"); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(icc_graph); + static struct icc_node *node_find(const int id) { return idr_find(&icc_idr, id); } @@ -800,10 +864,12 @@ EXPORT_SYMBOL_GPL(icc_provider_del); static int __init icc_init(void) { icc_debugfs_dir = debugfs_create_dir("interconnect", NULL); debugfs_create_file("interconnect_summary", 0444, icc_debugfs_dir, NULL, &icc_summary_fops); + debugfs_create_file("interconnect_graph", 0444, + icc_debugfs_dir, NULL, &icc_graph_fops); return 0; } static void __exit icc_exit(void) {
The interconnect graphs can be difficult to understand and the current "interconnect_summary" file doesn't even display links in any way. Add a new "interconnect_graph" file to debugfs in the graphviz "dot" format which describes interconnect providers, nodes and links. The file is human-readable and can be visualized by piping through graphviz. Example: ssh $TARGET cat /sys/kernel/debug/interconnect/interconnect_graph \ | dot -Tsvg > interconnect_graph.svg Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/interconnect/core.c | 66 +++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) Example output as a github gist: https://gist.github.com/cdleonard/2f74a7efe74587e3d4b57cf7983b46a8 The qcs404 driver was hacked to probe on imx, the links to "0" seem to from incorrect trailing 0s on DEFINE_QNODE. Possibly fallout from switching to ARRAY_SIZE(__VA_ARGS__)? I'm not sure that "graphviz" is allowed as an output format even in debugfs.