Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] sched/fair: Change sched_feat(x) in !CONFIG_SCHED_DEBUG
 case
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     lukas.bulwahn@gmail.com, kernelnewbies@kernelnewbies.org,
        llvmlinux@lists.linuxfoundation.org, sil2review@lists.osadl.org,
        der.herr@hofr.at, Ingo Molnar <mingo@redhat.com>,
        linux-kernel@vger.kernel.org
References: <20180416085426.24157-1-Phil_K97@gmx.de>
 <20180420075717.GB4064@hirez.programming.kicks-ass.net>
From:   Philipp Klocke <Phil_K97@gmx.de>
Openpgp: preference=signencrypt
Autocrypt: addr=Phil_K97@gmx.de; prefer-encrypt=mutual; keydata=
 xsFNBFXDU4UBEADrRr4gdy5zZz8dQ3sbFxIK9a4vH5t7T06SKT4DXi1W4Eo8bUt7+zPV0RKx
 QZ4RzeWsyW/jUHRKZWp2Nk4HJIfFce5CcxeVJCsRDIuGHOFAh2n5UBjGJ0idylPUtMjItyYY
 RAZ1IWBsNk5tLUht0IYkz7ytCDzwAyEZnfyxajjEiaceMDcJyx+oAE9w39SO7cgT5NsOORoT
 11lpobyanRB7UkQFR6yDZhfWlX/U0iEsr4l9hmBBzA0Pz5co9jqW2p61AondeIdkqMbjIakW
 1+7GYS/A+bZkXuOx55Tvxr0y3kUgXzonOP0qHa5Y9S9pqlcJGzrEM2cwYIH7qSmvTGG3+dr4
 M5e6/kafRx62Y98Ny35ojkjF6ckY2vB4LCeMN7EGax9CLUvnEXHqT2sfO8HmzUf7rKpeKlGW
 lzHlLfSA1vjEOeSd0+z6y8ptuWxLiDuhsweh6yIFP9GHhJApiFyHVK6vwoRSmD2oWQnADJ/a
 FeXHApOuf+7wRdsEfKetj4Te41jBJuIsDgcOPnsl6pvEK4jEV1tw1WX971CdsRxq8qix73Bi
 R4LNO7KOkzJ2YJzPAeDuU0Lfh0MQ14Bd/UY+iC6YfeDMdhpKQiLX+AyIREKu/yG1RR3nVMS8
 32sMyghCLcovAGWKV8s72z2Hv+yM9r2uga8x7FA1HiD7DdA8LwARAQABzSFQaGlsaXBwIEts
 b2NrZSA8a2xvY2tlQGluLnR1bS5kZT7CwX8EEwEIACkFAlibVugCGyMFCQlmAYAHCwkIBwMC
 AQYVCAIJCgsEFgIDAQIeAQIXgAAKCRBPmrpXHFpjqy/lEACmnNE5Irt+zC4BJTYTvPjvwQAs
 NNdWBQHOklt9K0I6+5AmGFfMjT0xvIuvN4BNnug/ftQ+RiLepvIX1QSxzllmJgemBLnPZIO4
 BoM2C4nqC4rXQgRiEsbQx9Y5lwUUv7cInlyv1evxmmXIJkrziNi1heZfIz3cWUhVVQ2YS/du
 z9i6IrXoEFbFeuL1nq12qIWdtZp0zvCxBZLSZvSZd9gFqEZ934pSe9kRX3njT6rQP8MRyZn+
 TbkmnoTKslVJhsVt3ASJ40vd7rQI3gp/3ZOxY8LhU2CpA7q/hy+avYZHo87ROhTlXtRe3U5F
 wF8Kt8G5RC13zBW8pUs4SWfikIr6Zaz815hvDc1ZcXOF6r18BWQ0jXhBHic/3ccuNJd8Qg4O
 p7e9NGPURxhyI6ntzHkTEl90eNvP3KujY6x0PTuGJ/0Ejgq1We1Uibz6x/Sv2ipGdkLO1Cy1
 Cnc2CZWxTx/3q2fAXr0mlu/Ogi0HoInRc5lZ9iukSz6/d8aJ3aS5zaJND+VrPdOhIK6sKkT/
 oRosTkwYi9k0LTXcKzLIaN7ryiPLb5I1i/upXIkOyLaIvxOw3rO77V/e3rIbsyoYhlQB1cij
 IW4tgi9UvWqMBmTaB/Fhj94dAgNQ1geZsce0t0kVTSmK6vw4GQxQujbGNtOrW4A/wN7KrA4k
 ek3wMJAR3c7BTQRVw1OFARAAttZQayw50rkOsEd33Cx0f8dgw2bk/T43NfIOUJVpKaK/70+z
 nC+pG+wOe0KhzGMcC2tJWt7AE927LCLmmVrW3n82ANOeLYHzH8X4VvI5Iv+EQGyaRwMFSkKn
 jMMJasAqHem9B1Cb7ILSUv+sW7bJiOCAXM1tgGF/FTeaJWasz/xoMIjwfbNORl+aAiz0BCIc
 VE0k5Gbm4KCJN7hylyQ9GOsqj1pVNN794OgxH4Agks1n3mWb+izWWICWHQ5+JQwdsSPXHrim
 E21niARw943McEocUYoQci+NywVpTIEA6sGu53HpSZjknxDhFq8C6B88fySachF/tEdFYD/R
 qc9+5gAs5vAMMM3aPNWnh/T7jaS6L/Juw9bxbfqCM4pk1/eoz2fEh91kPjA99Y+WqzBv3hQz
 fJsMMzFxtIkuDj2Zbps64UKTtE51mQO1fdaKSFRUm71G4eopcwb0sYXD+JB/syW26hcuMJoS
 qlflY8+5gggoTgDVP+Y0YYmyMIUhUyG/MuZ/xvdfqEzFrzW0g1vlO3zUgJEhl61ycxkoKdf/
 6M9e/E+90JLqUWiWZXzLRMLvOW3ZWeyUdGsM30c5WjXMIIwkOZ+8W5kbcLqEnHKT2Kx+KqKf
 yFdQnhDjshgKgu5hd67m6Lv+jD/lXLEkHMwYqBjqhcRinhDtLZOKskK7h0MAEQEAAcLBZQQY
 AQgADwUCVcNThQIbDAUJCWYBgAAKCRBPmrpXHFpjqy0QEADqUcSEJr5tRG0633Abw94K7Vf0
 zj2cM8TIGK099wrFAtqX45Lq0rzES0K8uWTCo46HG4EDIHB9AyAViOlb/Rlp3iq63S+mP+Mo
 9/amfn17fE07wMAm+fUtsfhrZcbBZpEjpCC2WBcqq2kzA5zV1HdcD6MhCV9MANkgBiRCGiLI
 Rc6hd0QuNlf6RdLdSCEibFVuC129860Jm6BUabjKWUoo4JgTadrqcm81e9f4uAxb6H8AdiPy
 RHzHNyNFwv4btWJBoz//tbk3loActraIag14TXWStAqFvnxaoTm+03/qisqyiTupJjjV87IA
 IHeJh7vajm000AjY1O8jdQBhKZ7OMsuBugtsM2B2KURfiDmBj+2/gyJDfJ4wg8GXc26DS8ON
 7XTJPtmpgChrnx6aSPSVH14ic1egjS4mpd5papSMJBM7YPM/W/hKvy4+bahfi7jX+9bTQG7y
 6jyB/OhtJ9R25mgJV9ixuGo+u/hKWTnuw3QkB3LnNcJRagM0JWUL1u+WGF/hdn5bk0lWMCEO
 LzzPe1A1zCtWKkbngoC+/XqkIFoGtftGXtxYLiOMBd87S7zCEniUQyL5dZWZl9xtpkcdbNEv
 uw5tIyT766GkXUFqUhuEzuVcY5Pz0lxOQh0L9WRciLWXcGHvvXhbNGtwsfOJf1UBEx/iVYZG
 12TtBTzNsw==
