[LibOS/regression] Add OpenMP simple for-loop test #749
Conversation
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)
a discussion (no related file):
Should this be in the app directory?
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @chiache)
a discussion (no related file):
Previously, chiache (Chia-Che Tsai) wrote…
Should this be in the app directory?
I find it useful to have a simple OpenMP test (indirect test of threading, raw syscalls, and whatever OpenMP uses underneath). But I would be also fine with moving it to apps. Let's see if anyone else has an opinion on this.
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @chiache and @dimakuv)
a discussion (no related file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
I find it useful to have a simple OpenMP test (indirect test of threading, raw syscalls, and whatever OpenMP uses underneath). But I would be also fine with moving it to apps. Let's see if anyone else has an opinion on this.
For a simple test, I'm ok with it being in regression.
LibOS/shim/test/regression/00_openmp.py, line 4 at r1 (raw file):
from regression import Regression loader = sys.argv[1]
Does this actually work on Linux, or just issue host system calls it shouldn't?
If this only goes through graphene on SGX, I might add something to the regression script to only run on sgx.
Perhaps better to hoist this check for sgx up to the makefile?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @donporter)
LibOS/shim/test/regression/00_openmp.py, line 4 at r1 (raw file):
Previously, donporter (Don Porter) wrote…
Does this actually work on Linux, or just issue host system calls it shouldn't?
If this only goes through graphene on SGX, I might add something to the regression script to only run on sgx.
Perhaps better to hoist this check for sgx up to the makefile?
Done. It works on Linux by issuing a host syscall (i.e., bypassing LibOS). Since we don't install a seccomp, this raw syscall works. I agree that this the scope of this test is Linux-SGX, so I added this limitation.
It turned out to be hard/confusing to add this check to the Makefile so I put it in the Python script.
|
Retest Jenkins please |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)
There was a problem hiding this comment.
Reviewed 4 of 5 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dimakuv)
Jenkinsfiles/ubuntu-16.04.dockerfile, line 24 at r2 (raw file):
texinfo \ wget \ libomp-dev \
Please remove the last \
LibOS/shim/test/regression/Makefile, line 46 at r2 (raw file):
openmp: %: %.c @echo [ $@ ]
This collides with changes from #732, whichever PR will get merged later will need to be fixed up.
LibOS/shim/test/regression/openmp.manifest.template, line 21 at r2 (raw file):
net.rules.1 = 127.0.0.1:8000:0.0.0.0:0-65535 # allow to connect to port 8000 net.rules.2 = 0.0.0.0:0-65535:127.0.0.1:8000
Leftovers from some other manifest?
There was a problem hiding this comment.
Reviewable status: 3 of 5 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @donporter and @mkow)
Jenkinsfiles/ubuntu-16.04.dockerfile, line 24 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
Please remove the last
\
Done.
LibOS/shim/test/regression/Makefile, line 46 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
This collides with changes from #732, whichever PR will get merged later will need to be fixed up.
Done.
LibOS/shim/test/regression/openmp.manifest.template, line 21 at r2 (raw file):
Previously, mkow (Michał Kowalczyk) wrote…
Leftovers from some other manifest?
Done.
There was a problem hiding this comment.
Reviewable status: 4 of 5 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @donporter and @mkow)
Jenkinsfiles/ubuntu-16.04.dockerfile, line 24 at r2 (raw file):
Previously, dimakuv (Dmitrii Kuvaiskii) wrote…
Done.
Actually, removing \ was wrong (Jenkins builds failed). The issue is that the next line && groupadd ... is actually a continuation of this huge command. So \ was important.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @mkow)
|
Retest Jenkins-SGX please |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:complete! all files reviewed, all discussions resolved
|
Retest Jenkins-SGX please |
Previously, OpenMP apps failed to run under Graphene-SGX because it did not support raw system calls (used inside of OpenMP lib, in particular the futex() syscall). Since recently, Graphene-SGX has support for raw syscall execution. This commit adds a test showing that OpenMP works.
mkow
force-pushed
the
dimakuv/openmp-test
branch
from
6fb4c59
to
84ec50a
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r4.
Reviewable status:
|
Retest Jenkins-SGX please (trying to hit a bug/timeout on OpenMP test) |
1 similar comment
|
Retest Jenkins-SGX please (trying to hit a bug/timeout on OpenMP test) |
dimakuv
force-pushed
the
dimakuv/openmp-test
branch
from
c756416
to
84ec50a
Compare
|
Retest Jenkins-Debug please |
There was a problem hiding this comment.
Reviewable status:
Affected components
Description of the changes
Previously, OpenMP apps failed to run under Graphene-SGX because it did
not support raw system calls (used inside of OpenMP lib, in particular
the futex() syscall). Since recently, Graphene-SGX has support for raw
syscall execution. This commit adds a test showing that OpenMP works.
Fixes #567.
How to test this PR?
This PR adds LibOS regression test
openmp.This change is