[PATCH] x86: modify inline asm constraints in __cmpxchg_double()

[PATCH] x86: modify inline asm constraints in __cmpxchg_double()

[Juergen Gross]

Some gcc versions seem to have problems with the constraints in
__cmpxchg_double() as they suddenly issue a build error when random
parts of sources calling __cmpxchg_double() are modified, like e.g.
slub.c. This has been observed on Debian systems only so far.

Using "0" instead of "a" in the input constraints has the same
semantics while avoiding that build error.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
I should note that I have observed gcc hangs instead sometimes. Not
taking any patches modifying users of __cmpxchg_double() due to a gcc
bug which seems to be distro-specific is a bad move IMO.

I'd rather make it clear from build behavior that this is a bug in gcc
by letting the build hang instead of throwing error warnings not in any
way related to changes in the code.

The gcc bug is filed under:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154
---
 arch/x86/include/asm/cmpxchg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
index a55d79b233d3..b3b4d61a8969 100644
--- a/arch/x86/include/asm/cmpxchg.h
+++ b/arch/x86/include/asm/cmpxchg.h
@@ -245,7 +245,7 @@ extern void __add_wrong_size(void)
 	asm volatile(pfx "cmpxchg%c4b %2; sete %0"			\
 		     : "=a" (__ret), "+d" (__old2),			\
 		       "+m" (*(p1)), "+m" (*(p2))			\
-		     : "i" (2 * sizeof(long)), "a" (__old1),		\
+		     : "i" (2 * sizeof(long)), "0" (__old1),		\
 		       "b" (__new1), "c" (__new2));			\
 	__ret;								\
 })
-- 
2.16.4

Re: [PATCH] x86: modify inline asm constraints in __cmpxchg_double()

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Some gcc versions seem to have problems with the constraints in
> __cmpxchg_double() as they suddenly issue a build error when random
> parts of sources calling __cmpxchg_double() are modified, like e.g.
> slub.c. This has been observed on Debian systems only so far.
> 
> Using "0" instead of "a" in the input constraints has the same
> semantics while avoiding that build error.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> I should note that I have observed gcc hangs instead sometimes. Not
> taking any patches modifying users of __cmpxchg_double() due to a gcc
> bug which seems to be distro-specific is a bad move IMO.
> 
> I'd rather make it clear from build behavior that this is a bug in gcc
> by letting the build hang instead of throwing error warnings not in any
> way related to changes in the code.
> 
> The gcc bug is filed under:
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154
> ---
>  arch/x86/include/asm/cmpxchg.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
> index a55d79b233d3..b3b4d61a8969 100644
> --- a/arch/x86/include/asm/cmpxchg.h
> +++ b/arch/x86/include/asm/cmpxchg.h
> @@ -245,7 +245,7 @@ extern void __add_wrong_size(void)
>  	asm volatile(pfx "cmpxchg%c4b %2; sete %0"			\
>  		     : "=a" (__ret), "+d" (__old2),			\
>  		       "+m" (*(p1)), "+m" (*(p2))			\
> -		     : "i" (2 * sizeof(long)), "a" (__old1),		\
> +		     : "i" (2 * sizeof(long)), "0" (__old1),		\
>  		       "b" (__new1), "c" (__new2));			\

This got changed to +a in -tip:

  c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()

Mind sending a patch on top of -tip?

Thanks,

	Ingo

Re: [PATCH] x86: modify inline asm constraints in __cmpxchg_double()

[Juergen Gross]

On 16/10/2018 08:25, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> Some gcc versions seem to have problems with the constraints in
>> __cmpxchg_double() as they suddenly issue a build error when random
>> parts of sources calling __cmpxchg_double() are modified, like e.g.
>> slub.c. This has been observed on Debian systems only so far.
>>
>> Using "0" instead of "a" in the input constraints has the same
>> semantics while avoiding that build error.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> I should note that I have observed gcc hangs instead sometimes. Not
>> taking any patches modifying users of __cmpxchg_double() due to a gcc
>> bug which seems to be distro-specific is a bad move IMO.
>>
>> I'd rather make it clear from build behavior that this is a bug in gcc
>> by letting the build hang instead of throwing error warnings not in any
>> way related to changes in the code.
>>
>> The gcc bug is filed under:
>>
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154
>> ---
>>  arch/x86/include/asm/cmpxchg.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
>> index a55d79b233d3..b3b4d61a8969 100644
>> --- a/arch/x86/include/asm/cmpxchg.h
>> +++ b/arch/x86/include/asm/cmpxchg.h
>> @@ -245,7 +245,7 @@ extern void __add_wrong_size(void)
>>  	asm volatile(pfx "cmpxchg%c4b %2; sete %0"			\
>>  		     : "=a" (__ret), "+d" (__old2),			\
>>  		       "+m" (*(p1)), "+m" (*(p2))			\
>> -		     : "i" (2 * sizeof(long)), "a" (__old1),		\
>> +		     : "i" (2 * sizeof(long)), "0" (__old1),		\
>>  		       "b" (__new1), "c" (__new2));			\
> 
> This got changed to +a in -tip:
> 
>   c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()
> 
> Mind sending a patch on top of -tip?

No, I think you can drop my patch.

c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()
has the same effect as my patch: the build failure is replaced by
gcc hanging instead in the critical configuration.


