| 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: <20200507155711.GZ4525@google.com>
Date: Thu, 7 May 2020 08:57:11 -0700
From: Matthias Kaehlcke <mka@...omium.org>
To: Georgi Djakov <georgi.djakov@...aro.org>
Cc: linux-pm@...r.kernel.org, evgreen@...omium.org,
bjorn.andersson@...aro.org, akashast@...eaurora.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] interconnect: Add helpers for enabling/disabling a
path
On Thu, May 07, 2020 at 03:08:46PM +0300, Georgi Djakov wrote:
> There is a repeated pattern in multiple drivers where they want to switch
> the bandwidth between zero and some other value. This is happening often
> in the suspend/resume callbacks. Let's add helper functions to enable and
> disable the path, so that callers don't have to take care of remembering
> the bandwidth values and handle this in the framework instead.
>
> With this patch the users can call icc_disable() and icc_enable() to lower
> their bandwidth request to zero and then restore it back to it's previous
> value.
>
> Suggested-by: Evan Green <evgreen@...omium.org>
> Suggested-by: Bjorn Andersson <bjorn.andersson@...aro.org>
> Signed-off-by: Georgi Djakov <georgi.djakov@...aro.org>
> ---
> v2: https://lore.kernel.org/r/20200428091650.27669-1-georgi.djakov@linaro.org/
> * Extract the common code into __icc_enable() (Matthias)
>
>
> drivers/interconnect/core.c | 39 ++++++++++++++++++++++++++++++++-
> drivers/interconnect/internal.h | 2 ++
> include/linux/interconnect.h | 12 ++++++++++
> 3 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index f5699ed34e43..d5e0f93c942d 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -158,6 +158,7 @@ static struct icc_path *path_init(struct device *dev, struct icc_node *dst,
> hlist_add_head(&path->reqs[i].req_node, &node->req_list);
> path->reqs[i].node = node;
> path->reqs[i].dev = dev;
> + path->reqs[i].enabled = true;
> /* reference to previous node was saved during path traversal */
> node = node->reverse;
> }
> @@ -249,9 +250,12 @@ static int aggregate_requests(struct icc_node *node)
> if (p->pre_aggregate)
> p->pre_aggregate(node);
>
> - hlist_for_each_entry(r, &node->req_list, req_node)
> + hlist_for_each_entry(r, &node->req_list, req_node) {
> + if (!r->enabled)
> + continue;
> p->aggregate(node, r->tag, r->avg_bw, r->peak_bw,
> &node->avg_bw, &node->peak_bw);
> + }
>
> return 0;
> }
> @@ -571,6 +575,39 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> }
> EXPORT_SYMBOL_GPL(icc_set_bw);
>
> +static int __icc_enable(struct icc_path *path, bool enable)
> +{
> + int i;
> +
> + if (!path)
> + return 0;
> +
> + if (WARN_ON(IS_ERR(path) || !path->num_nodes))
> + return -EINVAL;
> +
> + mutex_lock(&icc_lock);
> +
> + for (i = 0; i < path->num_nodes; i++)
> + path->reqs[i].enabled = enable;
> +
> + mutex_unlock(&icc_lock);
> +
> + return icc_set_bw(path, path->reqs[0].avg_bw,
> + path->reqs[0].peak_bw);
> +}
> +
> +int icc_disable(struct icc_path *path)
> +{
> + return __icc_enable(path, false);
> +}
> +EXPORT_SYMBOL_GPL(icc_disable);
> +
> +int icc_enable(struct icc_path *path)
> +{
> + return __icc_enable(path, true);
> +}
> +EXPORT_SYMBOL_GPL(icc_enable);
> +
uber-nit: my brain expects the order enable/disable, just like lock/unlock
or true/false instead of vice-versa, but it's certainly not really
important :)
> /**
> * icc_get() - return a handle for path between two endpoints
> * @dev: the device requesting the path
> diff --git a/drivers/interconnect/internal.h b/drivers/interconnect/internal.h
> index bf18cb7239df..f5f82a5c939e 100644
> --- a/drivers/interconnect/internal.h
> +++ b/drivers/interconnect/internal.h
> @@ -14,6 +14,7 @@
> * @req_node: entry in list of requests for the particular @node
> * @node: the interconnect node to which this constraint applies
> * @dev: reference to the device that sets the constraints
> + * @enabled: indicates whether the path with this request is enabled
> * @tag: path tag (optional)
> * @avg_bw: an integer describing the average bandwidth in kBps
> * @peak_bw: an integer describing the peak bandwidth in kBps
> @@ -22,6 +23,7 @@ struct icc_req {
> struct hlist_node req_node;
> struct icc_node *node;
> struct device *dev;
> + bool enabled;
> u32 tag;
> u32 avg_bw;
> u32 peak_bw;
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> index 770692421f4c..2b7b331c9354 100644
> --- a/include/linux/interconnect.h
> +++ b/include/linux/interconnect.h
> @@ -30,6 +30,8 @@ struct icc_path *icc_get(struct device *dev, const int src_id,
> struct icc_path *of_icc_get(struct device *dev, const char *name);
> struct icc_path *devm_of_icc_get(struct device *dev, const char *name);
> void icc_put(struct icc_path *path);
> +int icc_disable(struct icc_path *path);
> +int icc_enable(struct icc_path *path);
> int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> void icc_set_tag(struct icc_path *path, u32 tag);
>
> @@ -57,6 +59,16 @@ static inline void icc_put(struct icc_path *path)
> {
> }
>
> +static inline int icc_disable(struct icc_path *path)
> +{
> + return 0;
> +}
> +
> +static inline int icc_enable(struct icc_path *path)
> +{
> + return 0;
> +}
> +
> static inline int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> {
> return 0;
Reviewed-by: Matthias Kaehlcke <mka@...omium.org>
Powered by blists - more mailing lists