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 |
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);
Hello, it has been a week since the last reply. Is there any progress on this issue? Thanks.
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);