PREEMPT_RT support for EVE #3254

rouming · 2023-05-31T16:14:31Z

This PR introduces the PREEMPT_RT support for the main EVE kernel (pkg/kernel) for the x86_64 architecture by

Bumping up the minor version of the main kernel to the 5.1.131 to be aligned with PREEMPT_RT patches.
Adding the official patch-5.10.131-rt72.patch.gz set of patches.
Adding the patch which does not export the migrate_enable() and migrate_disable() kernel calls as GPL. OpenZFS is not GPL but uses those calls. Before the PREEMPT_RT patches those two calls were declared as "static inline" in the header, thus no export was needed.
Once the 'make pkg/kernel PREEMPT_RT=1' is specified the PREEMPT_RT kernel flavour is built in the 'eve-rt-kernel' docker image.
To build a live image with the PREEMPT_RT kernel call as the following: 'make live PREEMPT_RT=1'. The same applies for the installer images, e.g.: 'make installer-iso PREEMPT_RT=1'.

The PR is quite conservative and introduces a new kernel flavor which can be enabled on demand, so that by default other functionality is not touched.

This PR targets only amd64, since PREEMPT_RT scheduler can't be enabled on arm64 along with the KVM support (what a bummer).

There are a few changes in Dockerfile (recent security improvements) and kernel-build-yml.sh script, so @shjala please take a look.

rene · 2023-06-01T08:15:20Z

pkg/kernel/kernel-config/kernel_config-5.10.x-preempt-rt-x86_64.patch

+# CONFIG_SLUB_DEBUG is not set
+# CONFIG_SLUB_MEMCG_SYSFS_ON is not set
+ # CONFIG_COMPAT_BRK is not set
+-CONFIG_SLAB=y


I didn't notice we were using SLAB before.... but I guess you are changing to SLUB only for improvements, right?

This is the default, comes with the preemt_rt. I left untouched.

rene · 2023-06-01T08:18:53Z

pkg/kernel/kernel-config/kernel_config-5.10.x-preempt-rt-x86_64.patch

+ # end of Scheduler Debugging
+
+ # CONFIG_DEBUG_TIMEKEEPING is not set
+CONFIG_DEBUG_PREEMPT=y


Enable DEBUG here will increase overhead and affect the performance, I don't think we should enabled by default...

Left as is intentionally. Want to catch any complains from kernel if preempted in unsafe way. Want to see the whole virtualization stack in action. Will disable later.

rene · 2023-06-01T08:19:48Z

...el/patches-preempt-rt-5.10.x/0001-sched-core-make-migrate_disable-and-migrate_enable-a.patch

+ 	preempt_lazy_disable();
+ 	preempt_enable();
+ }
+-EXPORT_SYMBOL_GPL(migrate_disable);


Why this change was not needed before?

Before those methods were declared as static inline in the header, so no export beeded

eriknordmark

LGTM.
Should we do a separate PR which applies this to new-kernel as well?

pkg/kernel/Dockerfile

Makefile

rouming · 2023-06-01T13:16:40Z

Should we do a separate PR which applies this to new-kernel as well?

@eriknordmark Frankly I don't want to bother with a new kernel now, because it is used for risc only and the future of the whole new-kernel is not clear: do we drop it and switch the main kernel to a 5.15?

Update: or better to invest time and unify both Dockerfile and do the same trick with building a 5.15 on demand by 'KERNEL=5.15' make argument?

eriknordmark · 2023-06-01T13:32:39Z

Should we do a separate PR which applies this to new-kernel as well?

@eriknordmark Frankly I don't want to bother with a new kernel now, because it is used for risc only and the future of the whole new-kernel is not clear: do we drop it and switch the main kernel to a 5.15?

Over the next few months we should update the kernel. But if we keep them in synch that can be done by "rm -rf kernel; mv new-kernel kernel; mkdir new-kernel; // fill new-kernel with next kernel LTS"

Don't know if it is worth the effort to keep them in synch for that reason.
@rene any thoughts on this?

Update: or better to invest time and unify both Dockerfile and do the same trick with building a 5.15 on demand by 'KERNEL=5.15' make argument?

Probably not worth while if we are going to externalize the kernel build from the repo.

rouming · 2023-06-01T13:39:37Z

Probably not worth while if we are going to externalize the kernel build from the repo.

This does not eliminate the dockerfile and all the efforts to keep both in sync, we just skip the step of downloading the kernel sources and applying patches. Other than that stays the same.

tools/kernel-build-yml.sh

…0.131 In the next patches I introduce the preempt-rt patches, which can be applied without conflicts on the nearest stable 131 version. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

Parlez-vous francais? Ich auch. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

uncleDecart · 2023-06-02T10:54:14Z

Makefile

 kernel-yml:
-	$(QUIET)if [ ! -f "pkg/kernel/build.yml" ] || [ ! -f "pkg/new-kernel/build.yml" ]; then tools/kernel-build-yml.sh; fi
-
+	$(QUIET)$(KERNEL_BUILD_YML_TOOL)


Do I get it right, that now kernel-yml will always be quiet?

