[OE-core] [PATCH 1/1] db: disable the ARM assembler mutex code

Fri Jun 15 07:39:01 UTC 2018

On Fri, Jun 15, 2018 at 12:10 AM, Herve Jourdain <herve.jourdain at neuf.fr> wrote:
> Hi,
>
> So the issue is whether we want to change the behaviour of previous architectures, or if we try to fix the issue only for the architectures that don't work.
> Until now, the db recipe was enabling the 'swp' optimization, and that behavior could be disabled on .bbappend if needed.
> While that works fine until armv7ve, it does not work for armv8, which has removed support for those instructions.

I don't know if "works fine until armv7ve" is correct. Although the
swp instruction exists for armv7, according to the link I posted
yesterday, it is not guaranteed to work.

  https://community.arm.com/processors/b/blog/posts/locks-swps-and-two-smoking-barriers

Or do you have other evidence to suggest that swp is safe to use for armv7?

> Therefore, there is a need to fix it for armv8 - and armv8 only - whereas it can be safely used on previous architectures.
> If we remove the use for all ARM architectures, that might create some regression/issues.
> If we just remove the use of 'swp' only for armv8, we ensure it doesn't break anything that's running on previous ARM architectures.
>
> Cheers,
> Herve
>
> -----Original Message-----
> From: Andre McCurdy [mailto:armccurdy at gmail.com]
> Sent: vendredi 15 juin 2018 00:03
> To: Khem Raj <raj.khem at gmail.com>
> Cc: Herve Jourdain <herve.jourdain at neuf.fr>; Ovidiu Panait <ovidiu.panait at windriver.com>; Patches and discussions about the oe-core layer <openembedded-core at lists.openembedded.org>
> Subject: Re: [OE-core] [PATCH 1/1] db: disable the ARM assembler mutex code
>
> On Thu, Jun 14, 2018 at 2:48 PM, Khem Raj <raj.khem at gmail.com> wrote:
>> On Thu, Jun 14, 2018 at 1:01 PM Andre McCurdy <armccurdy at gmail.com> wrote:
>>> On Thu, Jun 14, 2018 at 12:24 PM, Khem Raj <raj.khem at gmail.com> wrote:
>>> > On Thu, Jun 14, 2018 at 12:12 PM Andre McCurdy
>>> > <armccurdy at gmail.com>
>>> > wrote:
>>> >>
>>> >> On Thu, Jun 14, 2018 at 9:40 AM, Khem Raj <raj.khem at gmail.com> wrote:
>>> >> > On 6/14/18 5:10 AM, Herve Jourdain wrote:
>>> >> >> Hi,
>>> >> >>
>>> >> >> I believe I solved that same problem by just adding, in the
>>> >> >> case of
>>> >> >> armv8
>>> >> >> (which I believe may be the new architecture you're referring to):
>>> >> >> MUTEX_armv8 = ""
>>> >> >> This way, it allows previous versions to work just like they
>>> >> >> did before, without having to disable ARM assembler mutex code
>>> >> >> for architectures that support it correctly - up to armv7ve I
>>> >> >> believe.
>>> >> >> Of course, we might need to also have a good definition for
>>> >> >> armv8, which is the object of another thread.
>>> >> >
>>> >> > right thats a better approach.
>>> >>
>>> >> SWP is not guaranteed to work on SMP systems... and even if it
>>> >> does, performance is likely to be worse than the pthreads version
>>> >> (which can take advantage of the newer instructions).
>>> >>
>>> >>
>>> >> https://community.arm.com/processors/b/blog/posts/locks-swps-and-t
>>> >> wo-smoking-barriers
>>> >>
>>> >> In general, use of hand optimised assembler mutex implementations
>>> >> in user space isn't something to be encouraged - use pthreads (or
>>> >> maybe a gcc intrinsic) instead.
>>> >>
>>> >
>>> > question is about disabling it on old arm machines, do we have data
>>> > where we know that other sync methods without swp works better on
>>> > armv5 and lower ?
>>>
>>> On armv5 and below a hand optimised implementation using SWP is
>>> likely to be faster than pthreads. No one is suggesting otherwise.
>>>
>>> On SMP (highly likely nowadays for armv7 and above), SWP simply might
>>> not work (aside from the fact that if it does work, it's likely to be
>>> slower than pthreads). It's not really a question of performance
>>> there, so the proposal to only disable SWP for armv8 doesn't seem
>>> like a safe solution.
>>
>> Suggestion is not to just do it for armv8 but To keep it there where
>> its beneficial
>
> You can always argue that micro optimisations are beneficial. The question is whether they make a big enough difference in some real world use case to be worth the maintenance effort.
>
>>> Using pthreads unconditionally is safe and easy. Unless you can prove
>>> that hand optimised SWP is really a big win for armv5 (is anyone
>>> really running a performance critical database on an armv5 system?)
>>> why not keep the recipe simple and use pthreads everywhere?
>>>
>>> >> I think the original patch is good.
>>> >>
>>> >> >> Cheers,
>>> >> >> Herve
>>> >> >>
>>> >> >> -----Original Message-----
>>> >> >> From: openembedded-core-bounces at lists.openembedded.org
>>> >> >> [mailto:openembedded-core-bounces at lists.openembedded.org] On
>>> >> >> Behalf Of Ovidiu Panait
>>> >> >> Sent: jeudi 14 juin 2018 13:55
>>> >> >> To: openembedded-core at lists.openembedded.org
>>> >> >> Subject: [OE-core] [PATCH 1/1] db: disable the ARM assembler
>>> >> >> mutex code
>>> >> >>
>>> >> >> The swpb in macro MUTEX_SET will cause "undefined instruction"
>>> >> >> error on the new arm arches which don't support this assembly
>>> >> >> instruction any more. If use ldrex/strex to replace swpb, the
>>> >> >> old arm arches don't support them. So to avoid this issue, just
>>> >> >> disable the ARM assembler mutex code, and use the default
>>> >> >> pthreads mutex.
>>> >> >>
>>> >> >> Signed-off-by: Li Zhou <li.zhou at windriver.com>
>>> >> >> Signed-off-by: Catalin Enache <catalin.enache at windriver.com>
>>> >> >> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
>>> >> >> ---
>>> >> >>  meta/recipes-support/db/db_5.3.28.bb | 13 +------------
>>> >> >>  1 file changed, 1 insertion(+), 12 deletions(-)
>>> >> >>
>>> >> >> diff --git a/meta/recipes-support/db/db_5.3.28.bb
>>> >> >> b/meta/recipes-support/db/db_5.3.28.bb
>>> >> >> index 093ee44909..15b4155a29 100644
>>> >> >> --- a/meta/recipes-support/db/db_5.3.28.bb
>>> >> >> +++ b/meta/recipes-support/db/db_5.3.28.bb
>>> >> >> @@ -59,18 +59,7 @@ FILES_SOLIBSDEV = "${libdir}/libdb.so
>>> >> >> ${libdir}/libdb_cxx.so"
>>> >> >>  # All the --disable-* options replace --enable-smallbuild,
>>> >> >> which breaks a bunch of stuff (eg. postfix)  DB5_CONFIG ?=
>>> >> >> "--enable-o_direct --disable-cryptography --disable-queue
>>> >> >> --disable-replication --disable-verify --disable-compat185
>>> >> >> --disable-sql"
>>> >> >>
>>> >> >> -EXTRA_OECONF = "${DB5_CONFIG} --enable-shared --enable-cxx
>>> >> >> --with-sysroot"
>>> >> >> -
>>> >> >> -# Override the MUTEX setting here, the POSIX library is -# the
>>> >> >> default - "POSIX/pthreads/library".
>>> >> >> -# Don't ignore the nice SWP instruction on the ARM:
>>> >> >> -# These enable the ARM assembler mutex code, this won't -#
>>> >> >> work with thumb compilation...
>>> >> >> -ARM_MUTEX = "--with-mutex=ARM/gcc-assembly"
>>> >> >> -MUTEX = ""
>>> >> >> -MUTEX_arm = "${ARM_MUTEX}"
>>> >> >> -MUTEX_armeb = "${ARM_MUTEX}"
>>> >> >> -EXTRA_OECONF += "${MUTEX} STRIP=true"
>>> >> >> +EXTRA_OECONF = "${DB5_CONFIG} --enable-shared --enable-cxx
>>> >> >> --with-sysroot
>>> >> >> STRIP=true"
>>> >> >>  EXTRA_OEMAKE += "LIBTOOL='./${HOST_SYS}-libtool'"
>>> >> >>
>>> >> >>  EXTRA_AUTORECONF += "--exclude=autoheader  -I
>>> >> >> ${S}/dist/aclocal -I${S}/dist/aclocal_java"
>>> >> >> --
>>> >> >> 2.17.1
>>> >> >>
>>> >> >> --
>>> >> >> _______________________________________________
>>> >> >> Openembedded-core mailing list
>>> >> >> Openembedded-core at lists.openembedded.org
>>> >> >> http://lists.openembedded.org/mailman/listinfo/openembedded-cor
>>> >> >> e
>>> >> >>
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > _______________________________________________
>>> >> > Openembedded-core mailing list
>>> >> > Openembedded-core at lists.openembedded.org
>>> >> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>> >> >
>