Re: [PATCH v2 2/2] linux/bits.h: Add compile time sanity check of GENMASK inputs

[Guenter Roeck <linux@roeck-us.net>]

On 8/7/19 6:08 PM, Joe Perches wrote:
> On Wed, 2019-08-07 at 17:58 -0700, Guenter Roeck wrote:
>> On 8/7/19 5:07 PM, Joe Perches wrote:
>>> On Wed, 2019-08-07 at 23:55 +0900, Masahiro Yamada wrote:
>>>> On Wed, Aug 7, 2019 at 11:27 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>> []
>>>>> Who is going to fix the fallout ? For example, arm64:defconfig no longer
>>>>> compiles with this patch applied.
>>>>>
>>>>> It seems to me that the benefit of catching misuses of GENMASK is much
>>>>> less than the fallout from no longer compiling kernels, since those
>>>>> kernels won't get any test coverage at all anymore.
>>>>
>>>> We cannot apply this until we fix all errors.
>>>> I do not understand why Andrew picked up this so soon.
>>>
>>> I think it makes complete sense to break -next (not mainline)
>>> and force people to fix defects.  Especially these types of
>>> defects that are trivial to fix.
>>>
>>
>> I don't think this (from next-20190807):
>>
>> Build results:
>> 	total: 158 pass: 137 fail: 21
>> Qemu test results:
>> 	total: 391 pass: 318 fail: 73
>>
>> is very useful. The situation is bad enough for newly introduced problems.
>> It is all but impossible to get fixes for all problems discovered (or introduced)
>> by adding checks like this one. In some cases, no one will care. In others,
>> no one will pick up patches. Sometimes people won't know or realize that
>> they are expected to fix something. Making the situation worse, the failures
>> introduced by the new checks will hide other accumulating problems.
>>
>> arch/sh has failed to build in mainline since 7/27 and in -next since
>> next-20190711, due to the added "fallthrough" warning. I don't think
>> that is too useful either. Ok, that situation may be a sign that the
>> architecture isn't maintained as well as it should, but I don't think
>> that this warrants breaking it on purpose in the hope to trigger
>> some kind of reaction.
>>
>> I don't mind if new checks are introduced, and I agree that it is useful
>> and makes sense. But the checks should only be introduced after a reasonable
>> attempt was made to fix _all_ associated problems. That doesn't mean that
>> the entire work has to be done by the person introducing the check, but I
>> do see that person responsible for making sure (or a reasonable definition
>> of "make sure") that all problems are fixed before actually introducing
>> the check. Yes, I understand, this is a lot of work, but adding checks
>> and letting all hell break loose can not be the answer.
> 
> No hell is unleashed.
> 
> It's -next, an integration build, not mainline.
> 

... and the breakages introduced in -next are making it into mainline
without being fixed, as I just pointed out above. That by itself is bad.
It is much worse if the breakage is introduced on purpose.

The criteria for -next _used_ to be "ready for mainline". If breaking -next
on purpose is the new normal, no one should be surprised if it will be tested
even less than it is today.

Guenter