Skip to content

pyrex.ini: add /tmp to default [run]bind#95

Open
rtollert wants to merge 1 commit intogarmin:masterfrom
rtollert:bind-tmp
Open

pyrex.ini: add /tmp to default [run]bind#95
rtollert wants to merge 1 commit intogarmin:masterfrom
rtollert:bind-tmp

Conversation

@rtollert
Copy link

@rtollert rtollert commented Sep 6, 2024

I observed anomalously high disk I/O on my root filesystem during an OpenEmbedded build which used pyrex. (In fact, no rootfs I/O should generally be happening on this machine during an OpenEmbedded build; both the sources and TMPDIR are on other disks, and /tmp is a tmpfs.) After some judicious debugging with fatrace(1) I noticed that virtually all of the I/O was happening against, e.g.,
/var/lib/docker/overlay2/dc702bbbb061fbbf13b9d06c36d8ccee398257070e6c0455ff209b656b2db2f3/diff/tmp/ccuwDoBf.o.

It appears that under pyrex, bitbake will use the /tmp in the container overlay, not the system's /tmp. I believe that this is a mistake: pyrex containers should always use /tmp directly through a bind mount. My reasoning is as follows.

  • On systems where /tmp is a tmpfs, using it will effect a major performance improvement, and potentially reduce root filesystem wear.
  • By using a tmpfs instead of the root filesystem, this change cannot introduce any OOM that wouldn't have happened anyway without pyrex.
  • The use of /tmp in recipes must already be robust in the presence of parallel builds; any collision between simultaneous builds under different containers represents a fault in the recipe, not in the bind-mounting.
  • No security problems can occur through bind-mounting /tmp that would not have happened anyway when building outside of pyrex.

I tested this change in my own OE build by editing args, not bind; I'm marking this as an RFC insofar as I have not literally tested this change yet.

@JoshuaWatt
Copy link
Collaborator

Ya, I think that makes sense. Please update with an non RFC PR

@JoshuaWatt JoshuaWatt changed the title [RFC] pyrex.ini: add /tmp to default [run]bind pyrex.ini: add /tmp to default [run]bind Dec 4, 2024
@JoshuaWatt
Copy link
Collaborator

I guess I can do that :)

@rtollert
Copy link
Author

Bump. Hmm, what's with this?

check Expected — Waiting for status to be reported

@JoshuaWatt
Copy link
Collaborator

I think if a PR waits too long to be approved for CI, it won't run CI even once it is approved.... pretty sure this is some github quirk. I think if you rebase your branch and push it again I can approve it and it will run, or maybe even amend the top commit to change the date and push again might work.

@rtollert
Copy link
Author

I think if a PR waits too long to be approved for CI, it won't run CI even once it is approved.... pretty sure this is some github quirk. I think if you rebase your branch and push it again I can approve it and it will run, or maybe even amend the top commit to change the date and push again might work.

okee, I just rebased and pushed.

@JoshuaWatt
Copy link
Collaborator

Ok, we had some trouble with our CI, which is now fixed. I did make #105 to try and merge your change without you needing to rebase again, but it appears that there is actually a problem with this change that breaks the CI

I observed anomalously high disk I/O on my root filesystem during an
OpenEmbedded build which used pyrex. (In fact, *no* rootfs I/O should generally
be happening on this machine during an OpenEmbedded build; both the sources and
TMPDIR are on other disks, and `/tmp` is a tmpfs.) After some judicious debugging
with fatrace(1) I noticed that virtually all of the I/O was happening against,
e.g.,
`/var/lib/docker/overlay2/dc702bbbb061fbbf13b9d06c36d8ccee398257070e6c0455ff209b656b2db2f3/diff/tmp/ccuwDoBf.o`.

It appears that under pyrex, bitbake will use the `/tmp` in the container
overlay, not the system's `/tmp`. I believe that this is a mistake: pyrex
containers should *always* use `/tmp` directly through a bind mount. My
reasoning is as follows.

- On systems where `/tmp` is a tmpfs, using it will effect a major performance
  improvement, and potentially reduce root filesystem wear.
- By using a tmpfs instead of the root filesystem, this change cannot introduce
  any OOM that wouldn't have happened anyway without pyrex.
- The use of `/tmp` in recipes must already be robust in the presence of
  parallel builds; any collision between simultaneous builds under different
  containers represents a fault in the recipe, not in the bind-mounting.
- No security problems can occur through bind-mounting `/tmp` that would not
  have happened anyway when building outside of pyrex.
@rtollert
Copy link
Author

Re-rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants