> On 16 Jun 2021, at 17:02, Stefan Metzmacher <metze@xxxxxxxxx> wrote:
>
> Hi Håkon,
>
>> +#define BIT_ACCESS_FUNCTIONS(b) \
>> + static inline void set_##b(unsigned long flags) \
>> + { \
>> + /* set_bit() does not imply a memory barrier */ \
>> + smp_mb__before_atomic(); \
>> + set_bit(b, &flags); \
>> + /* set_bit() does not imply a memory barrier */ \
>> + smp_mb__after_atomic(); \
>> + } \
>
> Don't you need to pass flags by reference to the inline function?
> As far as I can see set_bit() operates on a temporary variable.
Good catch Stefan! It started off as a define, then it worked, but as you say, this doesn't work when we pass flags by value to the inline function. Bummer!
I'll send a v2 tomorrow and see if I can include other review comments as well.
Again, thanks for the review !
Håkon
>
> In order to check if inline functions are special I wrote this little check:
>
> $ cat inline_arg.c
> #include <stdio.h>
>
> static inline void func(unsigned s, unsigned *p)
> {
> printf("&s=%p p=%p\n", &s, p);
> }
>
> int main(void)
> {
> unsigned flags = 0;
>
> func(flags, &flags);
> return 0;
> }
>
> $ gcc -O0 -o inline_arg inline_arg.c
> $ ./inline_arg
> &s=0x7ffd3a71616c p=0x7ffd3a716184
>
> $ gcc -O6 -o inline_arg inline_arg.c
> $ ./inline_arg
> &s=0x7ffd87781064 p=0x7ffd87781060
>
> It means s doesn't magically become 'flags' of the caller...
>
> I'm I missing something?
>
> metze
