linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor
@ 2022-06-13 13:42 Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER Christian 'Ansuel' Marangi
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Christian 'Ansuel' Marangi @ 2022-06-13 13:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Sibi Sankar,
	Saravana Kannan, linux-pm, linux-kernel
  Cc: Christian 'Ansuel' Marangi

While developing a krait cache scaling devfreq driver I encounter tons
of panics and errors with using the new cpufreq passive governor
functions. While the krait cache scaling is still WIP and required some
testing I would like to push all the fixes to make the new
implementation wroking since currently with a the governor
PROBE_DEFERRing all sort of things happen from kernel panic from invalid
address access to freq_table getting corrupted.

With the following fixes my WIP driver works correctly without any
warning/problems. 

v2:
- Fix wrong list_for_each_entry reported by Dan Carpenter

Christian 'Ansuel' Marangi (5):
  PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER
  PM / devfreq: Fix kernel warning with cpufreq passive register fail
  PM / devfreq: Fix kernel panic with cpu based scaling to passive gov
  PM / devfreq: Rework freq_table to be local to devfreq struct
  PM / devfreq: Mute warning on governor PROBE_DEFER

 drivers/devfreq/devfreq.c          | 75 ++++++++++++++----------------
 drivers/devfreq/governor_passive.c | 39 ++++++----------
 include/linux/devfreq.h            |  4 ++
 3 files changed, 53 insertions(+), 65 deletions(-)

-- 
2.36.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER
  2022-06-13 13:42 [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor Christian 'Ansuel' Marangi
@ 2022-06-13 13:42 ` Christian 'Ansuel' Marangi
  2022-06-14  6:59   ` kernel test robot
  2022-06-14  9:01   ` kernel test robot
  2022-06-13 13:42 ` [PATCH v2 2/5] PM / devfreq: Fix kernel warning with cpufreq passive register fail Christian 'Ansuel' Marangi
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 8+ messages in thread
From: Christian 'Ansuel' Marangi @ 2022-06-13 13:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Sibi Sankar,
	Saravana Kannan, linux-pm, linux-kernel
  Cc: Christian 'Ansuel' Marangi

With the passive governor, the cpu based scaling can PROBE_DEFER due to
the fact that CPU policy are not ready.
The cpufreq passive unregister notifier is called both from the
GOV_START errors and for the GOV_STOP and assume the notifier is
successfully registred every time. With GOV_START failing it's wrong to
loop over each possible CPU since the register path has failed for
some CPU policy not ready. Change the logic and unregister the notifer
based on the current allocated parent_cpu_data list to correctly handle
errors and the governor unregister path.

Fixes: a03dacb0316f ("PM / devfreq: Add cpu based scaling support to passive governor")
Signed-off-by: Christian 'Ansuel' Marangi <ansuelsmth@gmail.com>
---
 drivers/devfreq/governor_passive.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 72c67979ebe1..6252f109f8d1 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -223,7 +223,7 @@ static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq)
 	struct devfreq_passive_data *p_data
 			= (struct devfreq_passive_data *)devfreq->data;
 	struct devfreq_cpu_data *parent_cpu_data;
-	int cpu, ret = 0;
+	int ret;
 
 	if (p_data->nb.notifier_call) {
 		ret = cpufreq_unregister_notifier(&p_data->nb,
@@ -232,27 +232,16 @@ static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq)
 			return ret;
 	}
 
-	for_each_possible_cpu(cpu) {
-		struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
-		if (!policy) {
-			ret = -EINVAL;
-			continue;
-		}
-
-		parent_cpu_data = get_parent_cpu_data(p_data, policy);
-		if (!parent_cpu_data) {
-			cpufreq_cpu_put(policy);
-			continue;
-		}
-
+	list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
 		list_del(&parent_cpu_data->node);
+
 		if (parent_cpu_data->opp_table)
 			dev_pm_opp_put_opp_table(parent_cpu_data->opp_table);
+
 		kfree(parent_cpu_data);
-		cpufreq_cpu_put(policy);
 	}
 