Message-ID: <34572fee-36d0-36e1-ba6d-f098b145aba4@gmx.de>
Date:   Fri, 20 Apr 2018 18:29:07 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.7.0
MIME-Version: 1.0
In-Reply-To: <20180420075717.GB4064@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 20.04.2018 09:57, Peter Zijlstra wrote:

> On Mon, Apr 16, 2018 at 10:54:26AM +0200, Philipp Klocke wrote:
>
>> This patch is motivated by the clang warning Wconstant-logical-operand,
>> issued when logically comparing a variable to a constant integer that is
>> neither 1 nor 0.  It happens for sched_feat(x) when sysctl_sched_features
>> is constant, i.e., CONFIG_SCHED_DEBUG is not set.
>>
>> kernel/sched/fair.c:3927:14: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
>>         if (initial && sched_feat(START_DEBIT))
>>                     ^  ~~~~~~~~~~~~~~~~~~~~~~~
>> kernel/sched/fair.c:3927:14: note: use '&' for a bitwise operation
>>         if (initial && sched_feat(START_DEBIT))
>>                     ^~
>>                     &
>> kernel/sched/fair.c:3927:14: note: remove constant to silence this warning
>>         if (initial && sched_feat(START_DEBIT))
>>                    ~^~~~~~~~~~~~~~~~~~~~~~~~~~
>> @@ -1305,7 +1305,11 @@ static const_debug __maybe_unused unsigned int sysctl_sched_features =
>>  	0;
>>  #undef SCHED_FEAT
>>  
>> +#ifdef CONFIG_SCHED_DEBUG
>>  #define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
>> +#else
>> +#define sched_feat(x) ((sysctl_sched_features >> __SCHED_FEAT_##x) & 1UL)
>> +#endif
> So this is extra ugly, for no gain?
The gain is stopping a warning that clutters the output log of clang.
To improve readability, one can drop the ifdef-structure and just keep
the right shift version, like Nicholas suggested. This will have a (very
small)
impact on performance in CONFIG_SCHED_DEBUG case, but when
debugging, performance is no problem anyways.
> WTH does clang complain about a constant? Can't you just disable that
> stupid warning?
There are 2 ways to disable the warning. Either disable it for this
particular
occurrence, which clutters the code with #pragma's. THIS is really ugly.
Or disable it globally and maybe miss some important/helpful warnings.
> Also, if sysctl_sched_features is a constant, the both expressions
> _should_ really result in a constant and clang should still warn about
> it.
No, because clang only warns if the constant is neither 1 nor 0.
(These being the 'best' integer representations of booleans)
> I'm really not seeing why we'd want to do this. Just fix clang to not be
> stupid.
>