Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index()

From	"Rafael J. Wysocki" <rafael@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index()
Date	2016-07-22 02:00 +0200
Message-ID	<rXw2J-67H-1@gated-at.bofh.it> (permalink)
References	<rXtRf-4HJ-9@gated-at.bofh.it> <rXvq1-5T7-3@gated-at.bofh.it> <rXvzI-5X0-19@gated-at.bofh.it> <rXvJn-60q-17@gated-at.bofh.it> <rXvT3-647-1@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Jul 22, 2016 at 1:45 AM, Steve Muckle <steve.muckle@linaro.org> wrote:
> On Fri, Jul 22, 2016 at 01:32:00AM +0200, Rafael J. Wysocki wrote:
>> On Fri, Jul 22, 2016 at 1:22 AM, Steve Muckle <steve.muckle@linaro.org> wrote:
>> > On Fri, Jul 22, 2016 at 01:22:22AM +0200, Rafael J. Wysocki wrote:
>> >> OK, applied.
>> >
>> > FWIW I do have a concern on this patch, I think it adds unnecessary
>> > overhead.
>>
>> It isn't unnecessary.  It prevents an otherwise possible kernel crash
>> from happening.
>
> The logic may not be unecessary, but the overhead is. The crash could be
> prevented in a way that doesn't require repeatedly checking a pointer
> that doesn't change.

Well, you had the ->resolve_freq check in your patch, didn't you?

Viresh simply added a ->target_index check to it.

Now, you can argue that this is one check too many, but as long as
drivers are allowed to implement ->target without implementing
->resolve_freq, the *number* of checks in this routine cannot be
reduced.

There are three possible cases and two checks are required to
determine which case really takes place.

Thread

[PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index() Viresh Kumar <viresh.kumar@linaro.org> - 2016-07-21 23:40 +0200
  Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-22 01:20 +0200
    Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() Steve Muckle <steve.muckle@linaro.org> - 2016-07-22 01:30 +0200
      Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-22 01:40 +0200
        Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() Steve Muckle <steve.muckle@linaro.org> - 2016-07-22 01:50 +0200
          Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-22 02:00 +0200
            Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() Steve Muckle <steve.muckle@linaro.org> - 2016-07-22 02:10 +0200
              Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-22 02:20 +0200
                Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() Steve Muckle <steve.muckle@linaro.org> - 2016-07-22 02:40 +0200
                Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() Viresh Kumar <viresh.kumar@linaro.org> - 2016-07-22 17:20 +0200
                Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-22 23:10 +0200
                Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers  providing ->target_index() Viresh Kumar <viresh.kumar@linaro.org> - 2016-07-22 23:10 +0200

csiph-web