Hello Peter,

- Is someone prepared to help writing this patch ?
>
>
Here my contribution. This is a patch to replace all atomic operations with
GCC atomic builtins. It has been succesfully tested on x86_64 and ARMv7
(Cortex-A8) with GCC 4.4.3. What remains to do now is :

- Refactor the code as you proposed;
- Cleanup old targets (oro_ppc, oro_i386, ...);
- Add some validation about "known supported architecture".

I also attach a second patch to add the ARM architecture based on the new
GCC atomic builtins.

Philippe

On Wed, 13 Apr 2011, Peter Soetens wrote:

> This thread is meant to resolve a possible license issue in the
> rtt/os/oro_<arch> subdirectories[1]. These directories may contain code which
> is covered by the GPL, since some snippets are copied from the Linux kernel
> tree.

Then it is "GPL + linking exception", which is a somewhat different story...

> These snippets contain atomic operations on 'int' and compare-and-swap
> assembly instructions.
>
> Even if 'adding x to y' or 'swap a with b' could be written in only few ways,
> especially when using the assembly instructions of a specific compiler (gcc)
> for a specific CPU, the similarity with the 'atomic.h' and 'system.h' headers
> of the Linux kernel might be considered by some as a violation of the GPL.

Torvalds doesn't seem to think so:
<http://www.zimbio.com/Linus+Torvalds/articles/3STt5Iue5mC/Linus+Torvalds+Dismisses+Charges+Android+Commits>
Althought he Android case is of course not exactly the same as the Orocos
case...

> Fortunately, these are only a few functions and there are several ways in
> resolving this. Here is the list of functions we depend on:
>
> oro_atomic_set / oro_atomic_read
> oro_atomic_inc
> oro_atomic_dec
> oro_atomic_dec_and_test
> oro_atomic_inc_and_test
> oro_cmpxchg
>
> We would also require to adapt the os::Atomic class to only expose these
> functions.
>
> Since the assembly instructions are written in gcc assembler, the most
> straightforward way is to replace them with the gcc intrinsics[2], which is
> present for Intel architectures from GCC 4.1 and for ARM from GCC 4.4 on.
> Philippe Hamelin already tested these functions, and they emit, not
> surprisingly, the same assembler code as we have now in the headers hard-
> coded. An example of using these intrinsics can be found here: [3].

So, it is not derived work...! And hence it falls outside of the license
issue.

> I would also recommend to create a single new header, oro_arch.h which
> contains these architecture-specific functions, which replaces oro_atomic.h and
> oro_system.h.
>
> We can develop this in parallel by creating an os/oro_gcc directory, similar
> to os/oro_msvc which contains these intrinsics. When the gcc compiler is
> detected, we can switch to including headers from that directory.
>
> Another thing we can do in parallel is doing such as ROS has done, using the
> proposed boost::atomic library in an os/oro_boost_atomic subdirectory.
>
> Other alternatives to using GCC intrinsics or boost::atomic are found in [4].
> OPA is MIT licensed, for example. Note that MSWindows installations are not
> affected by this issue.
>
> These are the open questions I have for the community:
> - Did you use any of the oro_atomic/os::Atomic functions in your code ?
> - Should we support more atomic operations than the 7 listed above ?
> - Is someone prepared to help writing this patch ?
>
> We can also provide this patch for the RTT 1.x branches.
>
> Peter
>
> [1] Affected: oro_i386, oro_x86_64, oro_powerpc, oro_arm.
> Not affected: oro_msvc, oro_noasm
> [2] http://gcc.gnu.org/onlinedocs/gcc-4.4.0/gcc/Atomic-Builtins.html
> [3] http://golubenco.org/?p=7
> [4] http://stackoverflow.com/questions/1158374/portable-compare-and-swap-
> atomic-operations-c-c-library
> https://trac.mcs.anl.gov/projects/openpa/
> http://www.chaoticmind.net/~hcb/projects/boost.atomic/
> http://www.ros.org/wiki/rosatomic

Herman