[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 08/14] arm/mem_access: Introduce GENMASK_ULL bit operation
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |