After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 498810 - Suboptimal atomic operations on ARM
Suboptimal atomic operations on ARM
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2007-11-21 18:39 UTC by Sebastian Dröge (slomo)
Modified: 2013-07-17 13:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Support for ARMv6 instructions and Linux kernel helpers (7.81 KB, patch)
2008-01-07 14:20 UTC, Jussi Laako
none Details | Review
Support for ARMv6 instructions and Linux kernel helpers (take 2) (7.81 KB, patch)
2008-01-11 09:26 UTC, Jussi Laako
none Details | Review
Updated patch (v3) (5.60 KB, patch)
2009-09-01 07:33 UTC, Jussi Laako
none Details | Review
__kernel_cmpxchg based Linux implementation from Jussi's patches (4.17 KB, patch)
2009-09-09 17:11 UTC, Adrian Bunk
none Details | Review

Description Sebastian Dröge (slomo) 2007-11-21 18:39:18 UTC
Hi,
the atomic operations glib offers are suboptimal on arm, see
http://0pointer.de/blog/projects/atomic-rt

In the comments a solution is given that would work properly on Linux:
http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob_plain;f=arch/arm/kernel/entry-armv.S;hb=HEAD

(look for __kernel_cmpxchg)

On other platforms the current solution should probably be kept though.

Bye
Comment 1 Jussi Laako 2007-11-22 08:23:17 UTC
I've seen that approach, but it's probably not feasible in cases where scratchbox/qemu is used as a development environment. Autodetection for the support might be interesting...

I also have ARMv6 inline assembly implementation. I'll try to submit extended patch later.

sched_yield() works also in realtime mode (see kernel sources), but with certain limitations. Currently glib also has other "features" related to realtime scheduling. I'm trying to address these too. For realtime (SCHED_FIFO/SCHED_RR) usage, I would just disable the ARMv5 assembly support and fall back to the default behavior or use ARMv6 assembly implementation if possible.

The big question is if realtime scheduling is supposed to be supported in general and for all architectures?
Comment 2 Sebastian Dröge (slomo) 2007-11-22 08:40:58 UTC
As I understand it the approach that uses this kernel "function" will always work, if the hardware (i.e. qemu ;) ) doesn't support atomic ops the kernel will do some emulation instead.
Comment 3 Jussi Laako 2007-11-22 08:50:20 UTC
(In reply to comment #2)
> As I understand it the approach that uses this kernel "function" will always
> work, if the hardware (i.e. qemu ;) ) doesn't support atomic ops the kernel
> will do some emulation instead.

With qemu, you are likely not running ARM kernel, but for example x86 kernel and I slightly doubt that this kernel functionality would be emulated by qemu.
Comment 4 Jussi Laako 2007-11-22 08:59:11 UTC
(In reply to comment #3)
> With qemu, you are likely not running ARM kernel, but for example x86 kernel
> and I slightly doubt that this kernel functionality would be emulated by qemu.

Correction, according to some testing seems to work. :)
Comment 5 Sebastian Dröge (slomo) 2007-11-22 09:02:20 UTC
Ah, so where's the problem then if this code is #ifdef'd to only be compiled in on  Linux/ARM? It won't be compiled in with your testing code in qemu that is x86, will fall back on older arms to the kernel atomic ops emulation and will be the real thing on newer arms.

