2015-08-18 08:18:06

by Sarbojit Ganguly

[permalink] [raw]
Subject: [PATCH] arm: Adding support for atomic half word exchange


<Ping>

Since 16 bit half word exchange was not there and MCS based qspinlock by Waiman's xchg_tail() requires an atomic exchange on a half word, here is a small modification to __xchg() code to support the exchange.
ARMv6 and lower does not have support for LDREXH, so we need to make sure things do not break when we're compiling on ARMv6.

Signed-off-by: Sarbojit Ganguly <[email protected]>
---
arch/arm/include/asm/cmpxchg.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h index 1692a05..547101d 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -50,6 +50,24 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
: "r" (x), "r" (ptr)
: "memory", "cc");
break;
+#if !defined (CONFIG_CPU_V6)
+ /*
+ * Halfword exclusive exchange
+ * This is new implementation as qspinlock
+ * wants 16 bit atomic CAS.
+ * This is not supported on ARMv6.
+ */
+ case 2:
+ asm volatile("@ __xchg2\n"
+ "1: ldrexh %0, [%3]\n"
+ " strexh %1, %2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (ret), "=&r" (tmp)
+ : "r" (x), "r" (ptr)
+ : "memory", "cc");
+ break;
+#endif
case 4:
asm volatile("@ __xchg4\n"
"1: ldrex %0, [%3]\n"
--
Sarbojit????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?


2015-08-19 16:13:08

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm: Adding support for atomic half word exchange

On Tue, Aug 18, 2015 at 09:17:53AM +0100, Sarbojit Ganguly wrote:
> <Ping>
>
> Since 16 bit half word exchange was not there and MCS based qspinlock by Waiman's xchg_tail() requires an atomic exchange on a half word, here is a small modification to __xchg() code to support the exchange.
> ARMv6 and lower does not have support for LDREXH, so we need to make sure things do not break when we're compiling on ARMv6.

First of all, please wrap the text appropriately.

Secondly, you need to cc the relevant maintainer and mailing list (try
running ./scripts/get_maintainer.pl on this patch to get some hints).

--
Catalin

2015-08-20 06:45:53

by Sarbojit Ganguly

[permalink] [raw]
Subject: Re: Re: [PATCH] arm: Adding support for atomic half word exchange

My apologies, the e-mail editor was not configured properly.
CC'ed to relevant maintainers and reposting once again with proper formatting.

Since 16 bit half word exchange was not there and MCS based qspinlock
by Waiman's xchg_tail() requires an atomic exchange on a half word,
here is a small modification to __xchg() code to support the exchange.
ARMv6 and lower does not have support for LDREXH, so we need to make sure things
do not break when we're compiling on ARMv6.

Signed-off-by: Sarbojit Ganguly <[email protected]>
---
arch/arm/include/asm/cmpxchg.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 1692a05..547101d 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -50,6 +50,24 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
: "r" (x), "r" (ptr)
: "memory", "cc");
break;
+#if !defined (CONFIG_CPU_V6)
+ /*
+ * Halfword exclusive exchange
+ * This is new implementation as qspinlock
+ * wants 16 bit atomic CAS.
+ * This is not supported on ARMv6.
+ */
+ case 2:
+ asm volatile("@ __xchg2\n"
+ "1: ldrexh %0, [%3]\n"
+ " strexh %1, %2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (ret), "=&r" (tmp)
+ : "r" (x), "r" (ptr)
+ : "memory", "cc");
+ break;
+#endif
case 4:
asm volatile("@ __xchg4\n"
"1: ldrex %0, [%3]\n"



Regards,
Sarbojit

------- Original Message -------
Sender : Catalin Marinas<[email protected]>
Date : Aug 19, 2015 21:43 (GMT+05:30)
Title : Re: [PATCH] arm: Adding support for atomic half word exchange

On Tue, Aug 18, 2015 at 09:17:53AM +0100, Sarbojit Ganguly wrote:
>
>
> Since 16 bit half word exchange was not there and MCS based qspinlock by Waiman's xchg_tail() requires an atomic exchange on a half word, here is a small modification to __xchg() code to support the exchange.
> ARMv6 and lower does not have support for LDREXH, so we need to make sure things do not break when we're compiling on ARMv6.

First of all, please wrap the text appropriately.

Secondly, you need to cc the relevant maintainer and mailing list (try
running ./scripts/get_maintainer.pl on this patch to get some hints).

--
Catalin????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-20 08:24:41

by Sarbojit Ganguly

[permalink] [raw]
Subject: Re: Re: Re: [PATCH] arm: Adding support for atomic half word exchange

Hello Russell,

My apologies for the accidental HTML formatting.
I have reconfigured my e-mail client to use text-only format by default.
Here is the patch and its description.


Since 16 bit half word exchange was not there and MCS based qspinlock
by Waiman's xchg_tail() requires an atomic exchange on a half word,
here is a small modification to __xchg() code to support the exchange.
ARMv6 and lower does not have support for LDREXH, so we need to make sure things
do not break when we're compiling on ARMv6.

Signed-off-by: Sarbojit Ganguly
---
arch/arm/include/asm/cmpxchg.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 1692a05..547101d 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -50,6 +50,24 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
: "r" (x), "r" (ptr)
: "memory", "cc");
break;
+#if !defined (CONFIG_CPU_V6)
+ /*
+ * Halfword exclusive exchange
+ * This is new implementation as qspinlock
+ * wants 16 bit atomic CAS.
+ * This is not supported on ARMv6.
+ */
+ case 2:
+ asm volatile("@ __xchg2 "
+ "1: ldrexh %0, [%3] "
+ " strexh %1, %2, [%3] "
+ " teq %1, #0 "
+ " bne 1b"
+ : "=&r" (ret), "=&r" (tmp)
+ : "r" (x), "r" (ptr)
+ : "memory", "cc");
+ break;
+#endif
case 4:
asm volatile("@ __xchg4 "
"1: ldrex %0, [%3] "



Regards,
Sarbojit

------- Original Message -------
Sender : Russell King - ARM Linux<[email protected]>
Date : Aug 20, 2015 13:49 (GMT+05:30)
Title : Re: Re: [PATCH] arm: Adding support for atomic half word exchange

This email is unreadable. Do NOT use html email when discussing linux
matters. Many mailing lists will reject or discard your message.

On Thu, Aug 20, 2015 at 06:47:50AM +0000, Sarbojit Ganguly wrote:
>
>
>
>
>
>
>
>
>
My apologies, the e-mail editor was not configured properly.
CC'ed to relevant maintainers and reposting once again with proper formatting.

Since 16 bit half word exchange was not there and MCS based qspinlock
by Waiman's xchg_tail() requires an atomic exchange on a half word,
here is a small modification to __xchg() code to support the exchange.
ARMv6 and lower does not have support for LDREXH, so we need to make sure things
do not break when we're compiling on ARMv6.

Signed-off-by: Sarbojit Ganguly
---
arch/arm/include/asm/cmpxchg.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 1692a05..547101d 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -50,6 +50,24 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
: "r" (x), "r" (ptr)
: "memory", "cc");
break;
+#if !defined (CONFIG_CPU_V6)
+ /*
+ * Halfword exclusive exchange
+ * This is new implementation as qspinlock
+ * wants 16 bit atomic CAS.
+ * This is not supported on ARMv6.
+ */
+ case 2:
+ asm volatile("@ __xchg2 "
+ "1: ldrexh %0, [%3] "
+ " strexh %1, %2, [%3] "
+ " teq %1, #0 "
+ " bne 1b"
+ : "=&r" (ret), "=&r" (tmp)
+ : "r" (x), "r" (ptr)
+ : "memory", "cc");
+ break;
+#endif
case 4:
asm volatile("@ __xchg4 "
"1: ldrex %0, [%3] "



Regards,
Sarbojit

------- Original Message -------
Sender : Catalin Marinas
Date : Aug 19, 2015 21:43 (GMT+05:30)
Title : Re: [PATCH] arm: Adding support for atomic half word exchange

On Tue, Aug 18, 2015 at 09:17:53AM +0100, Sarbojit Ganguly wrote:
>
>
> Since 16 bit half word exchange was not there and MCS based qspinlock by Waiman's xchg_tail() requires an atomic exchange on a half word, here is a small modification to __xchg() code to support the exchange.
> ARMv6 and lower does not have support for LDREXH, so we need to make sure things do not break when we're compiling on ARMv6.

First of all, please wrap the text appropriately.

Secondly, you need to cc the relevant maintainer and mailing list (try
running ./scripts/get_maintainer.pl on this patch to get some hints).

--
Catalin

>


>

>
감사합니다

>
사보짓 선임 삼성 전자

>
----------------------------------------------------------------------+

>
The Tao lies beyond Yin and Yang. It is silent and still as a pool of water. |

>
It does not seek fame, therefore nobody knows its presence. |

>
It does not seek fortune, for it is complete within itself. |

>
It exists beyond space and time. |

>
----------------------------------------------------------------------+

>


>
>
>
>
>




--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.


감사합니다
사보짓 선임 삼성 전자
----------------------------------------------------------------------+
The Tao lies beyond Yin and Yang. It is silent and still as a pool of water. |
It does not seek fame, therefore nobody knows its presence. |
It does not seek fortune, for it is complete within itself. |
It exists beyond space and time. |
----------------------------------------------------------------------+????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-08-20 10:38:17

by Will Deacon

[permalink] [raw]
Subject: Re: Re: [PATCH] arm: Adding support for atomic half word exchange

On Thu, Aug 20, 2015 at 07:40:44AM +0100, Sarbojit Ganguly wrote:
> My apologies, the e-mail editor was not configured properly.
> CC'ed to relevant maintainers and reposting once again with proper formatting.
>
> Since 16 bit half word exchange was not there and MCS based qspinlock
> by Waiman's xchg_tail() requires an atomic exchange on a half word,
> here is a small modification to __xchg() code to support the exchange.
> ARMv6 and lower does not have support for LDREXH, so we need to make sure things
> do not break when we're compiling on ARMv6.
>
> Signed-off-by: Sarbojit Ganguly <[email protected]>
> ---
> arch/arm/include/asm/cmpxchg.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
> index 1692a05..547101d 100644
> --- a/arch/arm/include/asm/cmpxchg.h
> +++ b/arch/arm/include/asm/cmpxchg.h
> @@ -50,6 +50,24 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
> : "r" (x), "r" (ptr)
> : "memory", "cc");
> break;
> +#if !defined (CONFIG_CPU_V6)
> + /*
> + * Halfword exclusive exchange
> + * This is new implementation as qspinlock
> + * wants 16 bit atomic CAS.
> + * This is not supported on ARMv6.
> + */

I don't think you need this comment. We don't use qspinlock on arch/arm/.

> + case 2:
> + asm volatile("@ __xchg2\n"
> + "1: ldrexh %0, [%3]\n"
> + " strexh %1, %2, [%3]\n"
> + " teq %1, #0\n"
> + " bne 1b"
> + : "=&r" (ret), "=&r" (tmp)
> + : "r" (x), "r" (ptr)
> + : "memory", "cc");
> + break;
> +#endif
> case 4:
> asm volatile("@ __xchg4\n"
> "1: ldrex %0, [%3]\n"

We have the same issue with the byte exclusives, so I think you need
to extend the guard you're adding to cover that case too (which is a bug
in current mainline).

Will

2015-08-20 14:25:58

by Sarbojit Ganguly

[permalink] [raw]
Subject: Re: Re: Re: [PATCH] arm: Adding support for atomic half word exchange

>> My apologies, the e-mail editor was not configured properly.
>> CC'ed to relevant maintainers and reposting once again with proper formatting.
>>
>> Since 16 bit half word exchange was not there and MCS based qspinlock
>> by Waiman's xchg_tail() requires an atomic exchange on a half word,
>> here is a small modification to __xchg() code to support the exchange.
>> ARMv6 and lower does not have support for LDREXH, so we need to make
>> sure things do not break when we're compiling on ARMv6.
>>
>> Signed-off-by: Sarbojit Ganguly <[email protected]>>
>> ---
>> arch/arm/include/asm/cmpxchg.h | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/cmpxchg.h
>> b/arch/arm/include/asm/cmpxchg.h index 1692a05..547101d 100644
>> --- a/arch/arm/include/asm/cmpxchg.h
>> +++ b/arch/arm/include/asm/cmpxchg.h
>> @@ -50,6 +50,24 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
>> : "r" (x), "r" (ptr)
>> : "memory", "cc");
>> break;
>> +#if !defined (CONFIG_CPU_V6)
>> + /*
>> + * Halfword exclusive exchange
>> + * This is new implementation as qspinlock
>> + * wants 16 bit atomic CAS.
>> + * This is not supported on ARMv6.
>> + */

>I don't think you need this comment. We don't use qspinlock on arch/arm/.

Yes, till date mainline ARM does not support but I've ported Qspinlock on ARM hence I think that comment
might be required.

>> + case 2:
>> + asm volatile("@ __xchg2\n"
>> + "1: ldrexh %0, [%3]\n"
>> + " strexh %1, %2, [%3]\n"
>> + " teq %1, #0\n"
>> + " bne 1b"
>> + : "=&r" (ret), "=&r" (tmp)
>> + : "r" (x), "r" (ptr)
>> + : "memory", "cc");
>> + break;
>> +#endif
>> case 4:
>> asm volatile("@ __xchg4\n"
>> "1: ldrex %0, [%3]\n"

>We have the same issue with the byte exclusives, so I think you need to extend the guard you're adding to cover that case too (which is a bug in current mainline).

Ok, I will work on this and release a v2 soon.

>Will

- Sarbojit
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?