-	return ret;
+	return 0;
 }
 
 static int cpufreq_passive_register_notifier(struct devfreq *devfreq)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/5] PM / devfreq: Fix kernel warning with cpufreq passive register fail
  2022-06-13 13:42 [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER Christian 'Ansuel' Marangi
@ 2022-06-13 13:42 ` Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 3/5] PM / devfreq: Fix kernel panic with cpu based scaling to passive gov Christian 'Ansuel' Marangi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Christian 'Ansuel' Marangi @ 2022-06-13 13:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Sibi Sankar,
	Saravana Kannan, linux-pm, linux-kernel
  Cc: Christian 'Ansuel' Marangi

When the cpufreq passive register path from the passive governor fails,
the cpufreq_passive_unregister is called and a kernel WARNING is always
reported.
This is caused by the fact that the devfreq driver already call the
governor unregister with the GOV_STOP, for this reason the second
cpufreq_passive_unregister always return error and a WARN is printed
from the WARN_ON function.
Remove the unregister call from the error handling of the cpufreq register
notifier as it's fundamentally wrong and already handled by the devfreq
core code.

Fixes: a03dacb0316f ("PM / devfreq: Add cpu based scaling support to passive governor")
Signed-off-by: Christian 'Ansuel' Marangi <ansuelsmth@gmail.com>
---
 drivers/devfreq/governor_passive.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 6252f109f8d1..b1d3a1e7e3f4 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -325,7 +325,6 @@ static int cpufreq_passive_register_notifier(struct devfreq *devfreq)
 err_put_policy:
 	cpufreq_cpu_put(policy);
 err:
-	WARN_ON(cpufreq_passive_unregister_notifier(devfreq));
 
 	return ret;
 }
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/5] PM / devfreq: Fix kernel panic with cpu based scaling to passive gov
  2022-06-13 13:42 [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 2/5] PM / devfreq: Fix kernel warning with cpufreq passive register fail Christian 'Ansuel' Marangi
@ 2022-06-13 13:42 ` Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 4/5] PM / devfreq: Rework freq_table to be local to devfreq struct Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 5/5] PM / devfreq: Mute warning on governor PROBE_DEFER Christian 'Ansuel' Marangi
  4 siblings, 0 replies; 8+ messages in thread
From: Christian 'Ansuel' Marangi @ 2022-06-13 13:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Sibi Sankar,
	Saravana Kannan, linux-pm, linux-kernel
  Cc: Christian 'Ansuel' Marangi

The cpufreq passive register notifier can PROBE_DEFER and the devfreq
struct is freed and then reallocaed on probe retry.
The current logic assume that the code can't PROBE_DEFER so the devfreq
struct in the this variable in devfreq_passive_data is assumed to be
(if already set) always correct.
This cause kernel panic as the code try to access the wrong address.
To correctly handle this, update the this variable in
devfreq_passive_data to the devfreq reallocated struct.

Fixes: a03dacb0316f ("PM / devfreq: Add cpu based scaling support to passive governor")
Signed-off-by: Christian 'Ansuel' Marangi <ansuelsmth@gmail.com>
---
 drivers/devfreq/governor_passive.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index b1d3a1e7e3f4..f7f223555c4b 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -395,8 +395,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
 	if (!p_data)
 		return -EINVAL;
 
