Yes, I will try to do that. OTOH, I saw that there was a discussion on removal of bad_xchg() altogether. Perhaps that approach be better than adding this half word exchange?
Regards,
Sarbojit
------- Original Message -------
Sender : Arnd Bergmann<[email protected]>
Date : May 19, 2015 18:51 (GMT+09:00)
Title : Re: [RFC] arm: Add for atomic half word exchange
On Tuesday 19 May 2015 09:39:33 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.
We have discussed a similar patch before, see
https://lkml.org/lkml/2015/2/25/390
> #if __LINUX_ARM_ARCH__ >= 6
> @@ -50,6 +52,23 @@
> : "r" (x), "r" (ptr)
> : "memory", "cc");
> break;
> + /*
> + * halfword exclusive exchange
> + * This is new implementation as qspinlock
> + * wants 16 bit atomic CAS.
> + */
> + 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;
> case 4:
> asm volatile("@ __xchg4\n"
> "1: ldrex %0, [%3]\n"
Please try to find a way to make this compile when CONFIG_CPU_V6
is set.
Arnd
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Tuesday 19 May 2015 11:20:11 Sarbojit Ganguly wrote:
> Yes, I will try to do that. OTOH, I saw that there was a discussion on
> removal of bad_xchg() altogether. Perhaps that approach be better than
> adding this half word exchange?
Removing bad_xchg() only helps insofar as we get a link-time error
instead of a run-time error. That is certainly a good idea, but it
does not solve your problem.
Arnd
On Tue, May 19, 2015 at 11:20:11AM +0000, Sarbojit Ganguly wrote:
> Yes, I will try to do that. OTOH, I saw that there was a discussion on
> removal of bad_xchg() altogether. Perhaps that approach be better than
> adding this half word exchange?
The only possibility for removal of __bad_xchg() is to remove it's
_definition_ only, not its callsite, so that we get a _link_ time
error for use cases we don't support.
Removing its callsite leaves us open to code malfunction: xchg()
effectively becomes a no-op for sizes which are not supported, and
that's a _very_ bad thing to happen.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
On Tue, May 19, 2015 at 11:20:13AM +0000, Sarbojit Ganguly wrote:
> On Tuesday 19 May 2015 09:39:33 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.
Can you actually see a performance improvement with the qspinlock code
on ARM ?
The real improvements on x86 were on NUMA systems; although there were
real improvements on light loads as well.
Note that ARM (or any load-store arch) could get rid of all the cmpxchg
loops in that code. Although I suppose we replaced the most common ones
with these unconditional atomics already -- like that xchg16 -- so
implementing those with ll/sc, as you did, should be near optimal.
Yes, the main advantage of Qspinlock code can be observed in NUMA but when I tested in an embedded system, a slight advantage was observed.
------- Original Message -------
Sender : Peter Zijlstra<[email protected]>
Date : May 19, 2015 21:43 (GMT+09:00)
Title : Re: [RFC] arm: Add for atomic half word exchange
On Tue, May 19, 2015 at 11:20:13AM +0000, Sarbojit Ganguly wrote:
> On Tuesday 19 May 2015 09:39:33 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.
Can you actually see a performance improvement with the qspinlock code
on ARM ?
The real improvements on x86 were on NUMA systems; although there were
real improvements on light loads as well.
Note that ARM (or any load-store arch) could get rid of all the cmpxchg
loops in that code. Although I suppose we replaced the most common ones
with these unconditional atomics already -- like that xchg16 -- so
implementing those with ll/sc, as you did, should be near optimal.
?????
??? ?? ?? ??
----------------------------------------------------------------------+
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?
On Wednesday 20 May 2015 05:09:35 Sarbojit Ganguly wrote:
> > ------- Original Message -------
> > Sender : Peter Zijlstra<[email protected]>
> > Date : May 19, 2015 21:43 (GMT+09:00)
> > Title : Re: [RFC] arm: Add for atomic half word exchange
> >
> > On Tue, May 19, 2015 at 11:20:13AM +0000, Sarbojit Ganguly wrote:
> > > On Tuesday 19 May 2015 09:39:33 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.
> >
> > Can you actually see a performance improvement with the qspinlock code
> > on ARM ?
> >
> > The real improvements on x86 were on NUMA systems; although there were
> > real improvements on light loads as well.
> >
> >
> > Note that ARM (or any load-store arch) could get rid of all the cmpxchg
> > loops in that code. Although I suppose we replaced the most common ones
> > with these unconditional atomics already -- like that xchg16 -- so
> > implementing those with ll/sc, as you did, should be near optimal.
>
> Yes, the main advantage of Qspinlock code can be observed in NUMA but
> when I tested in an embedded system, a slight advantage was observed.
Is this a multi-cluster SMP system? Those can behave like NUMA
machines in some ways.
We could easily limit the use of 16-bit xchg() to ARMv7 machines
by using
select ARCH_USE_QUEUED_SPINLOCKS if !SMP_ON_UP
or
select ARCH_USE_QUEUED_SPINLOCKS if !CPU_V6
when enabling the qspinlock implementation.
Arnd
On Wed, May 20, 2015 at 05:09:34AM +0000, Sarbojit Ganguly wrote:
> Yes, the main advantage of Qspinlock code can be observed in NUMA but
> when I tested in an embedded system, a slight advantage was observed.
OK, great!
This is the first !x86 port I'm aware of and a load-store arch at that,
so its good to hear our efforts at making it generic actually paid off.
I made the CONFIG_ARCH_MULTI_V6=y and
CONFIG_CPU_V6K=y
CONFIG_CPU_32v6=y
CONFIG_CPU_32v6K=y
and compiled 4.0.4 with the patch. Result is a compilation success.
Regards,
Sarbojit
------- Original Message -------
Sender : Arnd Bergmann<[email protected]>
Date : May 19, 2015 18:51 (GMT+09:00)
Title : Re: [RFC] arm: Add for atomic half word exchange
On Tuesday 19 May 2015 09:39:33 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.
We have discussed a similar patch before, see
https://lkml.org/lkml/2015/2/25/390
> #if __LINUX_ARM_ARCH__ >= 6
> @@ -50,6 +52,23 @@
> : "r" (x), "r" (ptr)
> : "memory", "cc");
> break;
> + /*
> + * halfword exclusive exchange
> + * This is new implementation as qspinlock
> + * wants 16 bit atomic CAS.
> + */
> + 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;
> case 4:
> asm volatile("@ __xchg4\n"
> "1: ldrex %0, [%3]\n"
Please try to find a way to make this compile when CONFIG_CPU_V6
is set.
Arnd????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 06/02/2015 11:19 AM, Sarbojit Ganguly wrote:
> I made the CONFIG_ARCH_MULTI_V6=y and
> CONFIG_CPU_V6K=y
> CONFIG_CPU_32v6=y
> CONFIG_CPU_32v6K=y
>
> and compiled 4.0.4 with the patch. Result is a compilation success.
>
> Regards,
> Sarbojit
>
Hi Sarbojit,
I am not familiar about the implication of setting those options
unconditionally, But from Kconfig point of view
Arnd was expecting something like below IIUC
with the patch: (a quick example)
---8<---
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9f1f09a..194fc13 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -812,6 +812,8 @@ config ARCH_MULTI_V6
bool "ARMv6 based platforms (ARM11)"
select ARCH_MULTI_V6_V7
select CPU_V6K
+ select CPU_32v6
+ select CPU_32v6k
config ARCH_MULTI_V7
bool "ARMv7 based platforms (Cortex-A, PJ4, Scorpion, Krait)"
> ------- Original Message -------
> Sender : Arnd Bergmann<[email protected]>
> Date : May 19, 2015 18:51 (GMT+09:00)
> Title : Re: [RFC] arm: Add for atomic half word exchange
>
> On Tuesday 19 May 2015 09:39:33 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.
>
> We have discussed a similar patch before, see
> https://lkml.org/lkml/2015/2/25/390
>
>> #if __LINUX_ARM_ARCH__ >= 6
>> @@ -50,6 +52,23 @@
>> : "r" (x), "r" (ptr)
>> : "memory", "cc");
>> break;
>> + /*
>> + * halfword exclusive exchange
>> + * This is new implementation as qspinlock
>> + * wants 16 bit atomic CAS.
>> + */
>> + 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;
>> case 4:
>> asm volatile("@ __xchg4\n"
>> "1: ldrex %0, [%3]\n"
>
> Please try to find a way to make this compile when CONFIG_CPU_V6
> is set.
>
> Arnd
>
Hello Raghavendra,
That is exactly I had done albeit from menuconfig. Basically the whole point was to make sure my patch compiles against ARM11 architectures as well. Hence I ensured the .config contains the relevant flags on.
Regards,
Sarbojit
------- Original Message -------
Sender : Raghavendra K T<[email protected]>
Date : Jun 02, 2015 15:11 (GMT+09:00)
Title : Re: [RFC] arm: Add for atomic half word exchange
On 06/02/2015 11:19 AM, Sarbojit Ganguly wrote:
> I made the CONFIG_ARCH_MULTI_V6=y and
> CONFIG_CPU_V6K=y
> CONFIG_CPU_32v6=y
> CONFIG_CPU_32v6K=y
>
> and compiled 4.0.4 with the patch. Result is a compilation success.
>
> Regards,
> Sarbojit
>
Hi Sarbojit,
I am not familiar about the implication of setting those options
unconditionally, But from Kconfig point of view
Arnd was expecting something like below IIUC
with the patch: (a quick example)
---8<---
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9f1f09a..194fc13 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -812,6 +812,8 @@ config ARCH_MULTI_V6
bool "ARMv6 based platforms (ARM11)"
select ARCH_MULTI_V6_V7
select CPU_V6K
+ select CPU_32v6
+ select CPU_32v6k
config ARCH_MULTI_V7
bool "ARMv7 based platforms (Cortex-A, PJ4, Scorpion, Krait)"
> ------- Original Message -------
> Sender : Arnd Bergmann
> Date : May 19, 2015 18:51 (GMT+09:00)
> Title : Re: [RFC] arm: Add for atomic half word exchange
>
> On Tuesday 19 May 2015 09:39:33 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.
>
> We have discussed a similar patch before, see
> https://lkml.org/lkml/2015/2/25/390
>
>> #if __LINUX_ARM_ARCH__ >= 6
>> @@ -50,6 +52,23 @@
>> : "r" (x), "r" (ptr)
>> : "memory", "cc");
>> break;
>> + /*
>> + * halfword exclusive exchange
>> + * This is new implementation as qspinlock
>> + * wants 16 bit atomic CAS.
>> + */
>> + 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;
>> case 4:
>> asm volatile("@ __xchg4\n"
>> "1: ldrex %0, [%3]\n"
>
> Please try to find a way to make this compile when CONFIG_CPU_V6
> is set.
>
> Arnd
>
?????
??? ?? ?? ??
----------------------------------------------------------------------+
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?
On Tuesday 02 June 2015 06:21:43 Sarbojit Ganguly wrote:
> Hello Raghavendra,
>
> That is exactly I had done albeit from menuconfig. Basically the whole point was to make sure my patch compiles against ARM11 architectures as well. Hence I ensured the .config contains the relevant flags on.
>
>
Most ARM11 implementations are ARMv6k, which has support for the instruction,
but the older ARMv6 (without k) does not. From your description, I assume
you only tested with ARMv6k, not ARMv6. Please enable the OMAP2 and IMX31
platforms to test ARMv6.
Arnd
Yes, I have enabled :
#
# Processor Type
#
CONFIG_CPU_V6=y
CONFIG_CPU_V6K=y
CONFIG_CPU_32v6=y
CONFIG_CPU_32v6K=y
and
# TI OMAP/AM/DM/DRA Family
#
CONFIG_ARCH_OMAP2=y
CONFIG_ARCH_OMAP2PLUS=y
and
#
# MX31 platforms:
#
CONFIG_MACH_MX31ADS=y
CONFIG_MACH_MX31LILLY=y
CONFIG_MACH_MX31LITE=y
CONFIG_MACH_PCM037=y
CONFIG_MACH_PCM037_EET=y
CONFIG_MACH_MX31_3DS=y
CONFIG_MACH_MX31MOBOARD=y
CONFIG_MACH_QONG=y
CONFIG_MACH_ARMADILLO5X0=y
CONFIG_MACH_KZM_ARM11_01=y
CONFIG_MACH_BUG=y
CONFIG_MACH_IMX31_DT=y
The compilation is successful.
Regards,
Sarbojit
------- Original Message -------
Sender : Arnd Bergmann<[email protected]>
Date : Jun 02, 2015 19:49 (GMT+09:00)
Title : Re: [RFC] arm: Add for atomic half word exchange
On Tuesday 02 June 2015 06:21:43 Sarbojit Ganguly wrote:
> Hello Raghavendra,
>
> That is exactly I had done albeit from menuconfig. Basically the whole point was to make sure my patch compiles against ARM11 architectures as well. Hence I ensured the .config contains the relevant flags on.
>
>
Most ARM11 implementations are ARMv6k, which has support for the instruction,
but the older ARMv6 (without k) does not. From your description, I assume
you only tested with ARMv6k, not ARMv6. Please enable the OMAP2 and IMX31
platforms to test ARMv6.
Arnd
?????
??? ?? ?? ??
----------------------------------------------------------------------+
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?
Since the compilation is also a success for CONFIG_CPU_V6 with the patch, I think we're good to go.
------- Original Message -------
Sender : Sarbojit Ganguly<[email protected]> Technical Lead/SRI-Bangalore-AP Systems 1/Samsung Electronics
Date : Jun 02, 2015 20:11 (GMT+09:00)
Title : Re: Re: [RFC] arm: Add for atomic half word exchange
Yes, I have enabled :
#
# Processor Type
#
CONFIG_CPU_V6=y
CONFIG_CPU_V6K=y
CONFIG_CPU_32v6=y
CONFIG_CPU_32v6K=y
and
# TI OMAP/AM/DM/DRA Family
#
CONFIG_ARCH_OMAP2=y
CONFIG_ARCH_OMAP2PLUS=y
and
#
# MX31 platforms:
#
CONFIG_MACH_MX31ADS=y
CONFIG_MACH_MX31LILLY=y
CONFIG_MACH_MX31LITE=y
CONFIG_MACH_PCM037=y
CONFIG_MACH_PCM037_EET=y
CONFIG_MACH_MX31_3DS=y
CONFIG_MACH_MX31MOBOARD=y
CONFIG_MACH_QONG=y
CONFIG_MACH_ARMADILLO5X0=y
CONFIG_MACH_KZM_ARM11_01=y
CONFIG_MACH_BUG=y
CONFIG_MACH_IMX31_DT=y
The compilation is successful.
Regards,
Sarbojit
------- Original Message -------
Sender : Arnd Bergmann
Date : Jun 02, 2015 19:49 (GMT+09:00)
Title : Re: [RFC] arm: Add for atomic half word exchange
On Tuesday 02 June 2015 06:21:43 Sarbojit Ganguly wrote:
> Hello Raghavendra,
>
> That is exactly I had done albeit from menuconfig. Basically the whole point was to make sure my patch compiles against ARM11 architectures as well. Hence I ensured the .config contains the relevant flags on.
>
>
Most ARM11 implementations are ARMv6k, which has support for the instruction,
but the older ARMv6 (without k) does not. From your description, I assume
you only tested with ARMv6k, not ARMv6. Please enable the OMAP2 and IMX31
platforms to test ARMv6.
Arnd
?????
??? ?? ?? ??
----------------------------------------------------------------------+
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. |
----------------------------------------------------------------------+
?????
??? ?? ?? ??
----------------------------------------------------------------------+
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?
On Friday 05 June 2015 01:17:13 Sarbojit Ganguly wrote:
> Since the compilation is also a success for CONFIG_CPU_V6 with the patch,
> I think we're good to go.
I'm not following your logic. Did you create a new patch that addresses
compilation with CONFIG_CPU_V6?
The original patch should cause a compile error on v6, if it does not,
you either have a broken assembler or you forgot to enable the code
that uses these instructions. Neither of them would be a reason to
merge the patch. Please explain what is going on.
Arnd
Sorry about that. I dug a bit deeper and found that the code was not getting executed and rectified that. The compilation indeed fails for the lack of ldrexh instruction support on ARMv6.
Hence, my patch needs to be guarded with !CONFIG_CPU_V6 so as to allow the code run on >=6k and above.
I will post the v2 soon.
------- Original Message -------
Sender : Arnd Bergmann<[email protected]>
Date : Jun 05, 2015 18:03 (GMT+05:30)
Title : Re: [RFC] arm: Add for atomic half word exchange
On Friday 05 June 2015 01:17:13 Sarbojit Ganguly wrote:
> Since the compilation is also a success for CONFIG_CPU_V6 with the patch,
> I think we're good to go.
I'm not following your logic. Did you create a new patch that addresses
compilation with CONFIG_CPU_V6?
The original patch should cause a compile error on v6, if it does not,
you either have a broken assembler or you forgot to enable the code
that uses these instructions. Neither of them would be a reason to
merge the patch. Please explain what is going on.
Arnd
감사합니다
사보짓 선임 삼성 전자
----------------------------------------------------------------------+
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. |
----------------------------------------------------------------------+
감사합니다
사보짓 선임 삼성 전자
----------------------------------------------------------------------+
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?