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

initscripts-nilrt: add LV RT cgroup version detection to init scripts #612

Conversation

gratian
Copy link
Contributor

@gratian gratian commented Aug 30, 2023

The cgroup-v1 init scripts, currently used by LabVIEW RT, interfere with other uses of cgroups (ex. containers, systemd) and are run at boot regardless if LV RT is installed or not.

Selectively run the 'nicreatecpuacctgroups' and 'nicreatecpusets' init scripts only if cgroup-v1 is in use by LabVIEW RT. Currently cgroup-v1 is considered "in use" if LabVIEW RT is installed. This will change at a future date when LV RT adds cgroup-v2 support.

Additionally the new '/etc/default/lvrt-cgroup' configuration script can be used to force set the LVRT_CGROUP_VERSION to 0 and leave cgroups under OS control for testing.

Please note that the technically better alternative to make LabVIEW RT depend and directly install these scripts (instead of including them in the base image) was rejected due to the high cost of patching previous LabVIEW releases.

@gratian gratian requested review from amstewart and a team August 30, 2023 19:30
@gratian
Copy link
Contributor Author

gratian commented Aug 30, 2023

Testing

  • tested the scripts on a NI Linux RT target (cRIO-9042)
  • full OE image build/test
    • bitbake nilrt-base-system-image
    • installed on a cRIO-9042 and verified that cgroups are not mounted if LV RT is not installed
    • installed LV RT and verified cgroup v1 scripts run at boot and create the expected mount points/hierarchy

@gratian
Copy link
Contributor Author

gratian commented Aug 30, 2023

@rtollert not sure why I can't add you to the reviewers list but I would like your input on this PR.

@rtollert
Copy link
Contributor

rtollert commented Sep 6, 2023

All of my comments above are not showstoppers; ACK. Feel free to respond to the above comments either in this PR or a later one.

Copy link
Contributor

@rtollert rtollert left a comment

Choose a reason for hiding this comment

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

Uh, ^.

recipes-ni/initscripts-nilrt/files/lvrt-cgroup Outdated Show resolved Hide resolved
recipes-ni/initscripts-nilrt/files/lvrt-cgroup Outdated Show resolved Hide resolved
recipes-ni/initscripts-nilrt/files/lvrt-cgroup Outdated Show resolved Hide resolved
The cgroup-v1 init scripts, currently used by LabVIEW RT, interfere with
other uses of cgroups (ex. containers, systemd) and are run at boot
regardless if LV RT is installed or not.

Selectively run the 'nicreatecpuacctgroups' and 'nicreatecpusets' init
scripts only if cgroup-v1 is in use by LabVIEW RT. Currently cgroup-v1 is
considered "in use" if LabVIEW RT is installed. This will change at a
future date when LV RT adds cgroup-v2 support.

Additionally the new '/etc/default/lvrt-cgroup' configuration script can be
used to force set the LVRT_CGROUP_VERSION to 0 and leave cgroups under OS
control for testing.

Please note that the technically better alternative to make LabVIEW RT
depend and directly install these scripts (instead of including them in the
base image) was rejected due to the high cost of patching previous LabVIEW
releases.

Signed-off-by: Gratian Crisan <gratian.crisan@ni.com>
@gratian gratian force-pushed the dev/nilrt/master/kirkstone-cgroup-v1-detection branch from c831bb6 to 9335712 Compare September 14, 2023 21:34
@gratian
Copy link
Contributor Author

gratian commented Sep 14, 2023

Changes since V1

  • Removed 'local' variable designation from $lvrt_installed
  • Moved lvrt-cgroup.sh script logic to /usr/share/initscripts-nilrt/
  • Reworked the identify_lvrt_cgroup_version function per @rtollert suggestions

V2 Testing

  • bitbake initscripts-nilrt
  • Installed resulting .ipk on a cRIO-9043 with LVRT installed. Validated that the cgroup-v1 init scripts run correctly.
  • Installed resulting .ipk on a cRIO-9043 without LVRT installed. Validated that cgroup-v1 init scripts are skipped.
  • Installed resulting .ipk on a PXIe-8821 with LVRT 20.0.3 version installed (oldest selectable for this release). Validated that cgroup-v1 mount points are created correctly.
  • Validated that explicitly setting LVRT_CGROUP_VERSION=0 in /etc/default/lvrt-cgroup results in skipping the cgroup-v1 init scripts on boot.

@rtollert
Copy link
Contributor

👍🏻

@gratian gratian requested a review from amstewart September 19, 2023 20:10
@gratian
Copy link
Contributor Author

gratian commented Sep 19, 2023

@amstewart I've reset your vote as a reminder to review v2 and pull if it looks good to you.

Copy link
Contributor

@amstewart amstewart left a comment

Choose a reason for hiding this comment

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

LGTM

@amstewart amstewart merged commit 48f0fe9 into ni:nilrt/master/kirkstone Sep 19, 2023
@amstewart
Copy link
Contributor

Cherry-picked to nilrt/23.8/kirkstone.

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.

4 participants