linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cmpxchg: Discard unnecessary cast to volatile
@ 2014-10-01 17:57 Pranith Kumar
  2014-10-20 20:22 ` Pranith Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: Pranith Kumar @ 2014-10-01 17:57 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE...,
	Borislav Petkov, open list:X86 ARCHITECTURE...

Generating a volatile pointer is really not necessary here. This is the only
location where a volatile pointer is being generated for use in asm.

This commit removes the unnecessary volatile pointer being created.

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 arch/x86/include/asm/cmpxchg.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
index 99c105d7..f0baea8 100644
--- a/arch/x86/include/asm/cmpxchg.h
+++ b/arch/x86/include/asm/cmpxchg.h
@@ -90,36 +90,32 @@ extern void __add_wrong_size(void)
 	switch (size) {							\
 	case __X86_CASE_B:						\
 	{								\
-		volatile u8 *__ptr = (volatile u8 *)(ptr);		\
 		asm volatile(lock "cmpxchgb %2,%1"			\
-			     : "=a" (__ret), "+m" (*__ptr)		\
+			     : "=a" (__ret), "+m" (*ptr)		\
 			     : "q" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	}								\
 	case __X86_CASE_W:						\
 	{								\
-		volatile u16 *__ptr = (volatile u16 *)(ptr);		\
 		asm volatile(lock "cmpxchgw %2,%1"			\
-			     : "=a" (__ret), "+m" (*__ptr)		\
+			     : "=a" (__ret), "+m" (*ptr)		\
 			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	}								\
 	case __X86_CASE_L:						\
 	{								\
-		volatile u32 *__ptr = (volatile u32 *)(ptr);		\
 		asm volatile(lock "cmpxchgl %2,%1"			\
-			     : "=a" (__ret), "+m" (*__ptr)		\
+			     : "=a" (__ret), "+m" (*ptr)		\
 			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
 	}								\
 	case __X86_CASE_Q:						\
 	{								\
-		volatile u64 *__ptr = (volatile u64 *)(ptr);		\
 		asm volatile(lock "cmpxchgq %2,%1"			\
-			     : "=a" (__ret), "+m" (*__ptr)		\
+			     : "=a" (__ret), "+m" (*ptr)		\
 			     : "r" (__new), "0" (__old)			\
 			     : "memory");				\
 		break;							\
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] cmpxchg: Discard unnecessary cast to volatile
  2014-10-01 17:57 [PATCH] cmpxchg: Discard unnecessary cast to volatile Pranith Kumar
@ 2014-10-20 20:22 ` Pranith Kumar
  2014-10-21 10:14   ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Pranith Kumar @ 2014-10-20 20:22 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE...,
	Borislav Petkov, open list:X86 ARCHITECTURE...,
	Peter Zijlstra

ping.

