* [PATCH] x86: simplify sync_test_bit()
@ 2008-03-14 7:56 Jan Beulich
2008-03-14 15:03 ` Jeremy Fitzhardinge
2008-03-21 11:18 ` Ingo Molnar
0 siblings, 2 replies; 7+ messages in thread
From: Jan Beulich @ 2008-03-14 7:56 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: linux-kernel
There really is no need for a redundant implementation here, just keep
the alternative name for allowing consumers to use consistent naming.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/include/asm-x86/sync_bitops.h
+++ b/include/asm-x86/sync_bitops.h
@@ -130,26 +130,7 @@ static inline int sync_test_and_change_b
return oldbit;
}
-static __always_inline int sync_constant_test_bit(int nr, const volatile unsigned long *addr)
-{
- return ((1UL << (nr & 31)) &
- (((const volatile unsigned int *)addr)[nr >> 5])) != 0;
-}
-
-static inline int sync_var_test_bit(int nr, const volatile unsigned long * addr)
-{
- int oldbit;
-
- __asm__ __volatile__("btl %2,%1\n\tsbbl %0,%0"
- :"=r" (oldbit)
- :"m" (ADDR),"Ir" (nr));
- return oldbit;
-}
-
-#define sync_test_bit(nr,addr) \
- (__builtin_constant_p(nr) ? \
- sync_constant_test_bit((nr),(addr)) : \
- sync_var_test_bit((nr),(addr)))
+#define sync_test_bit test_bit
#undef ADDR
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: simplify sync_test_bit()
2008-03-14 7:56 [PATCH] x86: simplify sync_test_bit() Jan Beulich
@ 2008-03-14 15:03 ` Jeremy Fitzhardinge
2008-03-14 15:20 ` Jan Beulich
2008-03-21 11:18 ` Ingo Molnar
1 sibling, 1 reply; 7+ messages in thread
From: Jeremy Fitzhardinge @ 2008-03-14 15:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: linux-kernel
Jan Beulich wrote:
> There really is no need for a redundant implementation here, just keep
> the alternative name for allowing consumers to use consistent naming.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> --- a/include/asm-x86/sync_bitops.h
> +++ b/include/asm-x86/sync_bitops.h
> @@ -130,26 +130,7 @@ static inline int sync_test_and_change_b
> return oldbit;
> }
>
> -static __always_inline int sync_constant_test_bit(int nr, const volatile unsigned long *addr)
> -{
> - return ((1UL << (nr & 31)) &
> - (((const volatile unsigned int *)addr)[nr >> 5])) != 0;
> -}
> -
> -static inline int sync_var_test_bit(int nr, const volatile unsigned long * addr)
> -{
> - int oldbit;
> -
> - __asm__ __volatile__("btl %2,%1\n\tsbbl %0,%0"
> - :"=r" (oldbit)
> - :"m" (ADDR),"Ir" (nr));
> - return oldbit;
> -}
> -
> -#define sync_test_bit(nr,addr) \
> - (__builtin_constant_p(nr) ? \
> - sync_constant_test_bit((nr),(addr)) : \
> - sync_var_test_bit((nr),(addr)))
> +#define sync_test_bit test_bit
Hm,
#define sync_test_bit(nr, addr) test_bit(nr, addr)
would be better, but seems reasonable to me. Or even an inline for
consistency.
J
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: simplify sync_test_bit()
2008-03-14 15:03 ` Jeremy Fitzhardinge
@ 2008-03-14 15:20 ` Jan Beulich
2008-03-14 15:43 ` Jeremy Fitzhardinge
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2008-03-14 15:20 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: linux-kernel
>#define sync_test_bit(nr, addr) test_bit(nr, addr)
>
>would be better, but seems reasonable to me. Or even an inline for
>consistency.
I'm usually intentionally using just the names, without parameters and
not as inline, in such alias definitions so that in case the name gets used
as a function pointer (arguably unlikely here) there's not going to be
any missing definition or duplicate function instantiation. But from a
functionality point of view, either of the alternatives you suggest is
of course as good.
Jan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: simplify sync_test_bit()
2008-03-14 15:20 ` Jan Beulich
@ 2008-03-14 15:43 ` Jeremy Fitzhardinge
0 siblings, 0 replies; 7+ messages in thread
From: Jeremy Fitzhardinge @ 2008-03-14 15:43 UTC (permalink / raw)
To: Jan Beulich; +Cc: linux-kernel
Jan Beulich wrote:
> I'm usually intentionally using just the names, without parameters and
> not as inline, in such alias definitions so that in case the name gets used
> as a function pointer (arguably unlikely here) there's not going to be
> any missing definition or duplicate function instantiation. But from a
> functionality point of view, either of the alternatives you suggest is
> of course as good.
I'm especially wary of the naked #define, since it will affect any
instance of sync_test_bit:
struct bit_thingy {
unsigned sync_test_bit:1;
unsigned other_test_bit:1;
...
};
J
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: simplify sync_test_bit()
2008-03-14 7:56 [PATCH] x86: simplify sync_test_bit() Jan Beulich
2008-03-14 15:03 ` Jeremy Fitzhardinge
@ 2008-03-21 11:18 ` Ingo Molnar
2008-03-22 20:27 ` Jeremy Fitzhardinge
1 sibling, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2008-03-21 11:18 UTC (permalink / raw)
To: Jan Beulich; +Cc: Jeremy Fitzhardinge, linux-kernel
* Jan Beulich <jbeulich@novell.com> wrote:
> There really is no need for a redundant implementation here, just keep
> the alternative name for allowing consumers to use consistent naming.
> - __asm__ __volatile__("btl %2,%1\n\tsbbl %0,%0"
> - :"=r" (oldbit)
> - :"m" (ADDR),"Ir" (nr));
> +#define sync_test_bit test_bit
thanks, applied.
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: simplify sync_test_bit()
2008-03-21 11:18 ` Ingo Molnar
@ 2008-03-22 20:27 ` Jeremy Fitzhardinge
2008-03-26 7:27 ` Ingo Molnar
0 siblings, 1 reply; 7+ messages in thread
From: Jeremy Fitzhardinge @ 2008-03-22 20:27 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Jan Beulich, linux-kernel
Ingo Molnar wrote:
> * Jan Beulich <jbeulich@novell.com> wrote:
>
>
>> There really is no need for a redundant implementation here, just keep
>> the alternative name for allowing consumers to use consistent naming.
>>
>
>
>> - __asm__ __volatile__("btl %2,%1\n\tsbbl %0,%0"
>> - :"=r" (oldbit)
>> - :"m" (ADDR),"Ir" (nr));
>>
>
>
>> +#define sync_test_bit test_bit
>>
>
> thanks, applied.
Please use this instead.
J
Subject: x86: use macro parameters on sync_test_bit
Using a naked parameterless macro could lead to other tokens being
unexpectedly replaced.
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
include/asm-x86/sync_bitops.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
===================================================================
--- a/include/asm-x86/sync_bitops.h
+++ b/include/asm-x86/sync_bitops.h
@@ -123,7 +123,7 @@
return oldbit;
}
-#define sync_test_bit test_bit
+#define sync_test_bit(nr, addr) test_bit(nr, addr)
#undef ADDR
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: simplify sync_test_bit()
2008-03-22 20:27 ` Jeremy Fitzhardinge
@ 2008-03-26 7:27 ` Ingo Molnar
0 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2008-03-26 7:27 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: Jan Beulich, linux-kernel
* Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> Subject: x86: use macro parameters on sync_test_bit
>
> Using a naked parameterless macro could lead to other tokens being
> unexpectedly replaced.
> -#define sync_test_bit test_bit
> +#define sync_test_bit(nr, addr) test_bit(nr, addr)
thanks Jeremy, applied.
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-03-26 7:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-14 7:56 [PATCH] x86: simplify sync_test_bit() Jan Beulich
2008-03-14 15:03 ` Jeremy Fitzhardinge
2008-03-14 15:20 ` Jan Beulich
2008-03-14 15:43 ` Jeremy Fitzhardinge
2008-03-21 11:18 ` Ingo Molnar
2008-03-22 20:27 ` Jeremy Fitzhardinge
2008-03-26 7:27 ` Ingo Molnar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.