-	if (!p_data->this)
-		p_data->this = devfreq;
+	p_data->this = devfreq;
 
 	switch (event) {
 	case DEVFREQ_GOV_START:
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/5] PM / devfreq: Rework freq_table to be local to devfreq struct
  2022-06-13 13:42 [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor Christian 'Ansuel' Marangi
                   ` (2 preceding siblings ...)
  2022-06-13 13:42 ` [PATCH v2 3/5] PM / devfreq: Fix kernel panic with cpu based scaling to passive gov Christian 'Ansuel' Marangi
@ 2022-06-13 13:42 ` Christian 'Ansuel' Marangi
  2022-06-13 13:42 ` [PATCH v2 5/5] PM / devfreq: Mute warning on governor PROBE_DEFER Christian 'Ansuel' Marangi
  4 siblings, 0 replies; 8+ messages in thread
From: Christian 'Ansuel' Marangi @ 2022-06-13 13:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Sibi Sankar,
	Saravana Kannan, linux-pm, linux-kernel
  Cc: Christian 'Ansuel' Marangi

Currently we reference the freq_table to the profile defined one and we
make changes on it. Devfreq never supported PROBE_DEFER before the cpu
based scaling support to the passive governor and assumed that a devfreq
device could only had error and be done with it.
Now that a device can PROBE_DEFER a rework to the freq_table logic is
required.

If a device PROBE_DEFER on the GOV_START, the freq_table is already set
in the device profile struct and its init is skipped. This is due to the
fact that it's common for devs to declare this kind of struct static.
This cause the devfreq logic to find a freq table declared (freq_table
not NULL) with random data and poiting to the old addrs freed by devm.

This problem CAN be solved by devs by clearing the freq_table in their
profile struct on driver exit path but it should not be trusted and it
looks to use a flawed logic.

A better solution is to move the freq_table and max_state to the
devfreq struct and never change the profile struct.
This permit to correctly handle PROBE_DEFER since the devfreq struct is
reallocated and contains new values.
Also the profile struct should only be used to init the driver and should
not be used by the devfreq to write the freq_table if it's not provided
by the driver.

Fixes: a03dacb0316f ("PM / devfreq: Add cpu based scaling support to passive governor")
Signed-off-by: Christian 'Ansuel' Marangi <ansuelsmth@gmail.com>
---
 drivers/devfreq/devfreq.c          | 71 ++++++++++++++----------------
 drivers/devfreq/governor_passive.c | 14 +++---
 include/linux/devfreq.h            |  4 ++
 3 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 01474daf4548..2e2b3b414d67 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -123,7 +123,7 @@ void devfreq_get_freq_range(struct devfreq *devfreq,
 			    unsigned long *min_freq,
 			    unsigned long *max_freq)
 {
-	unsigned long *freq_table = devfreq->profile->freq_table;
+	unsigned long *freq_table = devfreq->freq_table;
 	s32 qos_min_freq, qos_max_freq;
 
 	lockdep_assert_held(&devfreq->lock);
@@ -133,11 +133,11 @@ void devfreq_get_freq_range(struct devfreq *devfreq,
 	 * The devfreq drivers can initialize this in either ascending or
 	 * descending order and devfreq core supports both.
 	 */
-	if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
+	if (freq_table[0] < freq_table[devfreq->max_state - 1]) {
 		*min_freq = freq_table[0];
-		*max_freq = freq_table[devfreq->profile->max_state - 1];
+		*max_freq = freq_table[devfreq->max_state - 1];
 	} else {
-		*min_freq = freq_table[devfreq->profile->max_state - 1];
+		*min_freq = freq_table[devfreq->max_state - 1];
 		*max_freq = freq_table[0];
 	}
 
@@ -169,8 +169,8 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
 {
 	int lev;
 
-	for (lev = 0; lev < devfreq->profile->max_state; lev++)
-		if (freq == devfreq->profile->freq_table[lev])
+	for (lev = 0; lev < devfreq->max_state; lev++)
+		if (freq == devfreq->freq_table[lev])
 			return lev;
 
 	return -EINVAL;
@@ -178,7 +178,6 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
 
 static int set_freq_table(struct devfreq *devfreq)
 {
-	struct devfreq_dev_profile *profile = devfreq->profile;
 	struct dev_pm_opp *opp;
 	unsigned long freq;
 	int i, count;
@@ -188,25 +187,22 @@ static int set_freq_table(struct devfreq *devfreq)
 	if (count <= 0)
 		return -EINVAL;
 
-	profile->max_state = count;
-	profile->freq_table = devm_kcalloc(devfreq->dev.parent,
-					profile->max_state,
-					sizeof(*profile->freq_table),
-					GFP_KERNEL);
-	if (!profile->freq_table) {
-		profile->max_state = 0;
+	devfreq->max_state = count;
+	devfreq->freq_table = devm_kcalloc(devfreq->dev.parent,
+					   devfreq->max_state,
+					   sizeof(*devfreq->freq_table),
+					   GFP_KERNEL);
+	if (!devfreq->freq_table)
 		return -ENOMEM;
-	}
 
-	for (i = 0, freq = 0; i < profile->max_state; i++, freq++) {
+	for (i = 0, freq = 0; i < devfreq->max_state; i++, freq++) {
 		opp = dev_pm_opp_find_freq_ceil(devfreq->dev.parent, &freq);
 		if (IS_ERR(opp)) {
-			devm_kfree(devfreq->dev.parent, profile->freq_table);
-			profile->max_state = 0;
+			devm_kfree(devfreq->dev.parent, devfreq->freq_table);
 			return PTR_ERR(opp);
 		}
 		dev_pm_opp_put(opp);
-		profile->freq_table[i] = freq;
+		devfreq->freq_table[i] = freq;
 	}
 
 	return 0;
@@ -246,7 +242,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
 
 	if (lev != prev_lev) {
 		devfreq->stats.trans_table[
-			(prev_lev * devfreq->profile->max_state) + lev]++;
+			(prev_lev * devfreq->max_state) + lev]++;
 		devfreq->stats.total_trans++;
 	}
 
@@ -835,6 +831,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		if (err < 0)
 			goto err_dev;
 		mutex_lock(&devfreq->lock);
+	} else {
+		devfreq->freq_table = devfreq->profile->freq_table;
+		devfreq->max_state = devfreq->profile->max_state;
 	}
 
 	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
@@ -870,8 +869,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	devfreq->stats.trans_table = devm_kzalloc(&devfreq->dev,
 			array3_size(sizeof(unsigned int),
-				    devfreq->profile->max_state,
-				    devfreq->profile->max_state),
+				    devfreq->max_state,
+				    devfreq->max_state),
 			GFP_KERNEL);
 	if (!devfreq->stats.trans_table) {
 		mutex_unlock(&devfreq->lock);
@@ -880,7 +879,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	}
 
 	devfreq->stats.time_in_state = devm_kcalloc(&devfreq->dev,
-			devfreq->profile->max_state,
+			devfreq->max_state,
 			sizeof(*devfreq->stats.time_in_state),
 			GFP_KERNEL);
 	if (!devfreq->stats.time_in_state) {
@@ -1665,9 +1664,9 @@ static ssize_t available_frequencies_show(struct device *d,
 
 	mutex_lock(&df->lock);
 
-	for (i = 0; i < df->profile->max_state; i++)
+	for (i = 0; i < df->max_state; i++)
 		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
-				"%lu ", df->profile->freq_table[i]);
+				"%lu ", df->freq_table[i]);
 
 	mutex_unlock(&df->lock);
 	/* Truncate the trailing space */
@@ -1690,7 +1689,7 @@ static ssize_t trans_stat_show(struct device *dev,
 
 	if (!df->profile)
 		return -EINVAL;
-	max_state = df->profile->max_state;
+	max_state = df->max_state;
 
 	if (max_state == 0)
 		return sprintf(buf, "Not Supported.\n");
@@ -1707,19 +1706,17 @@ static ssize_t trans_stat_show(struct device *dev,
 	len += sprintf(buf + len, "           :");
 	for (i = 0; i < max_state; i++)
 		len += sprintf(buf + len, "%10lu",
-				df->profile->freq_table[i]);
+				df->freq_table[i]);
 
 	len += sprintf(buf + len, "   time(ms)\n");
 
 	for (i = 0; i < max_state; i++) {
-		if (df->profile->freq_table[i]
-					== df->previous_freq) {
+		if (df->freq_table[i] == df->previous_freq)
 			len += sprintf(buf + len, "*");
-		} else {
+		else
 			len += sprintf(buf + len, " ");
-		}
-		len += sprintf(buf + len, "%10lu:",
-				df->profile->freq_table[i]);
+
+		len += sprintf(buf + len, "%10lu:", df->freq_table[i]);
 		for (j = 0; j < max_state; j++)
 			len += sprintf(buf + len, "%10u",
 				df->stats.trans_table[(i * max_state) + j]);
@@ -1743,7 +1740,7 @@ static ssize_t trans_stat_store(struct device *dev,
 	if (!df->profile)
 		return -EINVAL;
 
-	if (df->profile->max_state == 0)
+	if (df->max_state == 0)
 		return count;
 
 	err = kstrtoint(buf, 10, &value);
@@ -1751,11 +1748,11 @@ static ssize_t trans_stat_store(struct device *dev,
 		return -EINVAL;
 
 	mutex_lock(&df->lock);
-	memset(df->stats.time_in_state, 0, (df->profile->max_state *
+	memset(df->stats.time_in_state, 0, (df->max_state *
 					sizeof(*df->stats.time_in_state)));
 	memset(df->stats.trans_table, 0, array3_size(sizeof(unsigned int),
-					df->profile->max_state,
-					df->profile->max_state));
+					df->max_state,
+					df->max_state));
 	df->stats.total_trans = 0;
 	df->stats.last_update = get_jiffies_64();
 	mutex_unlock(&df->lock);
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index f7f223555c4b..bfa79b460dcd 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -131,18 +131,18 @@ static int get_target_freq_with_devfreq(struct devfreq *devfreq,
 		goto out;
 
 	/* Use interpolation if required opps is not available */
-	for (i = 0; i < parent_devfreq->profile->max_state; i++)
-		if (parent_devfreq->profile->freq_table[i] == *freq)
+	for (i = 0; i < parent_devfreq->max_state; i++)
+		if (parent_devfreq->freq_table[i] == *freq)
 			break;
 
-	if (i == parent_devfreq->profile->max_state)
+	if (i == parent_devfreq->max_state)
 		return -EINVAL;
 
-	if (i < devfreq->profile->max_state) {
-		child_freq = devfreq->profile->freq_table[i];
+	if (i < devfreq->max_state) {
+		child_freq = devfreq->freq_table[i];
 	} else {
-		count = devfreq->profile->max_state;
-		child_freq = devfreq->profile->freq_table[count - 1];
+		count = devfreq->max_state;
+		child_freq = devfreq->freq_table[count - 1];
 	}
 
 out:
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index dc10bee75a72..770a7532655c 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -185,6 +185,10 @@ struct devfreq {
 	struct notifier_block nb;
 	struct delayed_work work;
 
+	/* devfreq local freq_table */
+	unsigned long *freq_table;
+	unsigned int max_state;
+
 	unsigned long previous_freq;
 	struct devfreq_dev_status last_status;
 
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 5/5] PM / devfreq: Mute warning on governor PROBE_DEFER
  2022-06-13 13:42 [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor Christian 'Ansuel' Marangi
                   ` (3 preceding siblings ...)
  2022-06-13 13:42 ` [PATCH v2 4/5] PM / devfreq: Rework freq_table to be local to devfreq struct Christian 'Ansuel' Marangi
@ 2022-06-13 13:42 ` Christian 'Ansuel' Marangi
  4 siblings, 0 replies; 8+ messages in thread
From: Christian 'Ansuel' Marangi @ 2022-06-13 13:42 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Sibi Sankar,
	Saravana Kannan, linux-pm, linux-kernel
  Cc: Christian 'Ansuel' Marangi

Don't print warning when a governor PROBE_DEFER as it's not a real
GOV_START fail.

Fixes: a03dacb0316f ("PM / devfreq: Add cpu based scaling support to passive governor")
Signed-off-by: Christian 'Ansuel' Marangi <ansuelsmth@gmail.com>
---
 drivers/devfreq/devfreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2e2b3b414d67..df6972bb0ce8 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -931,8 +931,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
 						NULL);
 	if (err) {
-		dev_err(dev, "%s: Unable to start governor for the device\n",
-			__func__);
+		dev_err_probe(dev, -EPROBE_DEFER, "%s: Unable to start governor for the device\n",
+			      __func__);
 		goto err_init;
 	}
 	create_sysfs_files(devfreq, devfreq->governor);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER
  2022-06-13 13:42 ` [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER Christian 'Ansuel' Marangi
@ 2022-06-14  6:59   ` kernel test robot
  2022-06-14  9:01   ` kernel test robot
  1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-06-14  6:59 UTC (permalink / raw)
  To: Christian 'Ansuel' Marangi, MyungJoo Ham, Kyungmin Park,
	Chanwoo Choi, Sibi Sankar, Saravana Kannan, linux-pm,
	linux-kernel
  Cc: llvm, kbuild-all, Christian 'Ansuel' Marangi

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on chanwoo/devfreq-testing]
[also build test ERROR on linus/master v5.19-rc2 next-20220610]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Ansuel-Marangi/PM-devfreq-Various-Fixes-to-cpufreq-based-passive-governor/20220614-020616
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git devfreq-testing
config: arm-randconfig-r026-20220613 (https://download.01.org/0day-ci/archive/20220614/202206141415.1BsuJvCD-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c97436f8b6e2718286e8496faf53a2c800e281cf)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/378807dd7da24162524c0ba84e996157b3e289c6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Ansuel-Marangi/PM-devfreq-Various-Fixes-to-cpufreq-based-passive-governor/20220614-020616
        git checkout 378807dd7da24162524c0ba84e996157b3e289c6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/devfreq/governor_passive.c:235:72: error: too few arguments provided to function-like macro invocation
           list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
                                                                                 ^
   include/linux/list.h:760:9: note: macro 'list_for_each_entry_safe' defined here
   #define list_for_each_entry_safe(pos, n, head, member)                  \
           ^
>> drivers/devfreq/governor_passive.c:235:2: error: use of undeclared identifier 'list_for_each_entry_safe'
           list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
           ^
   2 errors generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for ARM_CPU_SUSPEND
   Depends on ARCH_SUSPEND_POSSIBLE
   Selected by
   - ARM_TEGRA_CPUIDLE && CPU_IDLE && (ARM || ARM64) && (ARCH_TEGRA || COMPILE_TEST && !ARM64 && MMU
   - ARM_QCOM_SPM_CPUIDLE && CPU_IDLE && (ARM || ARM64) && (ARCH_QCOM || COMPILE_TEST && !ARM64 && MMU


vim +235 drivers/devfreq/governor_passive.c

   220	
   221	static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq)
   222	{
   223		struct devfreq_passive_data *p_data
   224				= (struct devfreq_passive_data *)devfreq->data;
   225		struct devfreq_cpu_data *parent_cpu_data;
   226		int ret;
   227	
   228		if (p_data->nb.notifier_call) {
   229			ret = cpufreq_unregister_notifier(&p_data->nb,
   230						CPUFREQ_TRANSITION_NOTIFIER);
   231			if (ret < 0)
   232				return ret;
   233		}
   234	
 > 235		list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
   236			list_del(&parent_cpu_data->node);
   237	
   238			if (parent_cpu_data->opp_table)
   239				dev_pm_opp_put_opp_table(parent_cpu_data->opp_table);
   240	
   241			kfree(parent_cpu_data);
   242		}
   243	
   244		return 0;
   245	}
   246	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER
  2022-06-13 13:42 ` [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER Christian 'Ansuel' Marangi
  2022-06-14  6:59   ` kernel test robot
@ 2022-06-14  9:01   ` kernel test robot
  1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-06-14  9:01 UTC (permalink / raw)
  To: Christian 'Ansuel' Marangi, MyungJoo Ham, Kyungmin Park,
	Chanwoo Choi, Sibi Sankar, Saravana Kannan, linux-pm,
	linux-kernel
  Cc: kbuild-all, Christian 'Ansuel' Marangi

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on chanwoo/devfreq-testing]
[also build test ERROR on linus/master v5.19-rc2 next-20220614]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Ansuel-Marangi/PM-devfreq-Various-Fixes-to-cpufreq-based-passive-governor/20220614-020616
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git devfreq-testing
config: arc-randconfig-r043-20220613 (https://download.01.org/0day-ci/archive/20220614/202206141647.zyZy1pSw-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/378807dd7da24162524c0ba84e996157b3e289c6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Ansuel-Marangi/PM-devfreq-Various-Fixes-to-cpufreq-based-passive-governor/20220614-020616
        git checkout 378807dd7da24162524c0ba84e996157b3e289c6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/devfreq/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/devfreq/governor_passive.c: In function 'cpufreq_passive_unregister_notifier':
>> drivers/devfreq/governor_passive.c:235:79: error: macro "list_for_each_entry_safe" requires 4 arguments, but only 3 given
     235 |         list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
         |                                                                               ^
   In file included from include/linux/module.h:12,
                    from drivers/devfreq/governor_passive.c:10:
   include/linux/list.h:760: note: macro "list_for_each_entry_safe" defined here
     760 | #define list_for_each_entry_safe(pos, n, head, member)                  \
         | 
>> drivers/devfreq/governor_passive.c:235:9: error: 'list_for_each_entry_safe' undeclared (first use in this function)
     235 |         list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/devfreq/governor_passive.c:235:9: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/devfreq/governor_passive.c:235:33: error: expected ';' before '{' token
     235 |         list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
         |                                 ^                                               ~
         |                                 ;
   drivers/devfreq/governor_passive.c:225:34: warning: unused variable 'parent_cpu_data' [-Wunused-variable]
     225 |         struct devfreq_cpu_data *parent_cpu_data;
         |                                  ^~~~~~~~~~~~~~~
   drivers/devfreq/governor_passive.c:245:1: error: control reaches end of non-void function [-Werror=return-type]
     245 | }
         | ^
   cc1: some warnings being treated as errors


vim +/list_for_each_entry_safe +235 drivers/devfreq/governor_passive.c

   220	
   221	static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq)
   222	{
   223		struct devfreq_passive_data *p_data
   224				= (struct devfreq_passive_data *)devfreq->data;
   225		struct devfreq_cpu_data *parent_cpu_data;
   226		int ret;
   227	
   228		if (p_data->nb.notifier_call) {
   229			ret = cpufreq_unregister_notifier(&p_data->nb,
   230						CPUFREQ_TRANSITION_NOTIFIER);
   231			if (ret < 0)
   232				return ret;
   233		}
   234	
 > 235		list_for_each_entry_safe(parent_cpu_data, &p_data->cpu_data_list, node) {
   236			list_del(&parent_cpu_data->node);
   237	
   238			if (parent_cpu_data->opp_table)
   239				dev_pm_opp_put_opp_table(parent_cpu_data->opp_table);
   240	
   241			kfree(parent_cpu_data);
   242		}
   243	
   244		return 0;
   245	}
   246	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-06-14  9:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 13:42 [PATCH v2 0/5] PM / devfreq: Various Fixes to cpufreq based passive governor Christian 'Ansuel' Marangi
2022-06-13 13:42 ` [PATCH v2 1/5] PM / devfreq: Fix cpufreq passive unregister erroring on PROBE_DEFER Christian 'Ansuel' Marangi
2022-06-14  6:59   ` kernel test robot
2022-06-14  9:01   ` kernel test robot
2022-06-13 13:42 ` [PATCH v2 2/5] PM / devfreq: Fix kernel warning with cpufreq passive register fail Christian 'Ansuel' Marangi
2022-06-13 13:42 ` [PATCH v2 3/5] PM / devfreq: Fix kernel panic with cpu based scaling to passive gov Christian 'Ansuel' Marangi
2022-06-13 13:42 ` [PATCH v2 4/5] PM / devfreq: Rework freq_table to be local to devfreq struct Christian 'Ansuel' Marangi
2022-06-13 13:42 ` [PATCH v2 5/5] PM / devfreq: Mute warning on governor PROBE_DEFER Christian 'Ansuel' Marangi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).