On 14.02.2024 12:06, Oleksii wrote:
> On Mon, 2024-02-12 at 16:58 +0100, Jan Beulich wrote:
>> On 05.02.2024 16:32, Oleksii Kurochko wrote:
>>> +({ \
>>> + unsigned long __res, __mask; \
>>
>> Leftover leading underscores?
> It is how it was defined in Linux, so I thought that I've to leave it
> as it, but I am OK to rename this variables in next patch version.
My view: If you retain Linux style, retaining such names is also (kind
of) okay. If you convert to Xen style, then name changes are to occur
as part of that conversion.
>>> + __mask = BIT_MASK(nr); \
>>> + __asm__ __volatile__ ( \
>>> + __AMO(op) #ord " %0, %2, %1" \
>>> + : "=r" (__res), "+A" (addr[BIT_WORD(nr)]) \
>>> + : "r" (mod(__mask)) \
>>> + : "memory"); \
>>> + ((__res & __mask) != 0); \
>>> +})
>>> +
>>> +#define __op_bit_ord(op, mod, nr, addr, ord) \
>>> + __asm__ __volatile__ ( \
>>> + __AMO(op) #ord " zero, %1, %0" \
>>> + : "+A" (addr[BIT_WORD(nr)]) \
>>> + : "r" (mod(BIT_MASK(nr))) \
>>> + : "memory");
>>> +
>>> +#define __test_and_op_bit(op, mod, nr, addr) \
>>> + __test_and_op_bit_ord(op, mod, nr, addr, .aqrl)
>>> +#define __op_bit(op, mod, nr, addr) \
>>> + __op_bit_ord(op, mod, nr, addr, )
>>> +
>>> +/* Bitmask modifiers */
>>> +#define __NOP(x) (x)
>>> +#define __NOT(x) (~(x))
>>
>> Here the (double) leading underscores are truly worrying: Simple
>> names like this aren't impossible to be assigned meaninb by a
>> compiler.
> I am not really understand what is the difference for a compiler
> between NOP and __NOP? Do you mean that the leading double underscores
> (__) are often used to indicate that these macros are implementation-
> specific and might be reserved for the compiler or the standard
> library?
It's not "often used". Identifiers starting with two underscores or an
underscore and a capital letter are reserved for the implementation
(i.e. for the compiler's internal use). When not overly generic we
stand a fair chance of getting away. But NOP and NOT are pretty generic.
>>> +#include <asm-generic/bitops/fls.h>
>>> +#include <asm-generic/bitops/flsl.h>
>>> +#include <asm-generic/bitops/__ffs.h>
>>> +#include <asm-generic/bitops/ffs.h>
>>> +#include <asm-generic/bitops/ffsl.h>
>>> +#include <asm-generic/bitops/ffz.h>
>>> +#include <asm-generic/bitops/find-first-set-bit.h>
>>> +#include <asm-generic/bitops/hweight.h>
>>> +#include <asm-generic/bitops/test-bit.h>
>>
>> To be honest there's too much stuff being added here to asm-generic/,
>> all in one go. I'll see about commenting on the remaining parts here,
>> but I'd like to ask that you seriously consider splitting.
> Would it be better to send it outside of this patch series? I can
> create a separate patch series with a separate patch for each asm-
> generic/bitops/*.h
Not sure. Depends in part on whether then you'd effectively introduce
dead code. If the introduction was such that RISC-V used the new ones
right away, then yes, that would quite likely be better.
Jan
