Re: [PATCH] cpufreq: flush any pending policy update work scheduled before freeing

[Sudeep Holla]

On Mon, Oct 21, 2019 at 07:45:51AM +0530, Viresh Kumar wrote:
> On 18-10-19, 12:06, Sudeep Holla wrote:
> > Callstack is:
> >
> > (cpufreq_notifier_max)
> > (notifier_call_chain)
> > (blocking_notifier_call_chain)
> > (pm_qos_update_target)
> > (freq_qos_apply)
> > (freq_qos_remove_request)
> > (cpufreq_policy_free)
> > (subsys_interface_unregister)
> > (cpufreq_unregister_driver)
>
> @sudeep: I see that the patch is merged now, but as I said earlier the
> reasoning isn't clear yet. Please don't stop working on this and lets
> clean this once and for all.
>

Sure.

> What patches were you testing this with? My buggy patches or Rafael's
> patches as well ? At least with my patches, this can happen due to the
> other bug where the notifier doesn't get removed (as I said earlier),
> but once that bug isn't there then this shouldn't happen, else we have
> another bug in pipeline somewhere and should find it.
>

I just tested now with today's linux-pm/bleeding-edge branch.
And even if I move cancel_work_sync just after freq_qos_remove_notifier,
it works fine now. It was not the case on Friday.

Is that what you wanted to check or something else ?

Regards,
Sudeep

-->8

diff --git i/drivers/cpufreq/cpufreq.c w/drivers/cpufreq/cpufreq.c
index 829a3764df1b..48a224a6b178 100644
--- i/drivers/cpufreq/cpufreq.c
+++ w/drivers/cpufreq/cpufreq.c
@@ -1268,6 +1268,9 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
        freq_qos_remove_notifier(&policy->constraints, FREQ_QOS_MIN,
                                 &policy->nb_min);

+       /* Cancel any pending policy->update work before freeing the policy. */
+       cancel_work_sync(&policy->update);
+
        if (policy->max_freq_req) {
                /*
                 * CPUFREQ_CREATE_POLICY notification is sent only after
@@ -1279,8 +1282,6 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
        }

        freq_qos_remove_request(policy->min_freq_req);
-       /* Cancel any pending policy->update work before freeing the policy. */
-       cancel_work_sync(&policy->update);
        kfree(policy->min_freq_req);

        cpufreq_policy_put_kobj(policy);