Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

From	"Rafael J. Wysocki" <rafael@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops
Date	2016-02-03 03:00 +0100
Message-ID	<qXUTE-3IL-1@gated-at.bofh.it> (permalink)
References	(5 earlier) <qXP7A-7Ux-1@gated-at.bofh.it> <qXRCr-1mL-29@gated-at.bofh.it> <qXSRQ-2ds-9@gated-at.bofh.it> <qXU7g-3mj-5@gated-at.bofh.it> <qXUAi-3B6-9@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Wed, Feb 3, 2016 at 2:32 AM, Saravana Kannan <skannan@codeaurora.org> wrote:
> On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote:
>>
>> On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki <rafael@kernel.org>
>> wrote:
>>>
>>> On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan <skannan@codeaurora.org>
>>> wrote:
>>>>
>>>> On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote:
>>>>>
>>>>>
>>>>> On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli <juri.lelli@arm.com> wrote:
>>>>>>
>>>>>>
>>
>> [cut]
>>
>>>>
>>>> I also don't like this patch because it forces governors to either
>>>> implement
>>>> their own macros and management of their attributes or force them to use
>>>> the
>>>> governor structs that come with cpufreq_governor.h. cpufreq_governor.h
>>>> IMHO
>>>> is very ondemand and conservative governor specific and is very
>>>> irrelevant
>>>> for sched-dvfs or any other governors (hint hint).
>>>>
>>>> The only time this ABBA locking is an issue is when governor are
>>>> changing
>>>> and trying to add/remove attributes. That can easily be checked in
>>>> store_governor and dealt with without holding the policy rwsem if the
>>>> governors can provide their per sys and per policy attribute arrays as
>>>> part
>>>> of registering themselves.
>>>>
>>>> I'm sorry that I just keep talking about the idea and not sending out
>>>> the
>>>> patches.
>>>
>>>
>>> I think you have a point, though.
>>>
>>> The deadlock really is specific to the governors using the code in
>>> cpufreq_governor.c.
>>
>>
>> That said no other governors in the tree use any sysfs attributes for
>> tunables AFAICS and the out-of-the tree ones are out of interest here.
>
>
> But if we are expecting sched dvfs to come in, why make it worse for it. It
> would be completely pointless to try and shoehorn sched dvfs to use
> cpufreq_governor.c

Well, do you honestly think that using the existing stuff in it would
be a good idea?

If not, then why it matters at all?

>> Also the deadlock happens if one of the tunable attributes is accessed
>> while we're trying to remove it which very well may happen on read
>> access too.
>
> Isn't this THE deadlock we are talking about? The removal of the attributes
> only happen when governors are changes and we send a POLICY_EXIT and or all
> the cores are hotplugged out.

It generally happens when the "old" governor is going away, whatever the reason.

> And my suggestion would work just as well there.
>
> Why are you prefixing your sentence with "Also"? Is there some other case
> I'm not considering?

Say someone is reading sampling_rate for a policy with 1 CPU in it and
someone else is taking the CPU offline.  The governor EXIT code path
(that will trigger as a result) will try to remove the sampling_rate
attribute and (if it does that under policy->rwsem) it'll wait for the
read access to finish.  Where exactly would you put the deadlock
prevention in this case?

Thanks,
Rafael

Thread

[PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
  [PATCH 3/5] cpufreq: governor: Remove unused sysfs attribute macros Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
    Re: [PATCH 3/5] cpufreq: governor: Remove unused sysfs attribute macros "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 22:40 +0100
  [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
    Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 23:00 +0100
      Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling  CPUFREQ_GOV_POLICY_EXIT Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 07:00 +0100
        Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 13:30 +0100
          Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling  CPUFREQ_GOV_POLICY_EXIT Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 14:10 +0100
  [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
    Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Juri Lelli <juri.lelli@arm.com> - 2016-02-02 17:50 +0100
      Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 07:10 +0100
        Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Juri Lelli <juri.lelli@arm.com> - 2016-02-03 12:10 +0100
          Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 12:10 +0100
    Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 23:00 +0100
  [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
    Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Juri Lelli <juri.lelli@arm.com> - 2016-02-02 16:50 +0100
      Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 17:40 +0100
        Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Juri Lelli <juri.lelli@arm.com> - 2016-02-02 18:10 +0100
          Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 20:50 +0100
            Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-02 23:30 +0100
              Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 00:50 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 02:10 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 02:40 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 03:00 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 05:10 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 21:10 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Juri Lelli <juri.lelli@arm.com> - 2016-02-03 12:00 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 12:00 +0100
                Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 21:20 +0100
              Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
          Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 07:40 +0100
    Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 22:30 +0100
      Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
        Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 13:50 +0100
          Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 14:30 +0100
            Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 14:40 +0100
  Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Juri Lelli <juri.lelli@arm.com> - 2016-02-02 12:30 +0100
  Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 21:10 +0100
    Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 03:30 +0100
      Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 12:40 +0100

csiph-web