Re: [PATCH 1/1] db: disable the ARM assembler mutex code

[Andre McCurdy <armccurdy@gmail.com>]

On Thu, Jun 14, 2018 at 2:48 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Thu, Jun 14, 2018 at 1:01 PM Andre McCurdy <armccurdy@gmail.com> wrote:
>> On Thu, Jun 14, 2018 at 12:24 PM, Khem Raj <raj.khem@gmail.com> wrote:
>> > On Thu, Jun 14, 2018 at 12:12 PM Andre McCurdy <armccurdy@gmail.com>
>> > wrote:
>> >>
>> >> On Thu, Jun 14, 2018 at 9:40 AM, Khem Raj <raj.khem@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-two-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@lists.openembedded.org
>> >> >> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf
>> >> >> Of
>> >> >> Ovidiu Panait
>> >> >> Sent: jeudi 14 juin 2018 13:55
>> >> >> To: openembedded-core@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@windriver.com>
>> >> >> Signed-off-by: Catalin Enache <catalin.enache@windriver.com>
>> >> >> Signed-off-by: Ovidiu Panait <ovidiu.panait@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@lists.openembedded.org
>> >> >> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > _______________________________________________
>> >> > Openembedded-core mailing list
>> >> > Openembedded-core@lists.openembedded.org
>> >> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
>> >> >