diff mbox

[V2,02/14] cpufreq: stats: return -EEXIST when stats are already allocated

Message ID 85e5b1e6e871fdd8e7a80bba310a7b2948b52917.1420177186.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar Jan. 2, 2015, 5:46 a.m. UTC
__cpufreq_stats_create_table() is called from:
- policy create notifier: stats will always be NULL here
- cpufreq_stats_init() calls it for all CPUs as cpufreq-stats can be initialized
  after cpufreq driver. Because CPUs share clock lines, 'stats' will be NULL
  here for the first cpu only and will return back for others.

While we return for other CPUs, we don't return the right error value. We must
return -EEXIST, as that is the case here.

Reviewed-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq_stats.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Viresh Kumar Jan. 4, 2015, 3:18 a.m. UTC | #1
On 4 January 2015 at 03:28, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, January 02, 2015 11:16:39 AM Viresh Kumar wrote:
>> __cpufreq_stats_create_table() is called from:
>> - policy create notifier: stats will always be NULL here
>> - cpufreq_stats_init() calls it for all CPUs as cpufreq-stats can be initialized
>>   after cpufreq driver. Because CPUs share clock lines, 'stats' will be NULL
>>   here for the first cpu only and will return back for others.
>>
>> While we return for other CPUs, we don't return the right error value. We must
>> return -EEXIST, as that is the case here.
>
> What exactly is wrong with -EBUSY, then?

Its not that could would fail with EBUSY but this is what I thought.
Generally:
- Returning EBUSY means: we are busy right not try again. And that might be
immediate.
- Returning EEXIST means: We already have what you are trying to create and
there is no need to create it again, and so no more tries are required.

That's all..
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar Jan. 6, 2015, 3:42 a.m. UTC | #2
On 6 January 2015 at 05:22, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> Now note how much useful the above is than the stuff in your original changelog.

Yes, its all in logs now :)

> I'd say all depends on whether or not the callers deal with the error code
> appropriately.  If it is propagated all the way to user space, it really should
> *not* be -EBUSY, but otherwise this is a matter of how the callers handle the
> given error code.

It might not get propagated to user space for now, but not sure if that wouldn't
change in future. Even if it stays only within the kernel, we better return the
error codes that fit the best for a particular situation and so I
would still like
to get this through.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index 81be4d637ab4..403671b1a5ee 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -192,8 +192,10 @@  static int __cpufreq_stats_create_table(struct cpufreq_policy *policy)
 	if (unlikely(!table))
 		return 0;
 
+	/* stats already initialized */
 	if (per_cpu(cpufreq_stats_table, cpu))
-		return -EBUSY;
+		return -EEXIST;
+
 	stat = kzalloc(sizeof(*stat), GFP_KERNEL);
 	if ((stat) == NULL)
 		return -ENOMEM;