Re: [PATCH v1 1/1] IB/hfi1: Don't cast parameter in bit operations

From: Dennis Dalessandro
Date: Thu Feb 24 2022 - 08:44:41 EST


On 2/23/22 5:30 PM, 'Andy Shevchenko' wrote:
> On Wed, Feb 23, 2022 at 09:44:32PM +0000, David Laight wrote:
>> From: Andy Shevchenko
>>> Sent: 23 February 2022 18:54
>>>
>>> While in this particular case it would not be a (critical) issue,
>>> the pattern itself is bad and error prone in case somebody blindly
>>> copies to their code.
>>
>> It is horribly wrong on BE systems.
>
> You mean the pattern? Yes, it has three issues regarding to endianess and
> potential out of boundary access.
>
> ...
>
>>> - return handled;
>>> + return IRQ_RETVAL(!bitmap_empty(pending, CCE_NUM_INT_CSRS * 64));
>
>> You really don't want to scan the bitmap again.
>
> Either way it wastes cycles, the outcome depends on the actual distribution of
> the interrupts across the bitmap. If it gathered closer to the beginning of the
> bitmap, my code wins, otherwise the original ones.
>
>> Actually, of the face of it, you could merge the two loops.
>> Provided you clear the status bit before calling the relevant
>> handler I expect it will all work.
>
> True. I will consider that for v2.

Will wait for a v2 patch and I'll test it. We are very sensitive to performance
changes.

-Denny