| Message ID | 20191128141818.32168-3-georgi.djakov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | interconnect: Add basic tracepoints | expand |
On Thu 28 Nov 06:18 PST 2019, Georgi Djakov wrote: > When debugging interconnect things, it turned out that saving the path > name and including it in the traces is quite useful, especially for > devices with multiple paths. > > For the path name we use the one specified in DT, or if we use platform > data, the name is based on the source and destination node names. > > Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > drivers/interconnect/core.c | 18 +++++++++++++++--- > drivers/interconnect/internal.h | 2 ++ > 2 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c > index f30a326dc7ce..c9e16bc1331e 100644 > --- a/drivers/interconnect/core.c > +++ b/drivers/interconnect/core.c > @@ -356,9 +356,17 @@ struct icc_path *of_icc_get(struct device *dev, const char *name) > > mutex_lock(&icc_lock); > path = path_find(dev, src_node, dst_node); > - if (IS_ERR(path)) > - dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); > mutex_unlock(&icc_lock); > + if (IS_ERR(path)) { > + dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); > + return path; > + } > + > + if (name) > + path->name = kstrdup(name, GFP_KERNEL); path->name is declared as const and name is likely to be rodata, so using kstrdup_const() here instead have a good chance of avoiding an unnecessary allocation. > + else > + path->name = kasprintf(GFP_KERNEL, "%s-%s", > + src_node->name, dst_node->name); > > return path; > } > @@ -481,9 +489,12 @@ struct icc_path *icc_get(struct device *dev, const int src_id, const int dst_id) > goto out; > > path = path_find(dev, src, dst); > - if (IS_ERR(path)) > + if (IS_ERR(path)) { > dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); > + goto out; > + } > > + path->name = kasprintf(GFP_KERNEL, "%s-%s", src->name, dst->name); > out: > mutex_unlock(&icc_lock); > return path; > @@ -519,6 +530,7 @@ void icc_put(struct icc_path *path) > } > mutex_unlock(&icc_lock); > > + kfree(path->name); And then kfree_const() here (which will handle both the rodata and dynamically allocated case). Apart from this I think the patch looks good. Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > kfree(path); > } > EXPORT_SYMBOL_GPL(icc_put); > diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h > index 5853e8faf223..bf18cb7239df 100644 > --- a/drivers/interconnect/internal.h > +++ b/drivers/interconnect/internal.h > @@ -29,10 +29,12 @@ struct icc_req { > > /** > * struct icc_path - interconnect path structure > + * @name: a string name of the path (useful for ftrace) > * @num_nodes: number of hops (nodes) > * @reqs: array of the requests applicable to this path of nodes > */ > struct icc_path { > + const char *name; > size_t num_nodes; > struct icc_req reqs[]; > };
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index f30a326dc7ce..c9e16bc1331e 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -356,9 +356,17 @@ struct icc_path *of_icc_get(struct device *dev, const char *name) mutex_lock(&icc_lock); path = path_find(dev, src_node, dst_node); - if (IS_ERR(path)) - dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); mutex_unlock(&icc_lock); + if (IS_ERR(path)) { + dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); + return path; + } + + if (name) + path->name = kstrdup(name, GFP_KERNEL); + else + path->name = kasprintf(GFP_KERNEL, "%s-%s", + src_node->name, dst_node->name); return path; } @@ -481,9 +489,12 @@ struct icc_path *icc_get(struct device *dev, const int src_id, const int dst_id) goto out; path = path_find(dev, src, dst); - if (IS_ERR(path)) + if (IS_ERR(path)) { dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path)); + goto out; + } + path->name = kasprintf(GFP_KERNEL, "%s-%s", src->name, dst->name); out: mutex_unlock(&icc_lock); return path; @@ -519,6 +530,7 @@ void icc_put(struct icc_path *path) } mutex_unlock(&icc_lock); + kfree(path->name); kfree(path); } EXPORT_SYMBOL_GPL(icc_put); diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h index 5853e8faf223..bf18cb7239df 100644 --- a/drivers/interconnect/internal.h +++ b/drivers/interconnect/internal.h @@ -29,10 +29,12 @@ struct icc_req { /** * struct icc_path - interconnect path structure + * @name: a string name of the path (useful for ftrace) * @num_nodes: number of hops (nodes) * @reqs: array of the requests applicable to this path of nodes */ struct icc_path { + const char *name; size_t num_nodes; struct icc_req reqs[]; };
When debugging interconnect things, it turned out that saving the path name and including it in the traces is quite useful, especially for devices with multiple paths. For the path name we use the one specified in DT, or if we use platform data, the name is based on the source and destination node names. Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> --- drivers/interconnect/core.c | 18 +++++++++++++++--- drivers/interconnect/internal.h | 2 ++ 2 files changed, 17 insertions(+), 3 deletions(-)