On Fri, Jun 15, 2018 at 3:41 AM, Herve Jourdain <herve.jourd...@neuf.fr> wrote: > Hi, > > Actually, I meant "works" in the sense of "does compile" - as opposed to > armv8 where it does not compile, which is why we're having this discussion in > the first place. > So I was merely suggesting to not modify previous oe behavior for the db > package for previous architectures, and just add the removal of 'swp' for > armv8, where it matters most. > > If we want to look at it in details, based on the "ARM Architecture Reference > Manual ARMv7-A and ARMv7-R edition": > 1. SWP is the way to go before ARMv6. > 2. SWP has been deprecated in ARMv6. > 3. SWP has been deprecated AND made optional in ARMv7ve. > 4. "The SWP and SWPB instructions rely on the properties of the system beyond > the processor to ensure that no stores from other observers can occur between > the load access and the store access, and this might not be implemented for > all regions of memory on some system implementations. In all cases, SWP and > SWPB do ensure that no stores from the processor that executed the SWP or > SWPB instruction can occur between the load access and the store access of > the SWP or SWPB." > > This latest part means that it may or may not work in SMP environments, it > depends on how the system is architecture around the cores - most likely how > the bus system is designed I believe. So it may actually be working fine if > the system/bus designer has taken that into account. > > This said, I believe that from point #3 above, it might make sense to disable > SWP for armv7ve as well, since being optional means that it might be compiled > correctly, but still fail at runtime, depending on the choices of the SoC > manufacturer. > So my recommendation would be to add: > MUTEX_armv7ve = "" > MUTEX_armv8 = "" > > To disable 'swp' by default only for those 2 archs, while keeping things like > they are for previous architectures.
Thanks for your long and detailed explanation! Adding together the time which has gone into this thread so far and the time which went into a similar thread in 2016, it perfectly illustrates the maintenance effort which goes into enabling this architecture specific micro optimisation. https://patchwork.openembedded.org/patch/133590/ > Cheers, > Herve > > -----Original Message----- > From: Andre McCurdy [mailto:armccu...@gmail.com] > Sent: vendredi 15 juin 2018 09:39 > To: Herve Jourdain <herve.jourd...@neuf.fr> > Cc: Khem Raj <raj.k...@gmail.com>; Ovidiu Panait > <ovidiu.pan...@windriver.com>; Patches and discussions about the oe-core > layer <openembedded-core@lists.openembedded.org> > Subject: Re: [OE-core] [PATCH 1/1] db: disable the ARM assembler mutex code > > On Fri, Jun 15, 2018 at 12:10 AM, Herve Jourdain <herve.jourd...@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:armccu...@gmail.com] >> Sent: vendredi 15 juin 2018 00:03 >> To: Khem Raj <raj.k...@gmail.com> >> Cc: Herve Jourdain <herve.jourd...@neuf.fr>; Ovidiu Panait >> <ovidiu.pan...@windriver.com>; Patches and discussions about the >> oe-core layer <openembedded-core@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.k...@gmail.com> wrote: >>> On Thu, Jun 14, 2018 at 1:01 PM Andre McCurdy <armccu...@gmail.com> wrote: >>>> On Thu, Jun 14, 2018 at 12:24 PM, Khem Raj <raj.k...@gmail.com> wrote: >>>> > On Thu, Jun 14, 2018 at 12:12 PM Andre McCurdy >>>> > <armccu...@gmail.com> >>>> > wrote: >>>> >> >>>> >> On Thu, Jun 14, 2018 at 9:40 AM, Khem Raj <raj.k...@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-boun...@lists.openembedded.org >>>> >> >> [mailto:openembedded-core-boun...@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.z...@windriver.com> >>>> >> >> Signed-off-by: Catalin Enache <catalin.ena...@windriver.com> >>>> >> >> Signed-off-by: Ovidiu Panait <ovidiu.pan...@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-co >>>> >> >> r >>>> >> >> e >>>> >> >> >>>> >> > >>>> >> > >>>> >> > >>>> >> > -- >>>> >> > _______________________________________________ >>>> >> > Openembedded-core mailing list >>>> >> > Openembedded-core@lists.openembedded.org >>>> >> > http://lists.openembedded.org/mailman/listinfo/openembedded-cor >>>> >> > e >>>> >> > >> > -- _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