IMHO there's no reason to not have this in GLib (and libatomic-ops but that's another story) ;)
Comment 6 Jussi Laako 2007-11-22 09:11:42 UTC
(In reply to comment #5)
> on  Linux/ARM? It won't be compiled in with your testing code in qemu that is
> x86, will fall back on older arms to the kernel atomic ops emulation and will

No, the actual concern was that with qemu-arm target the actual host is likely x86 so there's no actual arm kernel running in emulated environment.

But looks like this functionality is emulated somewhere as I got it working under qemu.
Comment 7 Sebastian Dröge (slomo) 2007-12-09 05:15:26 UTC
Any news on this?
Comment 8 Jussi Laako 2008-01-07 14:20:28 UTC
Created attachment 102327 [details] [review]
Support for ARMv6 instructions and Linux kernel helpers

Better ARMv6 atomic operation detection code would be more than welcome...
Comment 9 Jussi Laako 2008-01-11 09:26:42 UTC
Created attachment 102578 [details] [review]
Support for ARMv6 instructions and Linux kernel helpers (take 2)

Fixed a bug...

Some extensive testing still pending...
Comment 10 Sebastian Dröge (slomo) 2008-02-13 14:31:11 UTC
Looks good from a short view... any news on testing this and getting it in?
Comment 11 Matthias Clasen 2008-05-15 05:23:14 UTC
Jussi, any update ?
Comment 12 Jussi Laako 2008-05-21 13:27:33 UTC
(In reply to comment #11)
> Jussi, any update ?

Not much, been too busy on other things recently.

Based on my limited testing this seems to work, but I would really prefer if someone else would also test this.

Any comments from others (CC)?
Comment 13 Joshua Element Green 2009-06-13 20:03:31 UTC
Not only are the ARM atomic operations sub-optimal currently, but they lead to deadlocks between SCHED_FIFO and normal SCHED_OTHER threads on Linux, since SCHED_FIFO threads will not yield to SCHED_OTHER.  The PulseAudio source code is LGPL and seems to have a pretty complete set of atomic operations (src/pulsecore/atomic.h) which will use GCC 4.1 atomic instructions, libatomic_ops, or the Linux kernel atomic operations if available.

From what I can tell, the proposed patch doesn't have memory barriers.
Comment 14 Matthias Clasen 2009-09-01 02:29:51 UTC
Someone needs to come up with a (tested) patch. I don't have access to any arm systems...
Comment 15 Jussi Laako 2009-09-01 07:33:11 UTC
Created attachment 142205 [details] [review]
Updated patch (v3)

Maemo 5 will likely contain a variant of attached patch. So I could consider it tested.

Inline assembly version is not SMP-safe though. I would recommend using Linux kernel helpers on SMP-systems as it has automatic barriers (based on kernel configuration). Having the barriers always built-in would result in performance degradation in UP systems which are majority of ARM systems these days.

I leave configure magic out of this patch. Previous patch had one approach, but I'm not an autotools wizard, so someone else could probably come up with better one...
Comment 16 Adrian Bunk 2009-09-09 17:11:39 UTC
Created attachment 142811 [details] [review]
__kernel_cmpxchg based Linux implementation from Jussi's patches

There are two things mixed in the patches and discussions:
1. a __kernel_cmpxchg based Linux implementation
2. an assembler implementation for !Linux >= ARMv6

1. seems to not be disputed, and that's the part of the patch that gets testing in Maemo 5.

It's also becoming an urgent urgent since:
- the swp instruction is not available in Thumb-2 builds and
- there are Cortex-A9 processors that don't support the swp instruction

I've therefore extracted 1. from Jussi's patches, hoping it gets applied quicker this way.
Comment 17 Sebastian Dröge (slomo) 2010-05-10 07:55:02 UTC
Any news on this? Is this maybe automagically fixed by 37716bd00a7911de545ebca3dc7a248503eaf46e, which added support for using the gcc atomic builtins?
Comment 18 Jussi Laako 2010-05-10 08:06:35 UTC
(In reply to comment #17)
> Any news on this? Is this maybe automagically fixed by
> 37716bd00a7911de545ebca3dc7a248503eaf46e, which added support for using the gcc
> atomic builtins?

There's one variant of inline assembly in use in Maemo 5 (N900), earlier versions used the Linux kernel helpers.

When I was developing these patches I also tested the gcc atomic builtins, which practically were not available on ARM. I have not checked latest gcc versions though...
Comment 19 Adrian Bunk 2010-05-16 22:55:32 UTC
(In reply to comment #17)
> Any news on this? Is this maybe automagically fixed by
> 37716bd00a7911de545ebca3dc7a248503eaf46e, which added support for using the gcc
> atomic builtins?

What about other compilers like RVDS?
Comment 20 Simon McVittie 2011-07-20 17:38:24 UTC
(In reply to comment #18)
> When I was developing these patches I also tested the gcc atomic builtins,
> which practically were not available on ARM. I have not checked latest gcc
> versions though...

Has this changed since? glib/gatomic.h in 2.29 appears to use the gcc builtins (__sync_fetch_and_add, etc.) or fall back to locking.
Comment 21 Simon McVittie 2011-07-20 18:17:34 UTC
(In reply to comment #20)
> Has this changed since? glib/gatomic.h in 2.29 appears to use the gcc builtins
> (__sync_fetch_and_add, etc.) or fall back to locking.

FYI, <http://gcc.gnu.org/gcc-4.4/changes.html> says that gcc 4.4 "now supports the __sync_* atomic operations for ARM EABI GNU/Linux".
Comment 22 Sebastian Dröge (slomo) 2013-07-17 13:20:23 UTC
GLib is using the compiler operations now, so obsolete.