Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix several arm7l portability issues #1023

Merged
merged 13 commits into from Apr 3, 2017

Conversation

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Apr 2, 2017

This PR gets flux-core working on the 32-bit arm7l architecture used in the Raspberry PI and other embedded platforms, e.g.

  • fix 64 bit values used as printf-style arguments with ambiguous format specifier (e.g. %lu)
  • relax some arbitrary test timeouts
  • fix 64-bit pointer assumption in makecontext() usage
  • reduce VM footprint of test that tried to malloc 20gb of memory

There are a few unrelated minor bug fixes here as well:

  • fix NULL deref in content-cache error path
  • fix missing newlines in log output
  • get working with the latest zeromq (4.2.1)

garlick added some commits Feb 8, 2017

libutil/coproc: handle 32-bit pointers on arm7l
Construct makecontext() arugments properly on systems
like arm7l that have 32 bit pointers.
cmd/flux-ping: use portable size_t format specifier
Use %zu format specifier to refer to a size_t argument
to avoid compilation errors on arm7l.
cmd/flux-jstat: use portable int64_t format specifer
Use %PRIi64 not %ld to refer to int64_t argument to avoid
compilation errors on arm7l.
test/coproc: avoid excessive VM usage
Problem: running 10,000 coprocs causes coproc_create()
uses 10000*2mb = 20gb of VM, resulting in ENOMEM on
a system with 32-bit pointers.

The requested amount of memory cannot be represented in
32-bit pointers.  In addition, the default vm.overcommit_memory
setting (0: heuristic overcommit handling) may reject an
allocation greatly in excess of physical memory.
Run 500 instead of 10K coprocesses, which is still
a reasonable test.

In addition, clean up the test so that it doesn't produce
a series of cascading segfaults and obscure failures when
this happens.  Fail the first test and continue on with
however many coprocs were able to be allocated.
test/broker: fix signed integer issue in attr test
Problem: broker attr test fails on arm7l architecture.

Change strtol() to strtoul() when converting UINT_MAX - 1.
lua-affinity: handle 32-bit lua_Integer typedef
Backport of grondo/lua-affinity#2 to copy of lua-affinity in
flux-core.

Fixes #975
cmd/flux-ping: handle 32-bit tv_sec/tv_usec
Problem: flux-ping segfaults in flux_rpcf_multi() on arm7l,
which has 32-bit struct timeval members.

The "I" format specifier refers to a 64-bit argument in
json_vpack_ex() and passing 32 bit arguments triggers a segfault.
The segfault is avoided in flux-ping by casting tv_sec and tv_usec
arguments to uint64_t.
test/ping: increase timeouts on two tests
Problem: 5s timeouts are too tight on tests 1 and 6 on
are too small for some systems.

Actual runtimes on raspberry pi were 5.7s and 8.4s,
respectively.  Increase timeouts to 10s and 20s.
modules/wreck: fix inconsitent \n use in wlog_fatal
Problem: wlog_fatal() sends log messages to flux_log() or
vfprintf (stderr, ...) depending on context.  The former
takes messages without newlines; the latter requires a newline.

Append a newline character to the stderr stream when that
mode is selected.  Drop trailing newlines from a couple
of wlog_fatal() calls to be consistent with the majority
of other calls.
libutil/log: fix up zmq_strerror() message
Problem: zeromq-4.2.1 reports EHOSTUNREACH as "Host unreachable",
but "No route to host" is canonical on Linux and we have some
tests that depend on it, so remap here.
libutil/flog: fix up zmq_strerror() message
Problem: zeromq-4.2.1 reports EHOSTUNREACH as "Host unreachable",
but "No route to host" is canonical on Linux and we have some
tests that depend on it, so remap here.
modules/wreck: fix printf-style type mismatch
Problem: on arm7l architecture, wrexecd behaves erratically.

Add printf-style format attributes to internal varargs debugging
functions to enable type checking, then fix a couple places where
the format specifier (%lu) implied 32-bit on this architecture,
but the argument was 64 bit.  Also fix non-const format specifier
warnings.
broker/content-cache: fix error path NULL deref
Due to a typo in which variables were interchanged, an error
handling code path in cache_store_continuation() could trigger
a SEGFAULT on arm7l test system.

Fix the typo.

@garlick garlick force-pushed the garlick:arm7l branch from 0928c7b to 62fdb65 Apr 2, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Apr 2, 2017

Coverage Status

Coverage increased (+0.008%) to 78.194% when pulling 62fdb65 on garlick:arm7l into 56b4c56 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Apr 2, 2017

Codecov Report

Merging #1023 into master will decrease coverage by 0.14%.
The diff coverage is 78.94%.

@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
- Coverage   78.01%   77.87%   -0.15%     
==========================================
  Files         130      150      +20     
  Lines       23912    25491    +1579     
==========================================
+ Hits        18655    19851    +1196     
- Misses       5257     5640     +383
Impacted Files Coverage Δ
src/bindings/lua/lua-affinity/lua-affinity.c 97.5% <ø> (ø) ⬆️
src/common/libutil/log.c 86.53% <100%> (ø)
src/common/libutil/coproc.c 87.87% <100%> (+0.18%) ⬆️
src/broker/content-cache.c 73.03% <100%> (ø) ⬆️
src/common/libflux/flog.c 95.32% <100%> (+0.04%) ⬆️
src/cmd/flux-jstat.c 85.32% <100%> (ø) ⬆️
src/cmd/flux-ping.c 87.4% <100%> (+0.18%) ⬆️
src/modules/wreck/wrexecd.c 75.66% <55.55%> (-0.07%) ⬇️
src/cmd/builtin/nodeset.c 5% <0%> (-72.35%) ⬇️
src/common/libcompat/info.c 0% <0%> (-67.57%) ⬇️
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56b4c56...62fdb65. Read the comment docs.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Apr 2, 2017

Coverage Status

Coverage decreased (-0.0005%) to 78.186% when pulling 62fdb65 on garlick:arm7l into 56b4c56 on flux-framework:master.

@grondo grondo merged commit a028c54 into flux-framework:master Apr 3, 2017

4 checks passed

codecov/patch 78.94% of diff hit (target 78.01%)
Details
codecov/project Absolute coverage decreased by -0.14% but relative coverage increased by +0.93% compared to 56b4c56
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.0005%) to 78.186%
Details

@garlick garlick deleted the garlick:arm7l branch Apr 4, 2017

@grondo grondo referenced this pull request Aug 23, 2017

Closed

0.8.0 Release #1160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.