On Thu, Jul 4, 2024 at 6:25 AM Sadineni, Harish <[email protected]> wrote: > > Hi Bruce, > > Thanks for reviewing the patch and your comments on the same. > > >> This has to be conditional, it is only required in certain configurations. > > Can you please let me know which condition would be better suited for this > configuration.
That's the thing .. While it is relatively rare, there are userspace dependencies for some kernel options, and of course it is more common to have kernel dependencies for userspace tools. So it is something that has been thought about and there are facilities in place to deal with it. But I'm stating the obvious, I just wanted to summarize that for the mailing list archive :) What we don't want is to enable dependencies "just in case", since although they are usually small, it goes against keeping things to be only pulled in when required. We also don't want fine grained distro or other mappings to individual kernel features. That creates a tight binding from userspace and the meta-data to the kernel, and often specific versions of the kernel. It becomes brittle and hard to maintain over time. The way that we coordinate userspace and kernel options is via the KERNEL_FEATURES variable. That allows us to bind distro / image / other options into a named kernel configuration fragment in the kernel-cache. It also allows us to group options and dependencies into categories. You haven't fully explained your issue / problem though. What is the exact issue ? That you are trying to run a specific ptest, it requires the BTF debug option, which in turn needs pahole ? As you can guess, enabling pahole and those options for all kernel builds, all the time .. doesn't really make sense. We may get to the point where we really do want to enable it for all kernels, and then it would be done in the kernel definitions in the kernel-cache .. but I'm not seeing enough evidence that we are at that point yet. The variable to add that userspace tool / dependency was intended to be the KERNEL_DEBUG variable (although the implementation is incomplete). The main issue may have been that you didn't know to set that variable, and the documentation around it could be better, but trying to do something that was really automatic, that grepped through .config files, magically adding dependencies, etc, is tempting, but like I was saying above, it doesn't scale and causes issues. So my simplest suggestion is: just fix the KERNEL_DEBUG enabling of pahole as a dependency and when KERNEL_DEBUG is set, add the proper KERNEL_FEATURE to the KERNEL_FEATURES variable. We could update the documentation to describe that when KERNEL_DEBUG is enabled, pahole and btf are among the things you get. The recipe that builds the test could also be updated to indicate that KERNEL_DEBUG should be set to run the test. It would also be possible to test on debug-btf being set in the KERNEL_FEATURES and use that to trigger the pahole dependency. i.e. use the KERNEL_FEATURES value as the trigger for the dependency, versus using KERNEL_DEBUG. Either way someone has to know to enable KERNEL_DEBUG or set the right value in KERNEL_FEATURES, and that's fine / expected. > > >> Fragments don't go in the meta data layers, this needs to be submitted > >> to the kernel-cache and enabled with a KERNEL_FEATURE. > > Will submit it to the kernel-cache as per your suggestion. > However, enabling the "CONFIG_DEBUG_INFO_BTF" will result in the following > build failure:- > --------------------------------------------------------------------------------- > | BTF: .tmp_vmlinux.btf: pahole (false) is not available > | Failed to generate BTF for vmlinux > | Try to disable CONFIG_DEBUG_INFO_BTF > --------------------------------------------------------------------------------- > > Can you let me know if the configuration changes has to be submitted in > kernel-cache after conditionally enabling the pahole in Kernel recipe? > We already have those options in the kernel-cache and a fragment for this purpose: % git grep CONFIG_DEBUG_INFO_BTF features/debug/debug-btf.cfg:CONFIG_DEBUG_INFO_BTF=y If it is missing an option, you could always add it to that one. Bruce Bruce > Thanks, > Harish > > ________________________________ > From: Bruce Ashfield <[email protected]> > Sent: Wednesday, July 3, 2024 6:33 PM > To: Sadineni, Harish <[email protected]> > Cc: [email protected] > <[email protected]>; MacLeod, Randy > <[email protected]>; Gowda, Naveen <[email protected]>; > Kokkonda, Sundeep <[email protected]>; Moodalappa, Shivaprasad > <[email protected]> > Subject: Re: [OE-core] [PATCH] linux: Modify kernel configuration to fix > runqlat issue > > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and > know the content is safe. > > On Wed, Jul 3, 2024 at 8:53 AM Sadineni, Harish via > lists.openembedded.org > <[email protected]> wrote: > > > > From: Harish Sadineni <[email protected]> > > > > Running /usr/share/bcc/tools/runqlat 1 5 gives following error:- > > > > libbpf: failed to find valid kernel BTF > > /virtual/main.c:92:15: error: no member named 'state' in 'struct > > task_struct' > > if (prev->state == TASK_RUNNING) { > > ~~~~ ^ > > 1 error generated. > > > > It is due to the modification of state to __state in latest kernels. > > > > The following commit addressed the issue which checks whether the struct > > "task_struct" has __state or state > > https://github.com/chenhengqi/bcc/commit/d3cf5dcecfaeb6d8d346e2228626a7dbe506ad38 > > The above patch requires enabling CONFIG_DEBUG_INFO_BTF in the kernel > > and requires pahole. > > > > Signed-off-by: Harish Sadineni <[email protected]> > > --- > > meta/recipes-kernel/linux/linux-yocto.inc | 3 ++- > > meta/recipes-kernel/linux/linux-yocto/cg2.cfg | 2 ++ > > meta/recipes-kernel/linux/linux-yocto/cg2.scc | 1 + > > meta/recipes-kernel/linux/linux-yocto_6.6.bb | 2 ++ > > 4 files changed, 7 insertions(+), 1 deletion(-) > > create mode 100644 meta/recipes-kernel/linux/linux-yocto/cg2.cfg > > create mode 100644 meta/recipes-kernel/linux/linux-yocto/cg2.scc > > > > diff --git a/meta/recipes-kernel/linux/linux-yocto.inc > > b/meta/recipes-kernel/linux/linux-yocto.inc > > index 0132fcffb3..d6b4794d32 100644 > > --- a/meta/recipes-kernel/linux/linux-yocto.inc > > +++ b/meta/recipes-kernel/linux/linux-yocto.inc > > @@ -65,11 +65,12 @@ KERNEL_DEBUG ?= "" > > DEPENDS += '${@bb.utils.contains_any("ARCH", [ "x86", "arm64", "powerpc" > > ], "elfutils-native", "", d)}' > > DEPENDS += "openssl-native util-linux-native" > > DEPENDS += "gmp-native libmpc-native" > > +DEPENDS += "pahole-native" > > This has to be conditional, it is only required in certain > configurations. You may need > another image or distro configuration option to control the > conditional depends, just > like we did with KERNEL_DEBUG (and see my comment below, I think we > still should). > > > > > # Some options depend on CONFIG_PAHOLE_VERSION, so need to make > > pahole-native available before do_kernel_configme > > do_kernel_configme[depends] += '${@bb.utils.contains("KERNEL_DEBUG", > > "True", "pahole-native:do_populate_sysroot", "", d)}' > > > > -EXTRA_OEMAKE += '${@bb.utils.contains("KERNEL_DEBUG", "True", "", > > "PAHOLE=false", d)}' > > You need to justify this change. It wasn't a mistake that we wanted to > force phole off > when debug wasn't enabled. > > > +EXTRA_OEMAKE += '${@bb.utils.contains("KERNEL_DEBUG", "True", "", "", d)}' > > > > do_devshell:prepend() { > > # setup native pkg-config variables (kconfig scripts call pkg-config > > directly, cannot generically be overriden to pkg-config-native) > > diff --git a/meta/recipes-kernel/linux/linux-yocto/cg2.cfg > > b/meta/recipes-kernel/linux/linux-yocto/cg2.cfg > > new file mode 100644 > > index 0000000000..7c60e87a1a > > --- /dev/null > > +++ b/meta/recipes-kernel/linux/linux-yocto/cg2.cfg > > @@ -0,0 +1,2 @@ > > +CONFIG_IKHEADERS=y > > +CONFIG_DEBUG_INFO_BTF=y > > diff --git a/meta/recipes-kernel/linux/linux-yocto/cg2.scc > > b/meta/recipes-kernel/linux/linux-yocto/cg2.scc > > new file mode 100644 > > index 0000000000..7047be85f7 > > --- /dev/null > > +++ b/meta/recipes-kernel/linux/linux-yocto/cg2.scc > > @@ -0,0 +1 @@ > > +kconf non-hardware cg2.cfg > > diff --git a/meta/recipes-kernel/linux/linux-yocto_6.6.bb > > b/meta/recipes-kernel/linux/linux-yocto_6.6.bb > > index 62c0f0ab36..cba61aa2d7 100644 > > --- a/meta/recipes-kernel/linux/linux-yocto_6.6.bb > > +++ b/meta/recipes-kernel/linux/linux-yocto_6.6.bb > > @@ -72,3 +72,5 @@ KERNEL_FEATURES:append:powerpc64le =" > > arch/powerpc/powerpc-debug.scc" > > > > INSANE_SKIP:kernel-vmlinux:qemuppc64 = "textrel" > > > > +SRC_URI += "file://cg2.scc" > > +KERNEL_FEATURES += "cg2.scc" > > Fragments don't go in the meta data layers, this needs to be submitted > to the kernel-cache and enabled with a KERNEL_FEATURE. > > Cheers, > > Bruce > > > -- > > 2.43.0 > > > > > > > > > > > -- > - Thou shalt not follow the NULL pointer, for chaos and madness await > thee at its end > - "Use the force Harry" - Gandalf, Star Trek II -- - Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end - "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#201576): https://lists.openembedded.org/g/openembedded-core/message/201576 Mute This Topic: https://lists.openembedded.org/mt/107018570/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
