Hi Leif, thanks for the comments. > - The v5 code doesn't actually make use of the kuser helper barriers > in its versions of opal_atomic_cmpset_acq/rel. Quote the documentation, __kuser_cmpxchg "already includes memory barriers as needed," so the code using it shouldn't need anything extra.
> - The line > +#if (OMPI_GCC_INLINE_ASSEMBLY || (OPAL_ASM_ARM_VERSION < 6)) > is correct, but does my head in. Could another define like > OPAL_ARM_KUSER_BARRIERS or similar be added by the barrier definition > and used here instead, to improve readability? > - Could you change the line > +#if (OPAL_ASM_ARM_VERSION >= 6 && OMPI_GCC_INLINE_ASSEMBLY) > to > +#if (OMPI_GCC_INLINE_ASSEMBLY && (OPAL_ASM_ARM_VERSION >= 6)) > again for readability? Tweaked the patch, should be a little nicer now. > - The patch does not do a runtime test for kernel helper version. While > normally not a problem, this could cause tricky to debug issues if > running on top of old kernels (as in "executing uninitialized > memory" tricky). I'm not familiar enough with the codebase to know where it should go, and I suspect the obvious check based on the __kuser_helper_version will not be available on kernels much older than the ones with the features we need anyway. We'd only have a problem on kernels older than 2.6.15, which was released in January of 2006, and I'm only confident we'd have __kuser_help_version from 2.6.12 onward. > - This patch does not include out-of-line assembly versions > (in opal/asm/base) for the atomic operations. However I am not sure > if this causes any practical problems. As far as I can tell, this isn't an issue, since no actual new assembly is added or changed. > - If 64-bit atomics are desirable, these are actually possible on most > ARMv6 platforms (including the Raspberry PI), so a configure test on > whether LDREXD assembles without errors could be used to detect this. I'd add another case to the architecture detection in the config script, but I'm not sure what strings to match on (i.e. uname outputs) for detecting presence of LDREXD, and I'm not aware of any particular benefit to having the 64bit functions. New patch against current trunk (r26333) attached. (sorry Leif and possibly Jeffrey if I double-posted, messed up some address stuff)
ompi_armv5-2nd.diff
Description: Binary data