On Wed, Oct 1, 2014 at 1:57 PM, Pranith Kumar <bobby.prani@gmail.com> wrote:
> Generating a volatile pointer is really not necessary here. This is the only
> location where a volatile pointer is being generated for use in asm.
>
> This commit removes the unnecessary volatile pointer being created.
>
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> ---
>  arch/x86/include/asm/cmpxchg.h | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
> index 99c105d7..f0baea8 100644
> --- a/arch/x86/include/asm/cmpxchg.h
> +++ b/arch/x86/include/asm/cmpxchg.h
> @@ -90,36 +90,32 @@ extern void __add_wrong_size(void)
>         switch (size) {                                                 \
>         case __X86_CASE_B:                                              \
>         {                                                               \
> -               volatile u8 *__ptr = (volatile u8 *)(ptr);              \
>                 asm volatile(lock "cmpxchgb %2,%1"                      \
> -                            : "=a" (__ret), "+m" (*__ptr)              \
> +                            : "=a" (__ret), "+m" (*ptr)                \
>                              : "q" (__new), "0" (__old)                 \
>                              : "memory");                               \
>                 break;                                                  \
>         }                                                               \
>         case __X86_CASE_W:                                              \
>         {                                                               \
> -               volatile u16 *__ptr = (volatile u16 *)(ptr);            \
>                 asm volatile(lock "cmpxchgw %2,%1"                      \
> -                            : "=a" (__ret), "+m" (*__ptr)              \
> +                            : "=a" (__ret), "+m" (*ptr)                \
>                              : "r" (__new), "0" (__old)                 \
>                              : "memory");                               \
>                 break;                                                  \
>         }                                                               \
>         case __X86_CASE_L:                                              \
>         {                                                               \
> -               volatile u32 *__ptr = (volatile u32 *)(ptr);            \
>                 asm volatile(lock "cmpxchgl %2,%1"                      \
> -                            : "=a" (__ret), "+m" (*__ptr)              \
> +                            : "=a" (__ret), "+m" (*ptr)                \
>                              : "r" (__new), "0" (__old)                 \
>                              : "memory");                               \
>                 break;                                                  \
>         }                                                               \
>         case __X86_CASE_Q:                                              \
>         {                                                               \
> -               volatile u64 *__ptr = (volatile u64 *)(ptr);            \
>                 asm volatile(lock "cmpxchgq %2,%1"                      \
> -                            : "=a" (__ret), "+m" (*__ptr)              \
> +                            : "=a" (__ret), "+m" (*ptr)                \
>                              : "r" (__new), "0" (__old)                 \
>                              : "memory");                               \
>                 break;                                                  \
> --
> 1.9.1
>



-- 
Pranith

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cmpxchg: Discard unnecessary cast to volatile
  2014-10-20 20:22 ` Pranith Kumar
@ 2014-10-21 10:14   ` Peter Zijlstra
  2014-10-21 15:48     ` H. Peter Anvin
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2014-10-21 10:14 UTC (permalink / raw)
  To: Pranith Kumar
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE...,
	Borislav Petkov, open list:X86 ARCHITECTURE...

On Mon, Oct 20, 2014 at 04:22:27PM -0400, Pranith Kumar wrote:
> > Generating a volatile pointer is really not necessary here. This is the only
> > location where a volatile pointer is being generated for use in asm.
> >
> > This commit removes the unnecessary volatile pointer being created.
> >
> > Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>

Seems sane enough to me.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cmpxchg: Discard unnecessary cast to volatile
  2014-10-21 10:14   ` Peter Zijlstra
@ 2014-10-21 15:48     ` H. Peter Anvin
  2014-10-21 22:15       ` Pranith Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: H. Peter Anvin @ 2014-10-21 15:48 UTC (permalink / raw)
  To: Peter Zijlstra, Pranith Kumar
  Cc: Thomas Gleixner, Ingo Molnar, maintainer:X86 ARCHITECTURE...,
	Borislav Petkov, open list:X86 ARCHITECTURE...

On 10/21/2014 03:14 AM, Peter Zijlstra wrote:
> On Mon, Oct 20, 2014 at 04:22:27PM -0400, Pranith Kumar wrote:
>>> Generating a volatile pointer is really not necessary here. This is the only
>>> location where a volatile pointer is being generated for use in asm.
>>>
>>> This commit removes the unnecessary volatile pointer being created.
>>>
>>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> 
> Seems sane enough to me.
> 

However, it seems like unnecessary churn.  Does the volatile hurt in any
way?

	-hpa


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] cmpxchg: Discard unnecessary cast to volatile
  2014-10-21 15:48     ` H. Peter Anvin
@ 2014-10-21 22:15       ` Pranith Kumar
  0 siblings, 0 replies; 5+ messages in thread
From: Pranith Kumar @ 2014-10-21 22:15 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Peter Zijlstra, Thomas Gleixner, Ingo Molnar,
	maintainer:X86 ARCHITECTURE...,
	Borislav Petkov, open list:X86 ARCHITECTURE...

On Tue, Oct 21, 2014 at 11:48 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 10/21/2014 03:14 AM, Peter Zijlstra wrote:
>> On Mon, Oct 20, 2014 at 04:22:27PM -0400, Pranith Kumar wrote:
>>>> Generating a volatile pointer is really not necessary here. This is the only
>>>> location where a volatile pointer is being generated for use in asm.
>>>>
>>>> This commit removes the unnecessary volatile pointer being created.
>>>>
>>>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>>
>> Seems sane enough to me.
>>
>
> However, it seems like unnecessary churn.  Does the volatile hurt in any
> way?

Removing 4 lines of unnecessary code seems to be worthwhile to me.
Also, we reduce the unnecessary use of 'volatile' data types which
IMHO makes this a good little clean up.

I am not sure if the use of volatile actually causes any loss of
chances of optimization. I've tried to come up with cases where this
happens and was unsuccessful.

-- 
Pranith

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-10-21 22:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-01 17:57 [PATCH] cmpxchg: Discard unnecessary cast to volatile Pranith Kumar
2014-10-20 20:22 ` Pranith Kumar
2014-10-21 10:14   ` Peter Zijlstra
2014-10-21 15:48     ` H. Peter Anvin
2014-10-21 22:15       ` Pranith Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).