Messages in this thread | | | Date | Tue, 11 Nov 2014 18:41:21 +0530 | Subject | Re: [PATCH 5/5] cpufreq, add BUG() messages in critical paths to aid debugging failures | From | Viresh Kumar <> |
| |
On 11 November 2014 17:48, Prarit Bhargava <prarit@redhat.com> wrote: > the problem is tht the userful information is the values of initialized, > enabled, and what the event was :( > > in every case i ended up needing the values.
So, just add a pr_debug instead and let every thing crash as it does today.
>>> @@ -258,7 +268,13 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy, >>> - WARN_ON(!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT)); >>> + if (!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT)) { >>> + pr_emerg("governor_data is NULL but governor %s is initialized = %d [governor_enabled = %d event = %u]\n", >>> + policy->governor->name, >>> + atomic_read(&policy->governor->initialized), >>> + policy->governor_enabled, event); >>> + BUG(); >> >> How is the BUG better than the WARN here ? >> > > we null pointer panic later on, and again the useful values are the ones displayed.
For the values, I would add a pr_debug() for all cases. And maybe just do s/WARN_ON/BUG_ON
>>> switch (event) { >>> case CPUFREQ_GOV_POLICY_INIT: >>> @@ -329,6 +345,12 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy, >>> case CPUFREQ_GOV_POLICY_EXIT: >>> mutex_lock(&dbs_data->usage_count_mutex); >>> if (atomic_dec_and_test(&dbs_data->usage_count)) { >>> + if (atomic_read(&policy->governor->initialized) > 1) { >> >> Isn't this wrong? Consider 4 CPUs with separate clock line and have set >> governor-per-policy to true. EXIT will be called for every CPU hotplug and >> initialized will be 4 initially.. >> >> Or I am still vacation lag'd ? :) > > oh, is that right? i'll look into that.
What is right? sorry couldn't understand :(
| |