Re: [PATCH 0/2] cpu-features.h rename

[Florian Fainelli <f.fainelli@gmail.com>]

On 03/14/2017 01:45 AM, Marcin Nowakowski wrote:
> Hi Ralf,
> 
> On 14.03.2017 09:29, Ralf Baechle wrote:
>> On Tue, Mar 14, 2017 at 08:40:21AM +0100, Marcin Nowakowski wrote:
>>
>>> On 13.03.2017 18:08, Florian Fainelli wrote:
>>>> On 03/13/2017 06:33 AM, Marcin Nowakowski wrote:
>>>>> Since the introduction of GENERIC_CPU_AUTOPROBE
>>>>> (https://patchwork.linux-mips.org/patch/15395/) we've got 2 very
>>>>> similarily
>>>>> named headers: cpu-features.h and cpufeature.h.
>>>>> Since the latter is used by all platforms that implement
>>>>> GENERIC_CPU_AUTOPROBE functionality, it's better to rename the
>>>>> MIPS-specific
>>>>> cpu-features.h.
>>>>>
>>>>> Marcin Nowakowski (2):
>>>>>   MIPS: mach-rm: Remove recursive include of cpu-feature-overrides.h
>>>>>   MIPS: rename cpu-features.h -> cpucaps.h
>>>>
>>>> That's a lot of churn that could cause some good headaches in
>>>> backporting stable changes affecting cpu-feature-overrides.h.
>>>>
>>>> Can we just do the cpu-features.h -> cpucaps.h rename and keep
>>>> cpu-feature-overrides.h around?
>>>
>>> That's of course possible, but I think it would make the naming quite
>>> confusing as well, as it would be very unclear for any reader as to
>>> why a
>>> 'cpu-feature-overrides' overrides 'cpucaps'.
>>>
>>> I've looked at the change history of these files and most receive very
>>> little updates (which is hardly surprising given the changes are done
>>> mostly
>>> during initial integration of a new cpu or soon after), and none of the
>>> changes in those files were marked for stable. I think it's safe to
>>> assume
>>> that this pattern is not likely to change, would you agree?
>>
>> I've noticed the same pattern - and it's a little concerning.  Not adding
>> values for later features means the'll probably be runtime detected
>> resulting in a bigger, slower kernel.
> 
> But that is a type of optimisation that may (should?) be done when new
> features are added, which in most cases doesn't make it a candidate for
> backporting to stable.

You may be fixing actual bugs by patching this file, e.g: selecting the
correct value for e.g: cpu_has_dc_aliases, cpu_has_ic_fills_ic,
cpu_dcache_line_size() and so on. Ideally every feature in there has
been properly set/cleared in arch/mips/kernel/cpu-probe.c but there
could be platform relying exclusively on cpu-feature-overrides.h to
provide the correct value.

If not about the backport argument, just changing that many files at
once (have they actually been build tested at least?) just does not seem
practical nor worth it to me. Just put a big fat disclaimer: this is not
to be conflated with cpucaps.h (and vice versa).
-- 
Florian