Re: [PATCH net v2] ice: use proper macro for testing bit

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Simon Horman <horms@kernel.org>
Date: Sat, 15 Jun 2024 16:16:41 +0100

> On Fri, Jun 14, 2024 at 11:43:38AM +0200, Petr Oros wrote:
>> Do not use _test_bit() macro for testing bit. The proper macro for this
>> is one without underline.
> 
> Hi Petr,
> 
> it might be nice to include a brief explanation as to
> why test_bit() is correct.

Let me explain this as the author of all those bitops wrappers :D
Petr is free to include either this or his own brief into v2.

_test_bit() is what test_bit() was prior to my const-optimization. It
directly calls arch_test_bit(), i.e. the arch-specific implementation
(or the generic one). It's strictly _internal_ and shouldn't be used
anywhere outside the actual test_bit() macro.

test_bit() is a wrapper which checks whether the bitmap and the bit
number are compile-time constants and if so, it calls the optimized
function which evaluates this call to a compile-time constant as well.
If either of them is not a compile-time constant, it just calls _test_bit().
test_bit() is the actual function to use anywhere in the kernel.

IOW, calling _test_bit() avoids potential compile-time optimizations.

From what I see in the code, &sensors is not a compile-time constant,
thus most probably there are no object code changes before and after
the patch. But anyway, we shouldn't call internal wrappers instead of
the actual API, so this fix is correct.

> 
>>
>> Fixes: 4da71a77fc3b ("ice: read internal temperature sensor")
>> Signed-off-by: Petr Oros <poros@redhat.com>
>> Acked-by: Ivan Vecera <ivecera@redhat.com>

To be added to v2:

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>

Thanks,
Olek