Stream: git-wasmtime

Topic: wasmtime / PR #3770 Patch qemu in CI to fix madvise seman...

Wasmtime GitHub notifications bot (Feb 06 2022 at 07:07):

cfallin requested alexcrichton for a review on PR #3770.

Wasmtime GitHub notifications bot (Feb 06 2022 at 07:07):

cfallin opened PR #3770 from qemu-madvise to main:

We currently skip some tests when running our qemu-based tests for
aarch64 and s390x. Qemu has broken madvise(MADV_DONTNEED) semantics --
specifically, it just ignores madvise() [1].

We could continue to whack-a-mole the tests whenever we create new
functionality that relies on madvise() semantics, but ideally we'd just
have emulation that properly emulates!

The earlier discussions on the qemu mailing list [2] had a proposed
patch for this, but (i) this patch doesn't seem to apply cleanly anymore
(it's 3.5 years old) and (ii) it's pretty complex due to the need to
handle qemu's ability to emulate differing page sizes on host and guest.

It turns out that we only really need this for CI when host and guest
have the same page size (4KiB), so we could just pass the madvise()s
through. I wouldn't expect such a patch to ever land upstream in qemu,
but it satisfies our needs I think. So this PR modifies our CI setup to
patch qemu before building it locally with a little one-off patch.

[1]
https://github.com/bytecodealliance/wasmtime/pull/2518#issuecomment-747280133

[2]
https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg05416.html

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 06 2022 at 07:11):

cfallin updated PR #3770 from qemu-madvise to main.

Wasmtime GitHub notifications bot (Feb 07 2022 at 15:04):

sunfishcode created PR review comment:

Unlike posix_fadvise, the libc API to madvise sets errno rather than returning an error code. If I understand the qemu code here, you can use get_errno to handle madvise's return value and errno.

Wasmtime GitHub notifications bot (Feb 07 2022 at 15:04):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (Feb 07 2022 at 15:04):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (Feb 07 2022 at 19:11):

cfallin updated PR #3770 from qemu-madvise to main.

Wasmtime GitHub notifications bot (Feb 07 2022 at 19:11):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Feb 07 2022 at 19:11):

cfallin created PR review comment:

Ah, yes, thanks for catching this!

Wasmtime GitHub notifications bot (Feb 07 2022 at 20:57):

sunfishcode submitted PR review.

Wasmtime GitHub notifications bot (Feb 07 2022 at 23:56):

cfallin merged PR #3770.

Last updated: Dec 23 2024 at 13:07 UTC