It seems nothing has been changed, QUIET was always there.

I guess I like python more than bash :D But nevertheless, you are removing check if pkg/kernel/build.yml and pkg/new-kernel/build.yml files exist and instead we always put tools/kernel-build-yml.sh with PREEMT_RT variable. Why do we skip it now?

The commit message should tell the whole story: e66a85a
What's probably is missing in the description is the following line: "We need to generate the build.yml each time to reflect the changes in the PREEMPT_RT_SUPPORT build argument"

uncleDecart · 2023-06-02T10:57:17Z

@rouming you say this PR targets only amd64, do we need to include some checks that PREEMT_RT will not build for arm or did I miss them?

rouming · 2023-06-02T11:10:36Z

@rouming you say this PR targets only amd64, do we need to include some checks that PREEMT_RT will not build for arm or did I miss them?

The config for other arches is missing, so no worries, you get an error :)

rouming · 2023-06-02T11:24:59Z

Difference to the previous version:

s/PREEMPT_RT_SUPPORT/PREEMPT_RT_SUPPORT_ARG/ in the Makefile

Tidy up how we call the kernel-build-yml.sh. Also don't check the existence of generated files and re-generate them unconditionally to reflect possible changes of the PREEMPT_RT_SUPPORT build argument. Re-generated build.yml files should not differ in other build arguments, which are passed for making reproducible kernel builds. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

…RT kernel Generate build.yml with the "eve-rt-kernel" image and PREEMPT_RT_SUPPORT build args set to true. In the future patches the Dockerfile will be modified to support the PREEMPT_RT flavour of the kernel. To build the "eve-rt-kernel" the following has to be called: make pkg/kernel PREEMPT_RT=1 Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

No functional change, just tidy the dockerfile up and make the procedure of keys copy before actual patches applying and building. This is needed for the next patches, which introduce the PREEMPT_RT patching. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

This patch introduces the PREEMPT_RT support for the main EVE kernel for the x86_64 architecture by: 1. Adding the official `patch-5.10.131-rt72.patch.gz` set of patches 2. Adding the patch which does not export the migrate_enable() and migrate_disable() kernel calls as GPL. OpenZFS is not GPL but uses those calls. 3. Once the 'make pkg/kernel PREEMPT_RT=1' is specified the PREEMPT_RT kernel flavour is built in the 'eve-rt-kernel' docker image. 4. To build a live image with the PREEMPT_RT kernel call as the following: 'make live PREEMPT_RT=1'. The same applies for the installer images, e.g.: 'make installer-iso PREEMPT_RT=1' Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

rouming · 2023-06-02T11:37:20Z

Difference to the previous version:

Just commit message tweaks

rouming · 2023-06-02T15:08:14Z

I pay 5 euro for approval. Offer is valid today only.

rene · 2023-06-02T15:24:01Z

Should we do a separate PR which applies this to new-kernel as well?

@eriknordmark Frankly I don't want to bother with a new kernel now, because it is used for risc only and the future of the whole new-kernel is not clear: do we drop it and switch the main kernel to a 5.15?

Over the next few months we should update the kernel. But if we keep them in synch that can be done by "rm -rf kernel; mv new-kernel kernel; mkdir new-kernel; // fill new-kernel with next kernel LTS"

Don't know if it is worth the effort to keep them in synch for that reason. @rene any thoughts on this?

Update: or better to invest time and unify both Dockerfile and do the same trick with building a 5.15 on demand by 'KERNEL=5.15' make argument?

Probably not worth while if we are going to externalize the kernel build from the repo.

They are not in that sync anyways, I don't think is worth to introduce all these change in new-kernel right now....

christoph-zededa · 2023-06-02T15:28:48Z

...el/patches-preempt-rt-5.10.x/0001-sched-core-make-migrate_disable-and-migrate_enable-a.patch

+ 	preempt_enable();
+ }
+-EXPORT_SYMBOL_GPL(migrate_disable);
+EXPORT_SYMBOL(migrate_disable);


Are we sure that from a law/license point of view this is allowed?

Until we don't ship the product - why not?

Or even we ship the product

Might be worth to ask a lawyer ...

If this raises questions I can drop the compilation of the openzfs for preempt_rt.

@eriknordmark interesting question from Christoph

I'm trying to understand why in the RT case you need to export migrate_{enabled,disabled} to ZFS, but that is not needed in the non-RT case. Are there some RT patches to the zfs module?

Before the PREEMPT_RT patches the "migrate_{enabled,disbled}" calls were defined as "static inline" in the header file (defined and declared!), so basically ANY driver with ANY license can use these API calls.

With the PREEMPT_RT patches those two calls were moved and defined in a sched/core.c. Since this is a public API, the exporting should be done. Authors of the PREEMPT_RT exported those two calls as GPL (basically limiting to only GPL users), so any driver which announce itself as non-GPL is not able to be linked with those two calls (since they are GPL).

I removed GPL export suffix to let the openzfs use those calls. If we think, this is dangerous etc, then I will remove a patch and exclude openzfs from compilation for the PREEMPT_RT case.

But frankly, this is pure debug feature. Who cares?

mikem-zed

LGTM after the issue with docker tag is resolved

Just a line, which exposes the possibility to build PREEMPT_RT EVE for x86_64 only. make pkg/kernel PREEMPT_RT=1 make live PREEMPT_RT=1 Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

rouming · 2023-06-02T15:39:28Z

Difference to the previous version:

Docs tweaks

shjala · 2023-06-05T13:05:12Z

LGTM

eriknordmark

LGTM

rouming requested review from eriknordmark and rvs as code owners May 31, 2023 16:14

rouming force-pushed the preempt-rt-eve branch 3 times, most recently from 4bc287f to c87aa23 Compare June 1, 2023 07:20

rene reviewed Jun 1, 2023

View reviewed changes

rouming force-pushed the preempt-rt-eve branch 2 times, most recently from 0ae507e to e2ae120 Compare June 1, 2023 09:37

eriknordmark reviewed Jun 1, 2023

View reviewed changes

pkg/kernel/Dockerfile Show resolved Hide resolved

eriknordmark reviewed Jun 1, 2023

View reviewed changes

pkg/kernel/Dockerfile Show resolved Hide resolved

eriknordmark reviewed Jun 1, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

eriknordmark reviewed Jun 1, 2023

View reviewed changes

tools/kernel-build-yml.sh Show resolved Hide resolved

rouming added 3 commits June 2, 2023 12:52

pkg/kernel: bump up minor version of the current stable kernel to 5.1…

4257812

…0.131 In the next patches I introduce the preempt-rt patches, which can be applied without conflicts on the nearest stable 131 version. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

Makefile: s/QUITE/QUIET/

ac00793

Parlez-vous francais? Ich auch. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

Makefile: indentation tweaks

95f7dec

Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

uncleDecart reviewed Jun 2, 2023

View reviewed changes

rouming force-pushed the preempt-rt-eve branch from e2ae120 to 30c15fd Compare June 2, 2023 11:23

rouming added 4 commits June 2, 2023 13:29

rouming force-pushed the preempt-rt-eve branch from 30c15fd to f5a48da Compare June 2, 2023 11:37

christoph-zededa reviewed Jun 2, 2023

View reviewed changes

mikem-zed approved these changes Jun 2, 2023

View reviewed changes

docs/BUILD.md: describe the PREEMPT_RT=1 make build argument

cb399a2

Just a line, which exposes the possibility to build PREEMPT_RT EVE for x86_64 only. make pkg/kernel PREEMPT_RT=1 make live PREEMPT_RT=1 Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

rouming force-pushed the preempt-rt-eve branch from f5a48da to cb399a2 Compare June 2, 2023 15:37

eriknordmark approved these changes Jun 5, 2023

View reviewed changes

rouming merged commit a9777ef into lf-edge:master Jun 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PREEMPT_RT support for EVE #3254

PREEMPT_RT support for EVE #3254

rouming commented May 31, 2023 •

edited

Loading

rene Jun 1, 2023

rouming Jun 1, 2023

rene Jun 1, 2023

rouming Jun 1, 2023

rene Jun 1, 2023

rouming Jun 1, 2023

rouming Jun 1, 2023

eriknordmark left a comment

rouming commented Jun 1, 2023 •

edited

Loading

eriknordmark commented Jun 1, 2023

rouming commented Jun 1, 2023

uncleDecart Jun 2, 2023

rouming Jun 2, 2023

uncleDecart Jun 2, 2023

rouming Jun 2, 2023

uncleDecart commented Jun 2, 2023

rouming commented Jun 2, 2023

rouming commented Jun 2, 2023

rouming commented Jun 2, 2023

rouming commented Jun 2, 2023

rene commented Jun 2, 2023

christoph-zededa Jun 2, 2023

rouming Jun 2, 2023

rouming Jun 2, 2023

christoph-zededa Jun 2, 2023

rouming Jun 2, 2023

rouming Jun 2, 2023

eriknordmark Jun 6, 2023

rouming Jun 6, 2023 •

edited

Loading

mikem-zed left a comment

rouming commented Jun 2, 2023

shjala commented Jun 5, 2023

eriknordmark left a comment

PREEMPT_RT support for EVE #3254

PREEMPT_RT support for EVE #3254

Conversation

rouming commented May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eriknordmark left a comment

Choose a reason for hiding this comment

rouming commented Jun 1, 2023 • edited Loading

eriknordmark commented Jun 1, 2023

rouming commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uncleDecart commented Jun 2, 2023

rouming commented Jun 2, 2023

rouming commented Jun 2, 2023

rouming commented Jun 2, 2023

rouming commented Jun 2, 2023

rene commented Jun 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rouming Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

mikem-zed left a comment

Choose a reason for hiding this comment

rouming commented Jun 2, 2023

shjala commented Jun 5, 2023

eriknordmark left a comment

Choose a reason for hiding this comment

rouming commented May 31, 2023 •

edited

Loading

rouming commented Jun 1, 2023 •

edited

Loading

rouming Jun 6, 2023 •

edited

Loading