From: Serge Petrenko <sergepetrenko@tarantool.org> To: "Alexander V. Tikhonov" <avtikhon@tarantool.org> Cc: tml <tarantool-patches@dev.tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v1] test: add ARM at travis-ci Date: Wed, 15 Apr 2020 19:03:31 +0300 [thread overview] Message-ID: <853BAC98-35BB-4111-9A9D-E0BF17DA1A0E@tarantool.org> (raw) In-Reply-To: <57e4a60ed158cec91fa18ed020dd0896d2da00f7.1576824858.git.avtikhon@tarantool.org> Hi! Thanks for the patch! Let me add my 5 cents. > 20 дек. 2019 г., в 09:55, Alexander V. Tikhonov <avtikhon@tarantool.org> написал(а): > > Added ARM architecture for testing release and > debug Tarantool builds on Travis-ci. > > Close #4270 > --- > > Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4270-travis-ci-arm > Issue: https://github.com/tarantool/tarantool/issues/4270 > > .gitlab-ci.yml | 11 +++++++++++ > .travis.mk | 12 ++++++++++++ > src/lib/core/fiber.c | 17 ++++++++++++++++- > 3 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index 3af5a3c8a..1603b65e3 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -90,6 +90,17 @@ release_asan_clang8: > script: > - ${GITLAB_MAKE} test_asan_debian_no_deps > > +release_arm: > + stage: test > + tags: > + - test > + variables: > + DOCKER_IMAGE: "${CI_REGISTRY}/${CI_PROJECT_PATH}/testing/arm64v8-debian-buster:latest" > + DOCKER_OPTIONS: "--network=host" > + CMAKE_EXTRA_PARAMS: -DENABLE_BACKTRACE=OFF > + script: > + - ${GITLAB_MAKE} test_arm_debian_no_deps > + > osx_13_release: > <<: *release_only_definition > <<: *vbox_definition > diff --git a/.travis.mk b/.travis.mk > index 42969ff56..1427d5b2f 100644 > --- a/.travis.mk > +++ b/.travis.mk > @@ -24,6 +24,7 @@ asan: docker_test_asan_debian > docker_%: > mkdir -p ~/.cache/ccache > docker run \ > + ${DOCKER_OPTIONS} \ > --rm=true --tty=true \ > --volume "${PWD}:/tarantool" \ > --volume "${HOME}/.cache:/cache" \ > @@ -123,6 +124,17 @@ test_asan_debian_no_deps: build_asan_debian > > test_asan_debian: deps_debian deps_buster_clang_8 test_asan_debian_no_deps > > +####### > +# ARM # > +####### > + > +qemu_arm_debian: > + docker run --rm --privileged multiarch/qemu-user-static --reset -p yes > + > +build_arm_debian: qemu_arm_debian docker_build_debian > + > +test_arm_debian_no_deps: qemu_arm_debian docker_test_debian_no_deps > + > ####### > # OSX # > ####### > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c > index 00ae8cded..6ef3c22db 100644 > --- a/src/lib/core/fiber.c > +++ b/src/lib/core/fiber.c > @@ -43,7 +43,22 @@ > #include "trigger.h" > > #if ENABLE_FIBER_TOP > +#if !defined(__arm__) && !defined(__aarch64__) && !defined(__powerpc64__) && !defined(__PPC64__) && \ > + !defined(__powerpc__) > #include <x86intrin.h> /* __rdtscp() */ > +#elif defined(__aarch64__) > +static __inline__ unsigned long long __rdtscp(uint32_t *cpu_id) Please check src/lib/core/fiber.h It disables fiber.top for ARM architecture: ``` /* * Fiber top doesn't work on ARM processors at the moment, * because we haven't chosen an alternative to rdtsc. */ #ifdef __CC_ARM #define ENABLE_FIBER_TOP 0 #else #define ENABLE_FIBER_TOP 1 #endif ``` So, it looks like your patch not only adds ARM testing, but also closes a relevant ticket re fiber.top (https://github.com/tarantool/tarantool/issues/4573) [Implement fiber.top() for ARM]. Anyway, this is worth a separate patch, and IMO in this patch you should just disable fiber.top() test on ARM somehow. As I can see you tried to implement it on arm because otherwise the test would fail. > +{ > + // System timer of ARMv8 runs at a different frequency than the CPU's. > + // The frequency is fixed, typically in the range 1-50MHz. It can be > + // read at CNTFRQ special register. We assume the OS has set up > + // the virtual timer properly. > + if (!cpu_id) { cpu_id = 0; } > + int64_t virtual_timer_value; > + __asm__ volatile("mrs %0, cntvct_el0" : "=r"(virtual_timer_value)); > + return virtual_timer_value; > +} > +#endif > > static inline void > clock_stat_add_delta(struct clock_stat *stat, uint64_t clock_delta) > @@ -109,7 +124,7 @@ cpu_stat_reset(struct cpu_stat *stat) > static uint64_t > cpu_stat_on_csw(struct cpu_stat *stat) > { > - uint32_t cpu_id; > + uint32_t cpu_id = 0; > uint64_t delta, clock = __rdtscp(&cpu_id); > > if (cpu_id == stat->prev_cpu_id) { > — > 2.17.1 > -- Serge Petrenko sergepetrenko@tarantool.org
prev parent reply other threads:[~2020-04-15 16:03 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-20 6:55 Alexander V. Tikhonov 2020-04-15 15:45 ` Kirill Yukhin 2020-04-15 15:52 ` Kirill Yukhin 2020-04-15 16:03 ` Serge Petrenko [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=853BAC98-35BB-4111-9A9D-E0BF17DA1A0E@tarantool.org \ --to=sergepetrenko@tarantool.org \ --cc=avtikhon@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v1] test: add ARM at travis-ci' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox