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

From: Rafael J. Wysocki
Date: Thu Jul 21 2016 - 19:53:31 EST


On Fri, Jul 22, 2016 at 1:45 AM, Steve Muckle <steve.muckle@xxxxxxxxxx> 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@xxxxxxxxxx> 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.