Skip to content

Commit 03758d6

Browse files
committed
opp: Replace list_kref with a local counter
A kref or refcount isn't the right tool to be used here for counting number of devices that are sharing the static OPPs created for the OPP table. For example, we are reinitializing the kref again, after it reaches a value of 0 and frees the resources, if the static OPPs get added for the same OPP table structure (as the OPP table structure was never freed). That is messy and very unclear. This patch makes parsed_static_opps an unsigned integer and uses it to count the number of users of the static OPPs. The increment and decrement to parsed_static_opps is done under opp_table->lock now to make sure no races are possible if the OPP table is getting added and removed in parallel (which doesn't happen in practice, but can in theory). Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
1 parent ba00331 commit 03758d6

File tree

3 files changed

+32
-48
lines changed

3 files changed

+32
-48
lines changed

drivers/opp/core.c

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,6 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
988988
BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
989989
INIT_LIST_HEAD(&opp_table->opp_list);
990990
kref_init(&opp_table->kref);
991-
kref_init(&opp_table->list_kref);
992991

993992
/* Secure the device table modification */
994993
list_add(&opp_table->node, &opp_tables);
@@ -1072,33 +1071,6 @@ static void _opp_table_kref_release(struct kref *kref)
10721071
mutex_unlock(&opp_table_lock);
10731072
}
10741073

1075-
void _opp_remove_all_static(struct opp_table *opp_table)
1076-
{
1077-
struct dev_pm_opp *opp, *tmp;
1078-
1079-
list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
1080-
if (!opp->dynamic)
1081-
dev_pm_opp_put(opp);
1082-
}
1083-
1084-
opp_table->parsed_static_opps = false;
1085-
}
1086-
1087-
static void _opp_table_list_kref_release(struct kref *kref)
1088-
{
1089-
struct opp_table *opp_table = container_of(kref, struct opp_table,
1090-
list_kref);
1091-
1092-
_opp_remove_all_static(opp_table);
1093-
mutex_unlock(&opp_table_lock);
1094-
}
1095-
1096-
void _put_opp_list_kref(struct opp_table *opp_table)
1097-
{
1098-
kref_put_mutex(&opp_table->list_kref, _opp_table_list_kref_release,
1099-
&opp_table_lock);
1100-
}
1101-
11021074
void dev_pm_opp_put_opp_table(struct opp_table *opp_table)
11031075
{
11041076
kref_put_mutex(&opp_table->kref, _opp_table_kref_release,
@@ -1202,6 +1174,24 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)
12021174
}
12031175
EXPORT_SYMBOL_GPL(dev_pm_opp_remove);
12041176

1177+
void _opp_remove_all_static(struct opp_table *opp_table)
1178+
{
1179+
struct dev_pm_opp *opp, *tmp;
1180+
1181+
mutex_lock(&opp_table->lock);
1182+
1183+
if (!opp_table->parsed_static_opps || --opp_table->parsed_static_opps)
1184+
goto unlock;
1185+
1186+
list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
1187+
if (!opp->dynamic)
1188+
dev_pm_opp_put_unlocked(opp);
1189+
}
1190+
1191+
unlock:
1192+
mutex_unlock(&opp_table->lock);
1193+
}
1194+
12051195
/**
12061196
* dev_pm_opp_remove_all_dynamic() - Remove all dynamically created OPPs
12071197
* @dev: device for which we do this operation
@@ -2276,7 +2266,7 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev)
22762266
return;
22772267
}
22782268

2279-
_put_opp_list_kref(opp_table);
2269+
_opp_remove_all_static(opp_table);
22802270

22812271
/* Drop reference taken by _find_opp_table() */
22822272
dev_pm_opp_put_opp_table(opp_table);

drivers/opp/of.c

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -658,17 +658,15 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
658658
struct dev_pm_opp *opp;
659659

660660
/* OPP table is already initialized for the device */
661+
mutex_lock(&opp_table->lock);
661662
if (opp_table->parsed_static_opps) {
662-
kref_get(&opp_table->list_kref);
663+
opp_table->parsed_static_opps++;
664+
mutex_unlock(&opp_table->lock);
663665
return 0;
664666
}
665667

666-
/*
667-
* Re-initialize list_kref every time we add static OPPs to the OPP
668-
* table as the reference count may be 0 after the last tie static OPPs
669-
* were removed.
670-
*/
671-
kref_init(&opp_table->list_kref);
668+
opp_table->parsed_static_opps = 1;
669+
mutex_unlock(&opp_table->lock);
672670

673671
/* We have opp-table node now, iterate over it and add OPPs */
674672
for_each_available_child_of_node(opp_table->np, np) {
@@ -678,7 +676,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
678676
dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
679677
ret);
680678
of_node_put(np);
681-
goto put_list_kref;
679+
goto remove_static_opp;
682680
} else if (opp) {
683681
count++;
684682
}
@@ -687,7 +685,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
687685
/* There should be one of more OPP defined */
688686
if (WARN_ON(!count)) {
689687
ret = -ENOENT;
690-
goto put_list_kref;
688+
goto remove_static_opp;
691689
}
692690

693691
list_for_each_entry(opp, &opp_table->opp_list, node)
@@ -698,18 +696,16 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
698696
dev_err(dev, "Not all nodes have performance state set (%d: %d)\n",
699697
count, pstate_count);
700698
ret = -ENOENT;
701-
goto put_list_kref;
699+
goto remove_static_opp;
702700
}
703701

704702
if (pstate_count)
705703
opp_table->genpd_performance_state = true;
706704

707-
opp_table->parsed_static_opps = true;
708-
709705
return 0;
710706

711-
put_list_kref:
712-
_put_opp_list_kref(opp_table);
707+
remove_static_opp:
708+
_opp_remove_all_static(opp_table);
713709

714710
return ret;
715711
}
@@ -746,7 +742,7 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)
746742
if (ret) {
747743
dev_err(dev, "%s: Failed to add OPP %ld (%d)\n",
748744
__func__, freq, ret);
749-
_put_opp_list_kref(opp_table);
745+
_opp_remove_all_static(opp_table);
750746
return ret;
751747
}
752748
nr -= 2;

drivers/opp/opp.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,10 @@ enum opp_table_access {
127127
* @dev_list: list of devices that share these OPPs
128128
* @opp_list: table of opps
129129
* @kref: for reference count of the table.
130-
* @list_kref: for reference count of the OPP list.
131130
* @lock: mutex protecting the opp_list and dev_list.
132131
* @np: struct device_node pointer for opp's DT node.
133132
* @clock_latency_ns_max: Max clock latency in nanoseconds.
134-
* @parsed_static_opps: True if OPPs are initialized from DT.
133+
* @parsed_static_opps: Count of devices for which OPPs are initialized from DT.
135134
* @shared_opp: OPP is shared between multiple devices.
136135
* @suspend_opp: Pointer to OPP to be used during device suspend.
137136
* @genpd_virt_dev_lock: Mutex protecting the genpd virtual device pointers.
@@ -167,7 +166,6 @@ struct opp_table {
167166
struct list_head dev_list;
168167
struct list_head opp_list;
169168
struct kref kref;
170-
struct kref list_kref;
171169
struct mutex lock;
172170

173171
struct device_node *np;
@@ -176,7 +174,7 @@ struct opp_table {
176174
/* For backward compatibility with v1 bindings */
177175
unsigned int voltage_tolerance_v1;
178176

179-
bool parsed_static_opps;
177+
unsigned int parsed_static_opps;
180178
enum opp_table_access shared_opp;
181179
struct dev_pm_opp *suspend_opp;
182180

0 commit comments

Comments
 (0)