Skip to content

core: add systemd-executor binary - #27890

Merged
bluca merged 20 commits into
systemd:mainfrom
bluca:executor
Oct 13, 2023
Merged

bluca merged 20 commits into
systemd:mainfrom
bluca:executor

Conversation

@bluca

@bluca bluca commented Jun 1, 2023 •

Copy link
Copy Markdown
Member

Currently we spawn services by forking a child process, doing a bunch
of work, and then exec'ing the service executable.

There are some advantages to this approach:

  • quick: we immediately have access to all the enourmous amount of
    state simply by virtue of sharing the memory with the parent
  • easy to refactor and add features
  • part of the same binary, will never be out of sync

There are however significant drawbacks:

  • doing work after fork and before exec is against glibc's supported
    case for several APIs we call
  • copy-on-write trap: anytime any memory is touched in either parent
    or child, a copy of that page will be triggered
  • memory footprint of the child process will be memory footprint of
    PID1, but using the cgroup memory limits of the unit

The last issue is especially problematic on resource constrained
systems where hard memory caps are enforced and swap is not allowed.
As soon as PID1 is under load, with no page out due to no swap, and a
service with a low MemoryMax= tries to start, hilarity ensues.

Add a new systemd-executor binary, that is able to receive all the
required state via memfd, deserialize it, prepare the appropriate
data structures and call exec_child.

Use posix_spawn which uses CLONE_VM + CLONE_VFORK, to ensure there is
no copy-on-write (same address space will be used, and parent process
will be frozen, until exec).
The sd-executor binary is pinned by FD on startup, so that we can
guarantee there will be no incompatibilities during upgrades.

@bluca

This comment was marked as resolved.

@mrc0mmand

This comment was marked as resolved.

@bluca

This comment was marked as resolved.

@mrc0mmand

This comment was marked as resolved.

@bluca

This comment was marked as resolved.

mrc0mmand added a commit to systemd/systemd-centos-ci that referenced this pull request Jun 2, 2023
Temporary workaround for systemd/systemd#27890 until that change lands
and dracut/mkinitcpio is updated.
@mrc0mmand

This comment was marked as resolved.

continue;
}

p->fds[i] = fd;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use deserialize_fd()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean deserialize_fd_from_array/set? It is already using those

}

p->idle_pipe[i] = fd;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deserialize_fd() (here and everywhere)

Comment thread src/core/execute-serialize.c Outdated
Comment thread src/core/execute-serialize.c Outdated
Comment thread src/shared/serialize.c Outdated
return log_debug_errno(fd, "Failed to parse FD out of value: %s", value);

if (!fdset_contains(fds, fd))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "FD %d not in fdset.", fd);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is redundant, fdset_remove() cecks for that too.

also, can we please get #29481 merged, it adds a deserializer for this, and moves everything over.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's dropped

Comment thread src/shared/fdset.c Outdated
Comment thread src/core/executor.c Outdated
Comment thread src/core/executor.c Outdated
Comment thread src/core/executor.c Outdated
Comment thread src/core/executor.c Outdated
bluca added 7 commits October 12, 2023 14:56
Currently we spawn services by forking a child process, doing a bunch
of work, and then exec'ing the service executable.

There are some advantages to this approach:

- quick: we immediately have access to all the enourmous amount of
  state simply by virtue of sharing the memory with the parent
- easy to refactor and add features
- part of the same binary, will never be out of sync

There are however significant drawbacks:

- doing work after fork and before exec is against glibc's supported
  case for several APIs we call
- copy-on-write trap: anytime any memory is touched in either parent
  or child, a copy of that page will be triggered
- memory footprint of the child process will be memory footprint of
  PID1, but using the cgroup memory limits of the unit

The last issue is especially problematic on resource constrained
systems where hard memory caps are enforced and swap is not allowed.
As soon as PID1 is under load, with no page out due to no swap, and a
service with a low MemoryMax= tries to start, hilarity ensues.

Add a new systemd-executor binary, that is able to receive all the
required state via memfd, deserialize it, prepare the appropriate
data structures and call exec_child.

Use posix_spawn which uses CLONE_VM + CLONE_VFORK, to ensure there is
no copy-on-write (same address space will be used, and parent process
will be frozen, until exec).
The sd-executor binary is pinned by FD on startup, so that we can
guarantee there will be no incompatibilities during upgrades.
No functional changes, only moving code that is only needed in
exec_invoke, and adding new dependencies for seccomp/selinux/apparmor/pam
in meson for the sd-executor binary.
@bluca

bluca commented Oct 31, 2023

Copy link
Copy Markdown
Member Author

DId some measurements before/after this PR, building a Fedora 39 image and checking with systemd-analyze the time to boot, and can't see a measurable difference:

f39 buildtype=release kvm preset-all enable * (disable dhcpd dhcpd6 systemd-journal-upload systemd-time-wait-sync as they fail) 157 loaded active units

main
Startup finished in 1.103s (kernel) + 2.849s (initrd) + 3.872s (userspace) = 7.825s
Startup finished in 1.172s (kernel) + 2.471s (initrd) + 3.284s (userspace) = 6.929s
Startup finished in 1.081s (kernel) + 2.499s (initrd) + 3.306s (userspace) = 6.888s
Startup finished in 1.289s (kernel) + 2.501s (initrd) + 3.461s (userspace) = 7.253s
Startup finished in 1.085s (kernel) + 2.662s (initrd) + 3.535s (userspace) = 7.283s

commit 0f1cb04f9ad7bf49d815787b3d194a36fa960f9f (pre sd-executor)
Startup finished in 1.881s (kernel) + 3.945s (initrd) + 4.110s (userspace) = 9.937s
Startup finished in 1.088s (kernel) + 2.419s (initrd) + 3.438s (userspace) = 6.946s
Startup finished in 1.167s (kernel) + 2.890s (initrd) + 4.214s (userspace) = 8.272s
Startup finished in 1.073s (kernel) + 2.341s (initrd) + 3.480s (userspace) = 6.895s
Startup finished in 1.074s (kernel) + 2.342s (initrd) + 3.467s (userspace) = 6.885s

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

Development

Successfully merging this pull request may close these issues.

(sd-pam) process is a CoW trap. Garbage data puts unecessary pressure on virtual memory.

8 participants