Juergen

Re: [PATCH] x86: modify inline asm constraints in __cmpxchg_double()

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> On 16/10/2018 08:25, Ingo Molnar wrote:
> > 
> > * Juergen Gross <jgross@suse.com> wrote:
> > 
> >> Some gcc versions seem to have problems with the constraints in
> >> __cmpxchg_double() as they suddenly issue a build error when random
> >> parts of sources calling __cmpxchg_double() are modified, like e.g.
> >> slub.c. This has been observed on Debian systems only so far.
> >>
> >> Using "0" instead of "a" in the input constraints has the same
> >> semantics while avoiding that build error.
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> ---
> >> I should note that I have observed gcc hangs instead sometimes. Not
> >> taking any patches modifying users of __cmpxchg_double() due to a gcc
> >> bug which seems to be distro-specific is a bad move IMO.
> >>
> >> I'd rather make it clear from build behavior that this is a bug in gcc
> >> by letting the build hang instead of throwing error warnings not in any
> >> way related to changes in the code.
> >>
> >> The gcc bug is filed under:
> >>
> >> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154
> >> ---
> >>  arch/x86/include/asm/cmpxchg.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
> >> index a55d79b233d3..b3b4d61a8969 100644
> >> --- a/arch/x86/include/asm/cmpxchg.h
> >> +++ b/arch/x86/include/asm/cmpxchg.h
> >> @@ -245,7 +245,7 @@ extern void __add_wrong_size(void)
> >>  	asm volatile(pfx "cmpxchg%c4b %2; sete %0"			\
> >>  		     : "=a" (__ret), "+d" (__old2),			\
> >>  		       "+m" (*(p1)), "+m" (*(p2))			\
> >> -		     : "i" (2 * sizeof(long)), "a" (__old1),		\
> >> +		     : "i" (2 * sizeof(long)), "0" (__old1),		\
> >>  		       "b" (__new1), "c" (__new2));			\
> > 
> > This got changed to +a in -tip:
> > 
> >   c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()
> > 
> > Mind sending a patch on top of -tip?
> 
> No, I think you can drop my patch.
> 
> c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()
> has the same effect as my patch: the build failure is replaced by
> gcc hanging instead in the critical configuration.

Ok. No sane workaround we can do to avoid the hang I suspect, right?

Should we propagate the fix/enhancement to x86/urgent? It's currently in 
x86/asm for a v4.20 merge.

Thanks,

	Ingo

Re: [PATCH] x86: modify inline asm constraints in __cmpxchg_double()

[Juergen Gross]

On 16/10/2018 10:01, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> On 16/10/2018 08:25, Ingo Molnar wrote:
>>>
>>> * Juergen Gross <jgross@suse.com> wrote:
>>>
>>>> Some gcc versions seem to have problems with the constraints in
>>>> __cmpxchg_double() as they suddenly issue a build error when random
>>>> parts of sources calling __cmpxchg_double() are modified, like e.g.
>>>> slub.c. This has been observed on Debian systems only so far.
>>>>
>>>> Using "0" instead of "a" in the input constraints has the same
>>>> semantics while avoiding that build error.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>> I should note that I have observed gcc hangs instead sometimes. Not
>>>> taking any patches modifying users of __cmpxchg_double() due to a gcc
>>>> bug which seems to be distro-specific is a bad move IMO.
>>>>
>>>> I'd rather make it clear from build behavior that this is a bug in gcc
>>>> by letting the build hang instead of throwing error warnings not in any
>>>> way related to changes in the code.
>>>>
>>>> The gcc bug is filed under:
>>>>
>>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908154
>>>> ---
>>>>  arch/x86/include/asm/cmpxchg.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/include/asm/cmpxchg.h b/arch/x86/include/asm/cmpxchg.h
>>>> index a55d79b233d3..b3b4d61a8969 100644
>>>> --- a/arch/x86/include/asm/cmpxchg.h
>>>> +++ b/arch/x86/include/asm/cmpxchg.h
>>>> @@ -245,7 +245,7 @@ extern void __add_wrong_size(void)
>>>>  	asm volatile(pfx "cmpxchg%c4b %2; sete %0"			\
>>>>  		     : "=a" (__ret), "+d" (__old2),			\
>>>>  		       "+m" (*(p1)), "+m" (*(p2))			\
>>>> -		     : "i" (2 * sizeof(long)), "a" (__old1),		\
>>>> +		     : "i" (2 * sizeof(long)), "0" (__old1),		\
>>>>  		       "b" (__new1), "c" (__new2));			\
>>>
>>> This got changed to +a in -tip:
>>>
>>>   c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()
>>>
>>> Mind sending a patch on top of -tip?
>>
>> No, I think you can drop my patch.
>>
>> c808c09b527c: x86/asm: Use CC_SET()/CC_OUT() in __cmpxchg_double()
>> has the same effect as my patch: the build failure is replaced by
>> gcc hanging instead in the critical configuration.
> 
> Ok. No sane workaround we can do to avoid the hang I suspect, right?

I'm not aware of any.

> Should we propagate the fix/enhancement to x86/urgent? It's currently in 
> x86/asm for a v4.20 merge.

My patch triggering the issue is queued for 4.20, too.

There might be other constellations triggering it, but I haven't
seen any yet.


Juergen