Re: [RFC PATCH 11/19] cpufreq: assert policy->rwsem is held in __cpufreq_governor

From: Saravana Kannan
Date: Tue Feb 02 2016 - 16:37:29 EST


On 02/01/2016 10:34 PM, Viresh Kumar wrote:
On 01-02-16, 12:24, Saravana Kannan wrote:
On 02/01/2016 02:22 AM, Rafael J. Wysocki wrote:
I'm not sure whose idea you are referring to. Viresh's (I don't think I saw
his proposal) or mine.

http://git.linaro.org/people/viresh.kumar/linux.git/commit/57714d5b1778f2f610bcc5c74d85b29ba1cc1995

Anyway, to explain my suggestion better, I'm proposing to make it so that we
don't have a need for the AB BA locking. The only reason the governor needs
to even grab the sysfs lock is to add/remove the sysfs attribute files.

That can be easily achieved if the policy struct has some "gov_attrs"
field(s) that each governor populates. Then the framework just has to create
them after POLICY_INIT is processed by the governor and remove them before
POILICY_EXIT is sent to the governor.

What will that solve? It will stay exactly same then as well, as we
would be adding/removing these attributes from within the same
policy->rwsem ..

The problem isn't that you are holding the policy rwsem. The problem is that we are trying to grab the same locks in different order. This is trying to fix that.

That way, we also avoid having to worry about the gov attributes accessed by
the show/store disappearing while the files are being accessed.

It can't happen. S_active lock should be taking care of that, isn't
it?

You are right. That can't happen because we have the s_active lock. I meant to say that in general we don't have to worry about the races between a show/store needing some policy specific data within the governor to be valid but racing with governor change where it ends up being invalid. The releasing of the policy rwsem across POLICY_EXIT allows this to happen today.

-Saravana


--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project