diff mbox series

interconnect:Replace mutex with rt_mutex for icc_bw_lock

Message ID 20240220074300.10805-1-wangrumeng@xiaomi.corp-partner.google.com
State New
Headers show
Series interconnect:Replace mutex with rt_mutex for icc_bw_lock | expand

Commit Message

Rumeng Wang Feb. 20, 2024, 7:43 a.m. UTC
From: wangrumeng <wangrumeng@xiaomi.corp-partner.google.com>

Replace existing mutex with rt_mutex to prevent priority inversion
between clients, which can cause unacceptable delays in some cases.

Signed-off-by: wangrumeng <wangrumeng@xiaomi.corp-partner.google.com>
---
 drivers/interconnect/core.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Georgi Djakov Feb. 20, 2024, 11:35 a.m. UTC | #1
Hello,

On 20.02.24 9:43, Rumeng Wang wrote:
> From: wangrumeng <wangrumeng@xiaomi.corp-partner.google.com>
> 
> Replace existing mutex with rt_mutex to prevent priority inversion
> between clients, which can cause unacceptable delays in some cases.
> 
> Signed-off-by: wangrumeng <wangrumeng@xiaomi.corp-partner.google.com>

A similar patch [1] has been posted some time ago. Please check the review
comments.

Thanks,
Georgi

[1] https://lore.kernel.org/all/20220906191423.30109-1-quic_mdtipton@quicinc.com/

> ---
>   drivers/interconnect/core.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 50bac2d79d9b..467d42cc7e49 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -14,6 +14,7 @@
>   #include <linux/interconnect-provider.h>
>   #include <linux/list.h>
>   #include <linux/mutex.h>
> +#include <linux/rtmutex.h>
>   #include <linux/slab.h>
>   #include <linux/of.h>
>   #include <linux/overflow.h>
> @@ -28,7 +29,7 @@ static LIST_HEAD(icc_providers);
>   static int providers_count;
>   static bool synced_state;
>   static DEFINE_MUTEX(icc_lock);
> -static DEFINE_MUTEX(icc_bw_lock);
> +static DEFINE_RT_MUTEX(icc_bw_lock);
>   static struct dentry *icc_debugfs_dir;
>   
>   static void icc_summary_show_one(struct seq_file *s, struct icc_node *n)
> @@ -698,7 +699,7 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
>   	if (WARN_ON(IS_ERR(path) || !path->num_nodes))
>   		return -EINVAL;
>   
> -	mutex_lock(&icc_bw_lock);
> +	rt_mutex_lock(&icc_bw_lock);
>   
>   	old_avg = path->reqs[0].avg_bw;
>   	old_peak = path->reqs[0].peak_bw;
> @@ -730,7 +731,7 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
>   		apply_constraints(path);
>   	}
>   
> -	mutex_unlock(&icc_bw_lock);
> +	rt_mutex_unlock(&icc_bw_lock);
>   
>   	trace_icc_set_bw_end(path, ret);
>   
> @@ -939,7 +940,7 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
>   		return;
>   
>   	mutex_lock(&icc_lock);
> -	mutex_lock(&icc_bw_lock);
> +	rt_mutex_lock(&icc_bw_lock);
>   
>   	node->provider = provider;
>   	list_add_tail(&node->node_list, &provider->nodes);
> @@ -968,7 +969,7 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
>   	node->avg_bw = 0;
>   	node->peak_bw = 0;
>   
> -	mutex_unlock(&icc_bw_lock);
> +	rt_mutex_unlock(&icc_bw_lock);
>   	mutex_unlock(&icc_lock);
>   }
>   EXPORT_SYMBOL_GPL(icc_node_add);
> @@ -1094,7 +1095,7 @@ void icc_sync_state(struct device *dev)
>   		return;
>   
>   	mutex_lock(&icc_lock);
> -	mutex_lock(&icc_bw_lock);
> +	rt_mutex_lock(&icc_bw_lock);
>   	synced_state = true;
>   	list_for_each_entry(p, &icc_providers, provider_list) {
>   		dev_dbg(p->dev, "interconnect provider is in synced state\n");
> @@ -1107,7 +1108,7 @@ void icc_sync_state(struct device *dev)
>   			}
>   		}
>   	}
> -	mutex_unlock(&icc_bw_lock);
> +	rt_mutex_unlock(&icc_bw_lock);
>   	mutex_unlock(&icc_lock);
>   }
>   EXPORT_SYMBOL_GPL(icc_sync_state);
Rumeng Wang Feb. 27, 2024, 3:40 a.m. UTC | #2
Hello, it has been a week since the last reply. Is there any progress on this issue?
Thanks.
diff mbox series

Patch

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 50bac2d79d9b..467d42cc7e49 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -14,6 +14,7 @@ 
 #include <linux/interconnect-provider.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/rtmutex.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/overflow.h>
@@ -28,7 +29,7 @@  static LIST_HEAD(icc_providers);
 static int providers_count;
 static bool synced_state;
 static DEFINE_MUTEX(icc_lock);
-static DEFINE_MUTEX(icc_bw_lock);
+static DEFINE_RT_MUTEX(icc_bw_lock);
 static struct dentry *icc_debugfs_dir;
 
 static void icc_summary_show_one(struct seq_file *s, struct icc_node *n)
@@ -698,7 +699,7 @@  int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
 	if (WARN_ON(IS_ERR(path) || !path->num_nodes))
 		return -EINVAL;
 
-	mutex_lock(&icc_bw_lock);
+	rt_mutex_lock(&icc_bw_lock);
 
 	old_avg = path->reqs[0].avg_bw;
 	old_peak = path->reqs[0].peak_bw;
@@ -730,7 +731,7 @@  int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
 		apply_constraints(path);
 	}
 
-	mutex_unlock(&icc_bw_lock);
+	rt_mutex_unlock(&icc_bw_lock);
 
 	trace_icc_set_bw_end(path, ret);
 
@@ -939,7 +940,7 @@  void icc_node_add(struct icc_node *node, struct icc_provider *provider)
 		return;
 
 	mutex_lock(&icc_lock);
-	mutex_lock(&icc_bw_lock);
+	rt_mutex_lock(&icc_bw_lock);
 
 	node->provider = provider;
 	list_add_tail(&node->node_list, &provider->nodes);
@@ -968,7 +969,7 @@  void icc_node_add(struct icc_node *node, struct icc_provider *provider)
 	node->avg_bw = 0;
 	node->peak_bw = 0;
 
-	mutex_unlock(&icc_bw_lock);
+	rt_mutex_unlock(&icc_bw_lock);
 	mutex_unlock(&icc_lock);
 }
 EXPORT_SYMBOL_GPL(icc_node_add);
@@ -1094,7 +1095,7 @@  void icc_sync_state(struct device *dev)
 		return;
 
 	mutex_lock(&icc_lock);
-	mutex_lock(&icc_bw_lock);
+	rt_mutex_lock(&icc_bw_lock);
 	synced_state = true;
 	list_for_each_entry(p, &icc_providers, provider_list) {
 		dev_dbg(p->dev, "interconnect provider is in synced state\n");
@@ -1107,7 +1108,7 @@  void icc_sync_state(struct device *dev)
 			}
 		}
 	}
-	mutex_unlock(&icc_bw_lock);
+	rt_mutex_unlock(&icc_bw_lock);
 	mutex_unlock(&icc_lock);
 }
 EXPORT_SYMBOL_GPL(icc_sync_state);