On Fri, Jun 15, 2018 at 3:41 AM Herve Jourdain <[email protected]> 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. >
the least intrusive fix it to fix what we need and let the defaults be as it in this case. > Cheers, > Herve > > -----Original Message----- > From: Andre McCurdy [mailto:[email protected]] > Sent: vendredi 15 juin 2018 09:39 > To: Herve Jourdain <[email protected]> > Cc: Khem Raj <[email protected]>; Ovidiu Panait > <[email protected]>; Patches and discussions about the oe-core > layer <[email protected]> > 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 <[email protected]> > 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:[email protected]] > > Sent: vendredi 15 juin 2018 00:03 > > To: Khem Raj <[email protected]> > > Cc: Herve Jourdain <[email protected]>; Ovidiu Panait > > <[email protected]>; Patches and discussions about the > > oe-core layer <[email protected]> > > 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 <[email protected]> wrote: > >> On Thu, Jun 14, 2018 at 1:01 PM Andre McCurdy <[email protected]> wrote: > >>> On Thu, Jun 14, 2018 at 12:24 PM, Khem Raj <[email protected]> wrote: > >>> > On Thu, Jun 14, 2018 at 12:12 PM Andre McCurdy > >>> > <[email protected]> > >>> > wrote: > >>> >> > >>> >> On Thu, Jun 14, 2018 at 9:40 AM, Khem Raj <[email protected]> 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: [email protected] > >>> >> >> [mailto:[email protected]] On > >>> >> >> Behalf Of Ovidiu Panait > >>> >> >> Sent: jeudi 14 juin 2018 13:55 > >>> >> >> To: [email protected] > >>> >> >> 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 <[email protected]> > >>> >> >> Signed-off-by: Catalin Enache <[email protected]> > >>> >> >> Signed-off-by: Ovidiu Panait <[email protected]> > >>> >> >> --- > >>> >> >> 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 > >>> >> >> [email protected] > >>> >> >> http://lists.openembedded.org/mailman/listinfo/openembedded-co > >>> >> >> r > >>> >> >> e > >>> >> >> > >>> >> > > >>> >> > > >>> >> > > >>> >> > -- > >>> >> > _______________________________________________ > >>> >> > Openembedded-core mailing list > >>> >> > [email protected] > >>> >> > http://lists.openembedded.org/mailman/listinfo/openembedded-cor > >>> >> > e > >>> >> > > > > -- _______________________________________________ Openembedded-core mailing list [email protected] http://lists.openembedded.org/mailman/listinfo/openembedded-core
