Re: [Xen-devel] [PATCH v6 08/14] arm/mem_access: Introduce GENMASK_ULL bit operation

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Jan,


On 07/06/2017 02:18 PM, Jan Beulich wrote:
>>>> On 06.07.17 at 13:50, <proskurin@xxxxxxxxxxxxx> wrote:
>> --- a/xen/include/asm-arm/config.h
>> +++ b/xen/include/asm-arm/config.h
>> @@ -19,6 +19,8 @@
>>  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
>>  #define POINTER_ALIGN BYTES_PER_LONG
>>  
>> +#define BITS_PER_LONG_LONG 64
> If we really want to be prepared for architectures where long long
> is other than 64 bits wide, you'll have to also add this to x86.
> Otherwise it's not needed here either.

Absolutely. I kinda got lost in the ARM world. Of course, I can (will)
do that as part of the next patch series. Thank you.

> Also how about BITS_PER_LLONG?
>

I am also fine with BITS_PER_LLONG. We used BITS_PER_LONG_LONG as it was
lifted from the Linux kernel (and suggested by Julien). It would be
great to know the opinion of the remaining maintainters about that.

>> @@ -128,7 +131,7 @@ static inline int generic_fls64(__u64 x)
>>  static __inline__ int get_bitmask_order(unsigned int count)
>>  {
>>      int order;
>> -    
>> +
>>      order = fls(count);
>>      return order;   /* We could be slightly more clever with -1 here... */
>>  }
> If you really want to do cleanup here, please shrink the function
> body to a single return statement. But then again I'm unconvinced
> the function is actually correct (which could easily be the case for
> an unused one), in particular for power-of-2 counts. Nor can I see
> how this would be useful with anything more narrow than size_t
> or unsigned long as parameter type.
>
> Jan
>

Right. That whitespace elimination was actually unintended. However, if
you wish, I could do the cleanup in a separate patch.

Concerning the correctness of this function: I am not sure whether this
would be the right patch series to address that.

Thank you,
~Sergej


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel