Message ID | ebf96ea600050f00ed567e80505ae8f242633640.1666113393.git.andreyknvl@google.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v3,1/3] kasan: switch kunit tests to console tracepoints | expand |
On Tue, Oct 18, 2022 at 10:17 AM <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@google.com> > > Switch KUnit-compatible KASAN tests from using per-task KUnit resources > to console tracepoints. > > This allows for two things: > > 1. Migrating tests that trigger a KASAN report in the context of a task > other than current to KUnit framework. > This is implemented in the patches that follow. > > 2. Parsing and matching the contents of KASAN reports. > This is not yet implemented. > > Reviewed-by: Marco Elver <elver@google.com> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > --- > > Changed v2->v3: > - Rebased onto 6.1-rc1 > > Changes v1->v2: > - Remove kunit_kasan_status struct definition. > --- > lib/Kconfig.kasan | 2 +- > mm/kasan/kasan.h | 8 ---- > mm/kasan/kasan_test.c | 85 +++++++++++++++++++++++++++++++------------ > mm/kasan/report.c | 31 ---------------- > 4 files changed, 63 insertions(+), 63 deletions(-) > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > index ca09b1cf8ee9..ba5b27962c34 100644 > --- a/lib/Kconfig.kasan > +++ b/lib/Kconfig.kasan > @@ -181,7 +181,7 @@ config KASAN_VMALLOC > > config KASAN_KUNIT_TEST > tristate "KUnit-compatible tests of KASAN bug detection capabilities" if !KUNIT_ALL_TESTS > - depends on KASAN && KUNIT > + depends on KASAN && KUNIT && TRACEPOINTS My build script for a KASAN-enabled kernel does something like: make defconfig scripts/config -e CONFIG_KUNIT -e CONFIG_KASAN -e CONFIG_KASAN_HW_TAGS -e CONFIG_KASAN_KUNIT_TEST yes '' | make syncconfig and after this change, the unit tests are no longer built. Should this use "select TRACING" instead? Peter
On Tue, 14 Feb 2023 at 02:21, Peter Collingbourne <pcc@google.com> wrote: > > On Tue, Oct 18, 2022 at 10:17 AM <andrey.konovalov@linux.dev> wrote: > > > > From: Andrey Konovalov <andreyknvl@google.com> > > > > Switch KUnit-compatible KASAN tests from using per-task KUnit resources > > to console tracepoints. > > > > This allows for two things: > > > > 1. Migrating tests that trigger a KASAN report in the context of a task > > other than current to KUnit framework. > > This is implemented in the patches that follow. > > > > 2. Parsing and matching the contents of KASAN reports. > > This is not yet implemented. > > > > Reviewed-by: Marco Elver <elver@google.com> > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > > > --- > > > > Changed v2->v3: > > - Rebased onto 6.1-rc1 > > > > Changes v1->v2: > > - Remove kunit_kasan_status struct definition. > > --- > > lib/Kconfig.kasan | 2 +- > > mm/kasan/kasan.h | 8 ---- > > mm/kasan/kasan_test.c | 85 +++++++++++++++++++++++++++++++------------ > > mm/kasan/report.c | 31 ---------------- > > 4 files changed, 63 insertions(+), 63 deletions(-) > > > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > > index ca09b1cf8ee9..ba5b27962c34 100644 > > --- a/lib/Kconfig.kasan > > +++ b/lib/Kconfig.kasan > > @@ -181,7 +181,7 @@ config KASAN_VMALLOC > > > > config KASAN_KUNIT_TEST > > tristate "KUnit-compatible tests of KASAN bug detection capabilities" if !KUNIT_ALL_TESTS > > - depends on KASAN && KUNIT > > + depends on KASAN && KUNIT && TRACEPOINTS > > My build script for a KASAN-enabled kernel does something like: > > make defconfig > scripts/config -e CONFIG_KUNIT -e CONFIG_KASAN -e CONFIG_KASAN_HW_TAGS > -e CONFIG_KASAN_KUNIT_TEST > yes '' | make syncconfig > > and after this change, the unit tests are no longer built. Should this > use "select TRACING" instead? I think we shouldn't select TRACING, which should only be selected by tracers. You'd need CONFIG_FTRACE=y. Since FTRACE is rather big, we probably also shouldn't implicitly select it. Instead, at least when using kunit.py tool, we could add a mm/kasan/.kunitconfig like: CONFIG_KUNIT=y CONFIG_KASAN=y CONFIG_KASAN_KUNIT_TEST=y # Additional dependencies. CONFIG_FTRACE=y Which mirrors the KFENCE mm/kfence/.kunitconfig. But that doesn't help if you want to run it with something other than KUnit tool.
On Mon, Feb 13, 2023 at 10:08 PM Marco Elver <elver@google.com> wrote: > > On Tue, 14 Feb 2023 at 02:21, Peter Collingbourne <pcc@google.com> wrote: > > > > On Tue, Oct 18, 2022 at 10:17 AM <andrey.konovalov@linux.dev> wrote: > > > > > > From: Andrey Konovalov <andreyknvl@google.com> > > > > > > Switch KUnit-compatible KASAN tests from using per-task KUnit resources > > > to console tracepoints. > > > > > > This allows for two things: > > > > > > 1. Migrating tests that trigger a KASAN report in the context of a task > > > other than current to KUnit framework. > > > This is implemented in the patches that follow. > > > > > > 2. Parsing and matching the contents of KASAN reports. > > > This is not yet implemented. > > > > > > Reviewed-by: Marco Elver <elver@google.com> > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > > > > > --- > > > > > > Changed v2->v3: > > > - Rebased onto 6.1-rc1 > > > > > > Changes v1->v2: > > > - Remove kunit_kasan_status struct definition. > > > --- > > > lib/Kconfig.kasan | 2 +- > > > mm/kasan/kasan.h | 8 ---- > > > mm/kasan/kasan_test.c | 85 +++++++++++++++++++++++++++++++------------ > > > mm/kasan/report.c | 31 ---------------- > > > 4 files changed, 63 insertions(+), 63 deletions(-) > > > > > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > > > index ca09b1cf8ee9..ba5b27962c34 100644 > > > --- a/lib/Kconfig.kasan > > > +++ b/lib/Kconfig.kasan > > > @@ -181,7 +181,7 @@ config KASAN_VMALLOC > > > > > > config KASAN_KUNIT_TEST > > > tristate "KUnit-compatible tests of KASAN bug detection capabilities" if !KUNIT_ALL_TESTS > > > - depends on KASAN && KUNIT > > > + depends on KASAN && KUNIT && TRACEPOINTS > > > > My build script for a KASAN-enabled kernel does something like: > > > > make defconfig > > scripts/config -e CONFIG_KUNIT -e CONFIG_KASAN -e CONFIG_KASAN_HW_TAGS > > -e CONFIG_KASAN_KUNIT_TEST > > yes '' | make syncconfig > > > > and after this change, the unit tests are no longer built. Should this > > use "select TRACING" instead? > > I think we shouldn't select TRACING, which should only be selected by > tracers. You'd need CONFIG_FTRACE=y. Doesn't CONFIG_FTRACE=y mean "function tracing", i.e. function entry/exit tracing using compiler instrumentation? As far as I can tell, the KASAN tests do not make use of this feature. They only use the kernel tracepoint infrastructure to trace the "console" tracepoint defined in include/trace/events/printk.h, which is not associated with function entry/exit. I have yet to find any evidence that TRACING ought to only be selected by tracers. As far as I can tell, TRACING appears to be the minimal config required in order for it to be possible to trace pre-defined (i.e. defined with TRACE_EVENT) tracepoints, which is all that KASAN needs. (I also tried selecting TRACEPOINTS, but this led to a number of link failures.) If select TRACING is only used by tracers, it could just mean that only tracers are making use of this functionality inside the kernel. From that perspective the KASAN tests can themselves be considered a "tracer" (albeit a very specialized one). If I locally revert the change to lib/Kconfig.kasan and add the TRACING select, the KASAN tests pass when using my kernel build script, which suggests that TRACING is all that is needed. > Since FTRACE is rather big, we probably also shouldn't implicitly > select it. Instead, at least when using kunit.py tool, we could add a > mm/kasan/.kunitconfig like: > > CONFIG_KUNIT=y > CONFIG_KASAN=y > CONFIG_KASAN_KUNIT_TEST=y > # Additional dependencies. > CONFIG_FTRACE=y > > Which mirrors the KFENCE mm/kfence/.kunitconfig. But that doesn't help > if you want to run it with something other than KUnit tool. In any case, I'm not sure I'm in favor of adding yet another config that folks need to know to enable in order to avoid silently disabling the unit tests. Many developers will maintain their own scripts for kernel development if the existing ones do not meet their needs. It's possible that kunit.py will work out for me now (when I looked at it before, it was useless for me because it only supported UML, but it looks like it supports QEMU now), but there's no guarantee that it will, so I might stick with my scripts for a while. Peter
+Cc tracing maintainers On Wed, 15 Feb 2023 at 03:56, Peter Collingbourne <pcc@google.com> wrote: > > On Mon, Feb 13, 2023 at 10:08 PM Marco Elver <elver@google.com> wrote: > > > > On Tue, 14 Feb 2023 at 02:21, Peter Collingbourne <pcc@google.com> wrote: > > > > > > On Tue, Oct 18, 2022 at 10:17 AM <andrey.konovalov@linux.dev> wrote: > > > > > > > > From: Andrey Konovalov <andreyknvl@google.com> > > > > > > > > Switch KUnit-compatible KASAN tests from using per-task KUnit resources > > > > to console tracepoints. > > > > > > > > This allows for two things: > > > > > > > > 1. Migrating tests that trigger a KASAN report in the context of a task > > > > other than current to KUnit framework. > > > > This is implemented in the patches that follow. > > > > > > > > 2. Parsing and matching the contents of KASAN reports. > > > > This is not yet implemented. > > > > > > > > Reviewed-by: Marco Elver <elver@google.com> > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > > > > > > > --- > > > > > > > > Changed v2->v3: > > > > - Rebased onto 6.1-rc1 > > > > > > > > Changes v1->v2: > > > > - Remove kunit_kasan_status struct definition. > > > > --- > > > > lib/Kconfig.kasan | 2 +- > > > > mm/kasan/kasan.h | 8 ---- > > > > mm/kasan/kasan_test.c | 85 +++++++++++++++++++++++++++++++------------ > > > > mm/kasan/report.c | 31 ---------------- > > > > 4 files changed, 63 insertions(+), 63 deletions(-) > > > > > > > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > > > > index ca09b1cf8ee9..ba5b27962c34 100644 > > > > --- a/lib/Kconfig.kasan > > > > +++ b/lib/Kconfig.kasan > > > > @@ -181,7 +181,7 @@ config KASAN_VMALLOC > > > > > > > > config KASAN_KUNIT_TEST > > > > tristate "KUnit-compatible tests of KASAN bug detection capabilities" if !KUNIT_ALL_TESTS > > > > - depends on KASAN && KUNIT > > > > + depends on KASAN && KUNIT && TRACEPOINTS > > > > > > My build script for a KASAN-enabled kernel does something like: > > > > > > make defconfig > > > scripts/config -e CONFIG_KUNIT -e CONFIG_KASAN -e CONFIG_KASAN_HW_TAGS > > > -e CONFIG_KASAN_KUNIT_TEST > > > yes '' | make syncconfig > > > > > > and after this change, the unit tests are no longer built. Should this > > > use "select TRACING" instead? > > > > I think we shouldn't select TRACING, which should only be selected by > > tracers. You'd need CONFIG_FTRACE=y. > > Doesn't CONFIG_FTRACE=y mean "function tracing", i.e. function > entry/exit tracing using compiler instrumentation? As far as I can > tell, the KASAN tests do not make use of this feature. They only use > the kernel tracepoint infrastructure to trace the "console" tracepoint > defined in include/trace/events/printk.h, which is not associated with > function entry/exit. Yes, you are right, and it's something I've wondered how to do better as well. Let's try to consult tracing maintainers on what the right approach is. > I have yet to find any evidence that TRACING ought to only be selected > by tracers. As far as I can tell, TRACING appears to be the minimal > config required in order for it to be possible to trace pre-defined > (i.e. defined with TRACE_EVENT) tracepoints, which is all that KASAN > needs. (I also tried selecting TRACEPOINTS, but this led to a number > of link failures.) If select TRACING is only used by tracers, it could > just mean that only tracers are making use of this functionality > inside the kernel. From that perspective the KASAN tests can > themselves be considered a "tracer" (albeit a very specialized one). > > If I locally revert the change to lib/Kconfig.kasan and add the > TRACING select, the KASAN tests pass when using my kernel build > script, which suggests that TRACING is all that is needed. > > > Since FTRACE is rather big, we probably also shouldn't implicitly > > select it. Instead, at least when using kunit.py tool, we could add a > > mm/kasan/.kunitconfig like: > > > > CONFIG_KUNIT=y > > CONFIG_KASAN=y > > CONFIG_KASAN_KUNIT_TEST=y > > # Additional dependencies. > > CONFIG_FTRACE=y > > > > Which mirrors the KFENCE mm/kfence/.kunitconfig. But that doesn't help > > if you want to run it with something other than KUnit tool. > > In any case, I'm not sure I'm in favor of adding yet another config > that folks need to know to enable in order to avoid silently disabling > the unit tests. Many developers will maintain their own scripts for > kernel development if the existing ones do not meet their needs. It's > possible that kunit.py will work out for me now (when I looked at it > before, it was useless for me because it only supported UML, but it > looks like it supports QEMU now), but there's no guarantee that it > will, so I might stick with my scripts for a while. > > Peter
On Wed, 15 Feb 2023 09:57:40 +0100 Marco Elver <elver@google.com> wrote: > Yes, you are right, and it's something I've wondered how to do better > as well. Let's try to consult tracing maintainers on what the right > approach is. I have to go and revisit the config options for CONFIG_FTRACE and CONFIG_TRACING, as they were added when this all started (back in 2008), and the naming was rather all misnomers back then. "ftrace" is really for just the function tracing, but CONFIG_FTRACE really should just be for the function tracing infrastructure, and perhaps not even include trace events :-/ But at the time it was created, it was for all the "tracers" (this was added before trace events). -- Steve
On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 15 Feb 2023 09:57:40 +0100 > Marco Elver <elver@google.com> wrote: > > > Yes, you are right, and it's something I've wondered how to do better > > as well. Let's try to consult tracing maintainers on what the right > > approach is. > > I have to go and revisit the config options for CONFIG_FTRACE and > CONFIG_TRACING, as they were added when this all started (back in > 2008), and the naming was rather all misnomers back then. > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > really should just be for the function tracing infrastructure, and > perhaps not even include trace events :-/ But at the time it was > created, it was for all the "tracers" (this was added before trace > events). It would be great to see this cleaned up. I found this aspect of how tracing works rather confusing. So do you think it makes sense for the KASAN tests to "select TRACING" for now if the code depends on the trace event infrastructure? Peter
On Thu, Feb 23, 2023 at 10:45 PM Peter Collingbourne <pcc@google.com> wrote: > > On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Wed, 15 Feb 2023 09:57:40 +0100 > > Marco Elver <elver@google.com> wrote: > > > > > Yes, you are right, and it's something I've wondered how to do better > > > as well. Let's try to consult tracing maintainers on what the right > > > approach is. > > > > I have to go and revisit the config options for CONFIG_FTRACE and > > CONFIG_TRACING, as they were added when this all started (back in > > 2008), and the naming was rather all misnomers back then. > > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > > really should just be for the function tracing infrastructure, and > > perhaps not even include trace events :-/ But at the time it was > > created, it was for all the "tracers" (this was added before trace > > events). > > It would be great to see this cleaned up. I found this aspect of how > tracing works rather confusing. > > So do you think it makes sense for the KASAN tests to "select TRACING" > for now if the code depends on the trace event infrastructure? Any thoughts? It looks like someone else got tripped up by this: https://reviews.llvm.org/D144057 Peter
On Mon, May 1, 2023 at 3:02 PM Peter Collingbourne <pcc@google.com> wrote: > > On Thu, Feb 23, 2023 at 10:45 PM Peter Collingbourne <pcc@google.com> wrote: > > > > On Wed, Feb 15, 2023 at 11:33 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > On Wed, 15 Feb 2023 09:57:40 +0100 > > > Marco Elver <elver@google.com> wrote: > > > > > > > Yes, you are right, and it's something I've wondered how to do better > > > > as well. Let's try to consult tracing maintainers on what the right > > > > approach is. > > > > > > I have to go and revisit the config options for CONFIG_FTRACE and > > > CONFIG_TRACING, as they were added when this all started (back in > > > 2008), and the naming was rather all misnomers back then. > > > > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > > > really should just be for the function tracing infrastructure, and > > > perhaps not even include trace events :-/ But at the time it was > > > created, it was for all the "tracers" (this was added before trace > > > events). > > > > It would be great to see this cleaned up. I found this aspect of how > > tracing works rather confusing. > > > > So do you think it makes sense for the KASAN tests to "select TRACING" > > for now if the code depends on the trace event infrastructure? > > Any thoughts? It looks like someone else got tripped up by this: > https://reviews.llvm.org/D144057 https://reviews.llvm.org/D144057#4311029 Peter, please triple check.
On Mon, 1 May 2023 15:02:37 -0700 Peter Collingbourne <pcc@google.com> wrote: > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > > > really should just be for the function tracing infrastructure, and > > > perhaps not even include trace events :-/ But at the time it was > > > created, it was for all the "tracers" (this was added before trace > > > events). > > > > It would be great to see this cleaned up. I found this aspect of how > > tracing works rather confusing. > > > > So do you think it makes sense for the KASAN tests to "select TRACING" > > for now if the code depends on the trace event infrastructure? > > Any thoughts? It looks like someone else got tripped up by this: > https://reviews.llvm.org/D144057 Yeah, it really does need to get cleaned up, but unfortunately it's not going to be a trivial change. We need to make sure it's done in a way that an old .config still keeps the same things enabled with the new config settings. That takes some trickery in the dependency. I'll add this to my todo list, hopefully it doesn't fall into the abyss portion of that list :-p -- Steve
Hi all! On 05.05.2023 09:58, Steven Rostedt wrote: > On Mon, 1 May 2023 15:02:37 -0700 > Peter Collingbourne <pcc@google.com> wrote: > > > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > > > > really should just be for the function tracing infrastructure, and > > > > perhaps not even include trace events :-/ But at the time it was > > > > created, it was for all the "tracers" (this was added before trace > > > > events). > > > > > > It would be great to see this cleaned up. I found this aspect of how > > > tracing works rather confusing. > > > > > > So do you think it makes sense for the KASAN tests to "select TRACING" > > > for now if the code depends on the trace event infrastructure? > > > > Any thoughts? It looks like someone else got tripped up by this: > > https://reviews.llvm.org/D144057 > > Yeah, it really does need to get cleaned up, but unfortunately it's not > going to be a trivial change. We need to make sure it's done in a way that > an old .config still keeps the same things enabled with the new config > settings. That takes some trickery in the dependency. > > I'll add this to my todo list, hopefully it doesn't fall into the abyss > portion of that list :-p > > -- Steve Just adding to Peter's concern re: CONFIG_KASAN_KUNIT_TEST's dependency on CONFIG_TRACEPOINTS. I'm having no luck running the KASan KUnit tests on arm64 with the following .kunitconfig on v6.6.0: CONFIG_KUNIT=y CONFIG_KUNIT_ALL_TESTS=n CONFIG_DEBUG_KERNEL=y CONFIG_KASAN=y CINFIG_KASAN_GENERIC=y CONFIG_KASAN_KUNIT_TEST=y CONFIG_TRACEPOINTS, which CONFIG_KASAN_TEST relies on since the patch this thread is based on, isn't defined for arm64, AFAICT. If I comment out the dependency on CONFIG_TRACEPOINTS, the tests appear to run, but KUnit isn't picking up the KASan output. If I revert the patch, the above .kunitconfig appears to work fine on arm64 and the tests pass. The above .kunitconfig works as intended on X86, no changes necessary. Am I missing something? Many thanks, Paul
On Mon, Dec 11, 2023 at 5:37 PM Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > Hi all! > > On 05.05.2023 09:58, Steven Rostedt wrote: > > On Mon, 1 May 2023 15:02:37 -0700 > > Peter Collingbourne <pcc@google.com> wrote: > > > > > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > > > > > really should just be for the function tracing infrastructure, and > > > > > perhaps not even include trace events :-/ But at the time it was > > > > > created, it was for all the "tracers" (this was added before trace > > > > > events). > > > > > > > > It would be great to see this cleaned up. I found this aspect of how > > > > tracing works rather confusing. > > > > > > > > So do you think it makes sense for the KASAN tests to "select TRACING" > > > > for now if the code depends on the trace event infrastructure? > > > > > > Any thoughts? It looks like someone else got tripped up by this: > > > https://reviews.llvm.org/D144057 > > > > Yeah, it really does need to get cleaned up, but unfortunately it's not > > going to be a trivial change. We need to make sure it's done in a way that > > an old .config still keeps the same things enabled with the new config > > settings. That takes some trickery in the dependency. > > > > I'll add this to my todo list, hopefully it doesn't fall into the abyss > > portion of that list :-p > > > > -- Steve > > Just adding to Peter's concern re: CONFIG_KASAN_KUNIT_TEST's dependency on > CONFIG_TRACEPOINTS. > > I'm having no luck running the KASan KUnit tests on arm64 with the following > .kunitconfig on v6.6.0: > > CONFIG_KUNIT=y > CONFIG_KUNIT_ALL_TESTS=n > CONFIG_DEBUG_KERNEL=y > CONFIG_KASAN=y > CINFIG_KASAN_GENERIC=y > CONFIG_KASAN_KUNIT_TEST=y > > CONFIG_TRACEPOINTS, which CONFIG_KASAN_TEST relies on since the patch this > thread is based on, isn't defined for arm64, AFAICT. > > If I comment out the dependency on CONFIG_TRACEPOINTS, the tests appear to run, > but KUnit isn't picking up the KASan output. > > If I revert the patch, the above .kunitconfig appears to work fine on arm64 and > the tests pass. > > The above .kunitconfig works as intended on X86, no changes necessary. > > Am I missing something? Hi Paul, I've been successfully running KASAN tests with CONFIG_TRACEPOINTS enabled on arm64 since this patch landed. What happens when you try running the tests with .kunitconfig? Does CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during kernel building? Or tests just don't get executed? Thanks!
Hi Andrey! On 11.12.2023 18:50, Andrey Konovalov wrote: > On Mon, Dec 11, 2023 at 5:37 PM Paul Heidekrüger > <paul.heidekrueger@tum.de> wrote: > > > > Hi all! > > > > On 05.05.2023 09:58, Steven Rostedt wrote: > > > On Mon, 1 May 2023 15:02:37 -0700 > > > Peter Collingbourne <pcc@google.com> wrote: > > > > > > > > > "ftrace" is really for just the function tracing, but CONFIG_FTRACE > > > > > > really should just be for the function tracing infrastructure, and > > > > > > perhaps not even include trace events :-/ But at the time it was > > > > > > created, it was for all the "tracers" (this was added before trace > > > > > > events). > > > > > > > > > > It would be great to see this cleaned up. I found this aspect of how > > > > > tracing works rather confusing. > > > > > > > > > > So do you think it makes sense for the KASAN tests to "select TRACING" > > > > > for now if the code depends on the trace event infrastructure? > > > > > > > > Any thoughts? It looks like someone else got tripped up by this: > > > > https://reviews.llvm.org/D144057 > > > > > > Yeah, it really does need to get cleaned up, but unfortunately it's not > > > going to be a trivial change. We need to make sure it's done in a way that > > > an old .config still keeps the same things enabled with the new config > > > settings. That takes some trickery in the dependency. > > > > > > I'll add this to my todo list, hopefully it doesn't fall into the abyss > > > portion of that list :-p > > > > > > -- Steve > > > > Just adding to Peter's concern re: CONFIG_KASAN_KUNIT_TEST's dependency on > > CONFIG_TRACEPOINTS. > > > > I'm having no luck running the KASan KUnit tests on arm64 with the following > > .kunitconfig on v6.6.0: > > > > CONFIG_KUNIT=y > > CONFIG_KUNIT_ALL_TESTS=n > > CONFIG_DEBUG_KERNEL=y > > CONFIG_KASAN=y > > CINFIG_KASAN_GENERIC=y > > CONFIG_KASAN_KUNIT_TEST=y > > > > CONFIG_TRACEPOINTS, which CONFIG_KASAN_TEST relies on since the patch this > > thread is based on, isn't defined for arm64, AFAICT. > > > > If I comment out the dependency on CONFIG_TRACEPOINTS, the tests appear to run, > > but KUnit isn't picking up the KASan output. > > > > If I revert the patch, the above .kunitconfig appears to work fine on arm64 and > > the tests pass. > > > > The above .kunitconfig works as intended on X86, no changes necessary. > > > > Am I missing something? > > Hi Paul, > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS > enabled on arm64 since this patch landed. Interesting ... > What happens when you try running the tests with .kunitconfig? Does > CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during > kernel building? Yes, exactly, that's what's happening. Here's the output kunit.py is giving me. I replaced CONFIG_DEBUG_KERNEL with CONFIG_TRACEPOINTS in my .kunitconfig. Otherwise, it's identical with the one I posted above. ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 Configuring KUnit Kernel ... Regenerating .config ... Populating config with: $ make ARCH=arm64 O=.kunit olddefconfig ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. This is probably due to unsatisfied dependencies. Missing: CONFIG_KASAN_KUNIT_TEST=y, CONFIG_TRACEPOINTS=y Does CONFIG_TRACEPOINTS have some dependency I'm not seeing? I couldn't find a reason why it would get disabled, but I could definitely be wrong. > Or tests just don't get executed? > > Thanks! Many thanks, Paul
On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > > Hi Paul, > > > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS > > enabled on arm64 since this patch landed. > > Interesting ... > > > What happens when you try running the tests with .kunitconfig? Does > > CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during > > kernel building? > > Yes, exactly, that's what's happening. > > Here's the output kunit.py is giving me. I replaced CONFIG_DEBUG_KERNEL with > CONFIG_TRACEPOINTS in my .kunitconfig. Otherwise, it's identical with the one I > posted above. > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > Configuring KUnit Kernel ... > Regenerating .config ... > Populating config with: > $ make ARCH=arm64 O=.kunit olddefconfig > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > This is probably due to unsatisfied dependencies. > Missing: CONFIG_KASAN_KUNIT_TEST=y, CONFIG_TRACEPOINTS=y > > Does CONFIG_TRACEPOINTS have some dependency I'm not seeing? I couldn't find a > reason why it would get disabled, but I could definitely be wrong. Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in the listing that you sent earlier.
On 11.12.2023 21:51, Andrey Konovalov wrote: > On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger > <paul.heidekrueger@tum.de> wrote: > > > > > Hi Paul, > > > > > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS > > > enabled on arm64 since this patch landed. > > > > Interesting ... > > > > > What happens when you try running the tests with .kunitconfig? Does > > > CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during > > > kernel building? > > > > Yes, exactly, that's what's happening. > > > > Here's the output kunit.py is giving me. I replaced CONFIG_DEBUG_KERNEL with > > CONFIG_TRACEPOINTS in my .kunitconfig. Otherwise, it's identical with the one I > > posted above. > > > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > > Configuring KUnit Kernel ... > > Regenerating .config ... > > Populating config with: > > $ make ARCH=arm64 O=.kunit olddefconfig > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > This is probably due to unsatisfied dependencies. > > Missing: CONFIG_KASAN_KUNIT_TEST=y, CONFIG_TRACEPOINTS=y > > > > Does CONFIG_TRACEPOINTS have some dependency I'm not seeing? I couldn't find a > > reason why it would get disabled, but I could definitely be wrong. > > Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in > the listing that you sent earlier. Yes. For the kunit.py output from my previous email, I replaced CONFIG_DEBUG_KERNEL=y with CONFIG_TRACEPOINTS=y. So, the .kunitconfig I used to produce the output above was: CONFIG_KUNIT=y CONFIG_KUNIT_ALL_TESTS=n CONFIG_TRACEPOINTS=y CONFIG_KASAN=y CONFIG_KASAN_GENERIC=y CONFIG_KASAN_KUNIT_TEST=y This more or less mirrors what mm/kfence/.kunitconfig is doing, which also isn't working on my side; kunit.py reports the same error. Many thanks, Paul
On Mon, 11 Dec 2023 at 23:48, Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > On 11.12.2023 21:51, Andrey Konovalov wrote: > > On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger > > <paul.heidekrueger@tum.de> wrote: > > > > > > > Hi Paul, > > > > > > > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS > > > > enabled on arm64 since this patch landed. > > > > > > Interesting ... > > > > > > > What happens when you try running the tests with .kunitconfig? Does > > > > CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during > > > > kernel building? > > > > > > Yes, exactly, that's what's happening. > > > > > > Here's the output kunit.py is giving me. I replaced CONFIG_DEBUG_KERNEL with > > > CONFIG_TRACEPOINTS in my .kunitconfig. Otherwise, it's identical with the one I > > > posted above. > > > > > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > > > Configuring KUnit Kernel ... > > > Regenerating .config ... > > > Populating config with: > > > $ make ARCH=arm64 O=.kunit olddefconfig > > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > > This is probably due to unsatisfied dependencies. > > > Missing: CONFIG_KASAN_KUNIT_TEST=y, CONFIG_TRACEPOINTS=y > > > > > > Does CONFIG_TRACEPOINTS have some dependency I'm not seeing? I couldn't find a > > > reason why it would get disabled, but I could definitely be wrong. > > > > Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in > > the listing that you sent earlier. > > Yes. For the kunit.py output from my previous email, I replaced > CONFIG_DEBUG_KERNEL=y with CONFIG_TRACEPOINTS=y. So, the .kunitconfig I used to > produce the output above was: > > CONFIG_KUNIT=y > CONFIG_KUNIT_ALL_TESTS=n > CONFIG_TRACEPOINTS=y > CONFIG_KASAN=y > CONFIG_KASAN_GENERIC=y > CONFIG_KASAN_KUNIT_TEST=y > > This more or less mirrors what mm/kfence/.kunitconfig is doing, which also isn't > working on my side; kunit.py reports the same error. mm/kfence/.kunitconfig does CONFIG_FTRACE=y. TRACEPOINTS is not user selectable. I don't think any of this has changed since the initial discussion above, so CONFIG_FTRACE=y is still needed.
On 11.12.2023 23:56, Marco Elver wrote: > On Mon, 11 Dec 2023 at 23:48, Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > > > On 11.12.2023 21:51, Andrey Konovalov wrote: > > > On Mon, Dec 11, 2023 at 7:59 PM Paul Heidekrüger > > > <paul.heidekrueger@tum.de> wrote: > > > > > > > > > Hi Paul, > > > > > > > > > > I've been successfully running KASAN tests with CONFIG_TRACEPOINTS > > > > > enabled on arm64 since this patch landed. > > > > > > > > Interesting ... > > > > > > > > > What happens when you try running the tests with .kunitconfig? Does > > > > > CONFIG_TRACEPOINTS or CONFIG_KASAN_KUNIT_TEST get disabled during > > > > > kernel building? > > > > > > > > Yes, exactly, that's what's happening. > > > > > > > > Here's the output kunit.py is giving me. I replaced CONFIG_DEBUG_KERNEL with > > > > CONFIG_TRACEPOINTS in my .kunitconfig. Otherwise, it's identical with the one I > > > > posted above. > > > > > > > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > > > > Configuring KUnit Kernel ... > > > > Regenerating .config ... > > > > Populating config with: > > > > $ make ARCH=arm64 O=.kunit olddefconfig > > > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > > > This is probably due to unsatisfied dependencies. > > > > Missing: CONFIG_KASAN_KUNIT_TEST=y, CONFIG_TRACEPOINTS=y > > > > > > > > Does CONFIG_TRACEPOINTS have some dependency I'm not seeing? I couldn't find a > > > > reason why it would get disabled, but I could definitely be wrong. > > > > > > Does your .kunitconfig include CONFIG_TRACEPOINTS=y? I don't see it in > > > the listing that you sent earlier. > > > > Yes. For the kunit.py output from my previous email, I replaced > > CONFIG_DEBUG_KERNEL=y with CONFIG_TRACEPOINTS=y. So, the .kunitconfig I used to > > produce the output above was: > > > > CONFIG_KUNIT=y > > CONFIG_KUNIT_ALL_TESTS=n > > CONFIG_TRACEPOINTS=y > > CONFIG_KASAN=y > > CONFIG_KASAN_GENERIC=y > > CONFIG_KASAN_KUNIT_TEST=y > > > > This more or less mirrors what mm/kfence/.kunitconfig is doing, which also isn't > > working on my side; kunit.py reports the same error. > > mm/kfence/.kunitconfig does CONFIG_FTRACE=y. TRACEPOINTS is not user > selectable. I don't think any of this has changed since the initial > discussion above, so CONFIG_FTRACE=y is still needed. Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error for me. So CONFIG_KUNIT=y CONFIG_KUNIT_ALL_TESTS=n CONFIG_FTRACE=y CONFIG_KASAN=y CONFIG_KASAN_GENERIC=y CONFIG_KASAN_KUNIT_TEST=y produces ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 Configuring KUnit Kernel ... Regenerating .config ... Populating config with: $ make ARCH=arm64 O=.kunit olddefconfig CC=clang ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. This is probably due to unsatisfied dependencies. Missing: CONFIG_KASAN_KUNIT_TEST=y By that error message, CONFIG_FTRACE appears to be present in the generated config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably, CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which must be CONFIG_TRACEPOINTS, unless I'm missing something ... If I just generate an arm64 defconfig and select CONFIG_FTRACE=y, CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related then? Andrey, you said that the tests have been working for you; are you running them with kunit.py? Many thanks, Paul
On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error > for me. > > So > > CONFIG_KUNIT=y > CONFIG_KUNIT_ALL_TESTS=n > CONFIG_FTRACE=y > CONFIG_KASAN=y > CONFIG_KASAN_GENERIC=y > CONFIG_KASAN_KUNIT_TEST=y > > produces > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > Configuring KUnit Kernel ... > Regenerating .config ... > Populating config with: > $ make ARCH=arm64 O=.kunit olddefconfig CC=clang > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > This is probably due to unsatisfied dependencies. > Missing: CONFIG_KASAN_KUNIT_TEST=y > > By that error message, CONFIG_FTRACE appears to be present in the generated > config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably, > CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which > must be CONFIG_TRACEPOINTS, unless I'm missing something ... > > If I just generate an arm64 defconfig and select CONFIG_FTRACE=y, > CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related > then? > > Andrey, you said that the tests have been working for you; are you running them > with kunit.py? No, I just run the kernel built with a config file that I put together based on defconfig.
On 12.12.2023 00:37, Andrey Konovalov wrote: > On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger > <paul.heidekrueger@tum.de> wrote: > > > > Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error > > for me. > > > > So > > > > CONFIG_KUNIT=y > > CONFIG_KUNIT_ALL_TESTS=n > > CONFIG_FTRACE=y > > CONFIG_KASAN=y > > CONFIG_KASAN_GENERIC=y > > CONFIG_KASAN_KUNIT_TEST=y > > > > produces > > > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > > Configuring KUnit Kernel ... > > Regenerating .config ... > > Populating config with: > > $ make ARCH=arm64 O=.kunit olddefconfig CC=clang > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > This is probably due to unsatisfied dependencies. > > Missing: CONFIG_KASAN_KUNIT_TEST=y > > > > By that error message, CONFIG_FTRACE appears to be present in the generated > > config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably, > > CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which > > must be CONFIG_TRACEPOINTS, unless I'm missing something ... > > > > If I just generate an arm64 defconfig and select CONFIG_FTRACE=y, > > CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related > > then? > > > > Andrey, you said that the tests have been working for you; are you running them > > with kunit.py? > > No, I just run the kernel built with a config file that I put together > based on defconfig. Ah. I believe I've figured it out. When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works. CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and CONFIG_TRACING selects CONFIG_TRACEPOINTS. CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it directly selects CONFIG_TRACEPOINTS. CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64. I believe the reason my .kunitconfig as well as the existing mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in an X86 defconfig. Does this make sense? Would you welcome a patch addressing this for the existing mm/kfence/.kunitconfig? I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think that would be helpful too? FWICT, kernel/kcsan/.kunitconfig might also be affected since CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to test that. That could be a third patch. What do you think? Many thanks, Paul
On Tue, 12 Dec 2023 at 10:19, Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > On 12.12.2023 00:37, Andrey Konovalov wrote: > > On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger > > <paul.heidekrueger@tum.de> wrote: > > > > > > Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error > > > for me. > > > > > > So > > > > > > CONFIG_KUNIT=y > > > CONFIG_KUNIT_ALL_TESTS=n > > > CONFIG_FTRACE=y > > > CONFIG_KASAN=y > > > CONFIG_KASAN_GENERIC=y > > > CONFIG_KASAN_KUNIT_TEST=y > > > > > > produces > > > > > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > > > Configuring KUnit Kernel ... > > > Regenerating .config ... > > > Populating config with: > > > $ make ARCH=arm64 O=.kunit olddefconfig CC=clang > > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > > This is probably due to unsatisfied dependencies. > > > Missing: CONFIG_KASAN_KUNIT_TEST=y > > > > > > By that error message, CONFIG_FTRACE appears to be present in the generated > > > config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably, > > > CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which > > > must be CONFIG_TRACEPOINTS, unless I'm missing something ... > > > > > > If I just generate an arm64 defconfig and select CONFIG_FTRACE=y, > > > CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related > > > then? > > > > > > Andrey, you said that the tests have been working for you; are you running them > > > with kunit.py? > > > > No, I just run the kernel built with a config file that I put together > > based on defconfig. > > Ah. I believe I've figured it out. > > When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works. CONFIG_FTRACE should be enough - maybe also check x86 vs. arm64 to debug more. > CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER > selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and > CONFIG_TRACING selects CONFIG_TRACEPOINTS. > > CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it > directly selects CONFIG_TRACEPOINTS. > > CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64. When you build manually with just CONFIG_FTRACE, is CONFIG_TRACEPOINTS enabled? > I believe the reason my .kunitconfig as well as the existing > mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in > an X86 defconfig. > > Does this make sense? > > Would you welcome a patch addressing this for the existing > mm/kfence/.kunitconfig? > > I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think > that would be helpful too? > > FWICT, kernel/kcsan/.kunitconfig might also be affected since > CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to > test that. That could be a third patch. I'd support figuring out the minimal config (CONFIG_FTRACE or something else?) that satisfies the TRACEPOINTS dependency. I always thought CONFIG_FTRACE ought to be the one config option, but maybe something changed. Also maybe one of the tracing maintainers can help untangle what's going on here. Thanks, -- Marco
On 12.12.2023 10:32, Marco Elver wrote: > On Tue, 12 Dec 2023 at 10:19, Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: > > > > On 12.12.2023 00:37, Andrey Konovalov wrote: > > > On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger > > > <paul.heidekrueger@tum.de> wrote: > > > > > > > > Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error > > > > for me. > > > > > > > > So > > > > > > > > CONFIG_KUNIT=y > > > > CONFIG_KUNIT_ALL_TESTS=n > > > > CONFIG_FTRACE=y > > > > CONFIG_KASAN=y > > > > CONFIG_KASAN_GENERIC=y > > > > CONFIG_KASAN_KUNIT_TEST=y > > > > > > > > produces > > > > > > > > ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 > > > > Configuring KUnit Kernel ... > > > > Regenerating .config ... > > > > Populating config with: > > > > $ make ARCH=arm64 O=.kunit olddefconfig CC=clang > > > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > > > This is probably due to unsatisfied dependencies. > > > > Missing: CONFIG_KASAN_KUNIT_TEST=y > > > > > > > > By that error message, CONFIG_FTRACE appears to be present in the generated > > > > config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably, > > > > CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which > > > > must be CONFIG_TRACEPOINTS, unless I'm missing something ... > > > > > > > > If I just generate an arm64 defconfig and select CONFIG_FTRACE=y, > > > > CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related > > > > then? > > > > > > > > Andrey, you said that the tests have been working for you; are you running them > > > > with kunit.py? > > > > > > No, I just run the kernel built with a config file that I put together > > > based on defconfig. > > > > Ah. I believe I've figured it out. > > > > When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works. > > CONFIG_FTRACE should be enough - maybe also check x86 vs. arm64 to debug more. See below. > > CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER > > selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and > > CONFIG_TRACING selects CONFIG_TRACEPOINTS. > > > > CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it > > directly selects CONFIG_TRACEPOINTS. > > > > CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64. > > When you build manually with just CONFIG_FTRACE, is CONFIG_TRACEPOINTS enabled? When I add CONFIG_FTRACE and enter-key my way through the FTRACE prompts - I believe because CONFIG_FTRACE is a menuconfig? - at the beginning of a build, CONFIG_TRACEPOINTS does get set on arm64, yes. On X86, the defconfig already includes CONIFG_TRACEPOINTS. I also had a closer look at how kunit.py builds its configs. I believe it does something along the following lines: cp <path_to_kunitconfig> .kunit/.config make ARCH=arm64 O=.kunit olddefconfig On arm64, that isn't enough to set CONFIG_TRACEPOINTS; same behaviour when run outside of kunit.py. For CONFIG_TRACEPOINTS, `make ARCH=arm64 menuconfig` shows: Symbol: TRACEPOINTS [=n] Type : bool Defined at init/Kconfig:1920 Selected by [n]: - TRACING [=n] - BLK_DEV_IO_TRACE [=n] && FTRACE [=y] && SYSFS [=y] && BLOCK [=y] So, CONFIG_TRACING or CONFIG_BLK_DEV_IO_TRACE are the two options that prevent CONFIG_TRACEPOINTS from being set on arm64. For CONFIG_TRACING we have: Symbol: TRACING [=n] Type : bool Defined at kernel/trace/Kconfig:157 Selects: RING_BUFFER [=n] && STACKTRACE [=y] && TRACEPOINTS [=n] && NOP_TRACER [=n] && BINARY_PRINTF [=n] && EVENT_TRACING [=n] && TRACE_CLOCK [=y] && TASKS_RCU [=n] Selected by [n]: - DRM_I915_TRACE_GEM [=n] && HAS_IOMEM [=y] && DRM_I915 [=n] && EXPERT [=n] && DRM_I915_DEBUG_GEM [=n] - DRM_I915_TRACE_GTT [=n] && HAS_IOMEM [=y] && DRM_I915 [=n] && EXPERT [=n] && DRM_I915_DEBUG_GEM [=n] - PREEMPTIRQ_TRACEPOINTS [=n] && (TRACE_PREEMPT_TOGGLE [=n] || TRACE_IRQFLAGS [=n]) - GENERIC_TRACER [=n] - ENABLE_DEFAULT_TRACERS [=n] && FTRACE [=y] && !GENERIC_TRACER [=n] - FPROBE_EVENTS [=n] && FTRACE [=y] && FPROBE [=n] && HAVE_REGS_AND_STACK_ACCESS_API [=y] - KPROBE_EVENTS [=n] && FTRACE [=y] && KPROBES [=n] && HAVE_REGS_AND_STACK_ACCESS_API [=y] - UPROBE_EVENTS [=n] && FTRACE [=y] && ARCH_SUPPORTS_UPROBES [=y] && MMU [=y] && PERF_EVENTS [=n] - SYNTH_EVENTS [=n] && FTRACE [=y] - USER_EVENTS [=n] && FTRACE [=y] - HIST_TRIGGERS [=n] && FTRACE [=y] && ARCH_HAVE_NMI_SAFE_CMPXCHG [=y] > > I believe the reason my .kunitconfig as well as the existing > > mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in > > an X86 defconfig. > > > > Does this make sense? > > > > Would you welcome a patch addressing this for the existing > > mm/kfence/.kunitconfig? > > > > I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think > > that would be helpful too? > > > > FWICT, kernel/kcsan/.kunitconfig might also be affected since > > CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to > > test that. That could be a third patch. > > I'd support figuring out the minimal config (CONFIG_FTRACE or > something else?) that satisfies the TRACEPOINTS dependency. I always > thought CONFIG_FTRACE ought to be the one config option, but maybe > something changed. If we want a minimal config, setting CONFIG_BLK_DEV_IO_TRACE, CONFIG_SYNTH_EVENTS or CONFIG_USER_EVENTS seem like viable options, for instance. But AFAICT, setting them in the context of KASan doesn't really make sense, and I might be missing an obvious choice here too. What do you think? > Also maybe one of the tracing maintainers can help untangle what's > going on here. > > Thanks, > -- Marco Many thanks, Paul
On 7 Jan 2024, at 19:22, Paul Heidekrüger wrote: > On 12.12.2023 10:32, Marco Elver wrote: >> On Tue, 12 Dec 2023 at 10:19, Paul Heidekrüger <paul.heidekrueger@tum.de> wrote: >>> >>> On 12.12.2023 00:37, Andrey Konovalov wrote: >>>> On Tue, Dec 12, 2023 at 12:35 AM Paul Heidekrüger >>>> <paul.heidekrueger@tum.de> wrote: >>>>> >>>>> Using CONFIG_FTRACE=y instead of CONFIG_TRACEPOINTS=y produces the same error >>>>> for me. >>>>> >>>>> So >>>>> >>>>> CONFIG_KUNIT=y >>>>> CONFIG_KUNIT_ALL_TESTS=n >>>>> CONFIG_FTRACE=y >>>>> CONFIG_KASAN=y >>>>> CONFIG_KASAN_GENERIC=y >>>>> CONFIG_KASAN_KUNIT_TEST=y >>>>> >>>>> produces >>>>> >>>>> ➜ ./tools/testing/kunit/kunit.py run --kunitconfig=mm/kasan/.kunitconfig --arch=arm64 >>>>> Configuring KUnit Kernel ... >>>>> Regenerating .config ... >>>>> Populating config with: >>>>> $ make ARCH=arm64 O=.kunit olddefconfig CC=clang >>>>> ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. >>>>> This is probably due to unsatisfied dependencies. >>>>> Missing: CONFIG_KASAN_KUNIT_TEST=y >>>>> >>>>> By that error message, CONFIG_FTRACE appears to be present in the generated >>>>> config, but CONFIG_KASAN_KUNIT_TEST still isn't. Presumably, >>>>> CONFIG_KASAN_KUNIT_TEST is missing because of an unsatisfied dependency, which >>>>> must be CONFIG_TRACEPOINTS, unless I'm missing something ... >>>>> >>>>> If I just generate an arm64 defconfig and select CONFIG_FTRACE=y, >>>>> CONFIG_TRACEPOINTS=y shows up in my .config. So, maybe this is kunit.py-related >>>>> then? >>>>> >>>>> Andrey, you said that the tests have been working for you; are you running them >>>>> with kunit.py? >>>> >>>> No, I just run the kernel built with a config file that I put together >>>> based on defconfig. >>> >>> Ah. I believe I've figured it out. >>> >>> When I add CONFIG_STACK_TRACER=y in addition to CONFIG_FTRACE=y, it works. >> >> CONFIG_FTRACE should be enough - maybe also check x86 vs. arm64 to debug more. > > See below. > >>> CONFIG_STACK_TRACER selects CONFIG_FUNCTION_TRACER, CONFIG_FUNCTION_TRACER >>> selects CONFIG_GENERIC_TRACER, CONFIG_GENERIC_TRACER selects CONFIG_TRACING, and >>> CONFIG_TRACING selects CONFIG_TRACEPOINTS. >>> >>> CONFIG_BLK_DEV_IO_TRACE=y also works instead of CONFIG_STACK_TRACER=y, as it >>> directly selects CONFIG_TRACEPOINTS. >>> >>> CONFIG_FTRACE=y on its own does not appear suffice for kunit.py on arm64. >> >> When you build manually with just CONFIG_FTRACE, is CONFIG_TRACEPOINTS enabled? > > When I add CONFIG_FTRACE and enter-key my way through the FTRACE prompts - I > believe because CONFIG_FTRACE is a menuconfig? - at the beginning of a build, > CONFIG_TRACEPOINTS does get set on arm64, yes. > > On X86, the defconfig already includes CONIFG_TRACEPOINTS. > > I also had a closer look at how kunit.py builds its configs. > I believe it does something along the following lines: > > cp <path_to_kunitconfig> .kunit/.config > make ARCH=arm64 O=.kunit olddefconfig > > On arm64, that isn't enough to set CONFIG_TRACEPOINTS; same behaviour when run > outside of kunit.py. > > For CONFIG_TRACEPOINTS, `make ARCH=arm64 menuconfig` shows: > > Symbol: TRACEPOINTS [=n] > Type : bool > Defined at init/Kconfig:1920 > Selected by [n]: > - TRACING [=n] > - BLK_DEV_IO_TRACE [=n] && FTRACE [=y] && SYSFS [=y] && BLOCK [=y] > > So, CONFIG_TRACING or CONFIG_BLK_DEV_IO_TRACE are the two options that prevent > CONFIG_TRACEPOINTS from being set on arm64. > > For CONFIG_TRACING we have: > > Symbol: TRACING [=n] > Type : bool > Defined at kernel/trace/Kconfig:157 > Selects: RING_BUFFER [=n] && STACKTRACE [=y] && TRACEPOINTS [=n] && NOP_TRACER [=n] && BINARY_PRINTF [=n] && EVENT_TRACING [=n] && TRACE_CLOCK [=y] && TASKS_RCU [=n] > Selected by [n]: > - DRM_I915_TRACE_GEM [=n] && HAS_IOMEM [=y] && DRM_I915 [=n] && EXPERT [=n] && DRM_I915_DEBUG_GEM [=n] > - DRM_I915_TRACE_GTT [=n] && HAS_IOMEM [=y] && DRM_I915 [=n] && EXPERT [=n] && DRM_I915_DEBUG_GEM [=n] > - PREEMPTIRQ_TRACEPOINTS [=n] && (TRACE_PREEMPT_TOGGLE [=n] || TRACE_IRQFLAGS [=n]) > - GENERIC_TRACER [=n] > - ENABLE_DEFAULT_TRACERS [=n] && FTRACE [=y] && !GENERIC_TRACER [=n] > - FPROBE_EVENTS [=n] && FTRACE [=y] && FPROBE [=n] && HAVE_REGS_AND_STACK_ACCESS_API [=y] > - KPROBE_EVENTS [=n] && FTRACE [=y] && KPROBES [=n] && HAVE_REGS_AND_STACK_ACCESS_API [=y] > - UPROBE_EVENTS [=n] && FTRACE [=y] && ARCH_SUPPORTS_UPROBES [=y] && MMU [=y] && PERF_EVENTS [=n] > - SYNTH_EVENTS [=n] && FTRACE [=y] > - USER_EVENTS [=n] && FTRACE [=y] > - HIST_TRIGGERS [=n] && FTRACE [=y] && ARCH_HAVE_NMI_SAFE_CMPXCHG [=y] > >>> I believe the reason my .kunitconfig as well as the existing >>> mm/kfence/.kunitconfig work on X86 is because CONFIG_TRACEPOINTS=y is present in >>> an X86 defconfig. >>> >>> Does this make sense? >>> >>> Would you welcome a patch addressing this for the existing >>> mm/kfence/.kunitconfig? >>> >>> I would also like to submit a patch for an mm/kasan/.kunitconfig. Do you think >>> that would be helpful too? >>> >>> FWICT, kernel/kcsan/.kunitconfig might also be affected since >>> CONFIG_KCSAN_KUNIT_TEST also depends on CONFIG_TRACEPOITNS, but I would have to >>> test that. That could be a third patch. >> >> I'd support figuring out the minimal config (CONFIG_FTRACE or >> something else?) that satisfies the TRACEPOINTS dependency. I always >> thought CONFIG_FTRACE ought to be the one config option, but maybe >> something changed. > > If we want a minimal config, setting CONFIG_BLK_DEV_IO_TRACE, > CONFIG_SYNTH_EVENTS or CONFIG_USER_EVENTS seem like viable options, for > instance. But AFAICT, setting them in the context of KASan doesn't really make > sense, and I might be missing an obvious choice here too. > > What do you think? > >> Also maybe one of the tracing maintainers can help untangle what's >> going on here. >> >> Thanks, >> -- Marco > > Many thanks, > Paul Hi all, Just giving this thread a polite bump, hoping that someone has some pointers. The TL;DR is the following: I’m trying to run KASan KUnit tests with the following local .kunitconfig: CONFIG_KUNIT=y CONFIG_KUNIT_ALL_TESTS=n CONFIG_FTRACE=y CONFIG_KASAN=y CONFIG_KASAN_GENERIC=y CONFIG_KASAN_KUNIT_TEST=y The problem is that on arm64, this does not appear to be enough to set all of CONFIG_KASAN_KUNIT_TEST’s dependencies, namely CONFIG_TRACEPOINTS. An additional option is needed to enable CONFIG_TRACEPOINTS. As per `make menuconfig`, this is either CONFIG_BLK_DEV_IO_TRACE or any (combination of) option(s) that enable(s) CONFIG_TRACING. See the `make menuconfig` output in my previous email for details. Which option do you think is appropriate here? Or am I missing something? For anyone wanting to reproduce, use: ./tools/testing/kunit/kunit.py run —kunitconfig=<path_to_above_kunitconfig> --arch=arm64 Many thanks, Paul
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan index ca09b1cf8ee9..ba5b27962c34 100644 --- a/lib/Kconfig.kasan +++ b/lib/Kconfig.kasan @@ -181,7 +181,7 @@ config KASAN_VMALLOC config KASAN_KUNIT_TEST tristate "KUnit-compatible tests of KASAN bug detection capabilities" if !KUNIT_ALL_TESTS - depends on KASAN && KUNIT + depends on KASAN && KUNIT && TRACEPOINTS default KUNIT_ALL_TESTS help A KUnit-based KASAN test suite. Triggers different kinds of diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index abbcc1b0eec5..a84491bc4867 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -261,14 +261,6 @@ struct kasan_stack_ring { #endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */ -#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST) -/* Used in KUnit-compatible KASAN tests. */ -struct kunit_kasan_status { - bool report_found; - bool sync_fault; -}; -#endif - #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) static inline const void *kasan_shadow_to_mem(const void *shadow_addr) diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c index 0d59098f0876..0ff20bfa3376 100644 --- a/mm/kasan/kasan_test.c +++ b/mm/kasan/kasan_test.c @@ -5,8 +5,12 @@ * Author: Andrey Ryabinin <a.ryabinin@samsung.com> */ +#define pr_fmt(fmt) "kasan_test: " fmt + +#include <kunit/test.h> #include <linux/bitops.h> #include <linux/delay.h> +#include <linux/io.h> #include <linux/kasan.h> #include <linux/kernel.h> #include <linux/mm.h> @@ -14,21 +18,28 @@ #include <linux/module.h> #include <linux/printk.h> #include <linux/random.h> +#include <linux/set_memory.h> #include <linux/slab.h> #include <linux/string.h> +#include <linux/tracepoint.h> #include <linux/uaccess.h> -#include <linux/io.h> #include <linux/vmalloc.h> -#include <linux/set_memory.h> +#include <trace/events/printk.h> #include <asm/page.h> -#include <kunit/test.h> - #include "kasan.h" #define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 : KASAN_GRANULE_SIZE) +static bool multishot; + +/* Fields set based on lines observed in the console. */ +static struct { + bool report_found; + bool async_fault; +} test_status; + /* * Some tests use these global variables to store return values from function * calls that could otherwise be eliminated by the compiler as dead code. @@ -36,35 +47,61 @@ void *kasan_ptr_result; int kasan_int_result; -static struct kunit_resource resource; -static struct kunit_kasan_status test_status; -static bool multishot; +/* Probe for console output: obtains test_status lines of interest. */ +static void probe_console(void *ignore, const char *buf, size_t len) +{ + if (strnstr(buf, "BUG: KASAN: ", len)) + WRITE_ONCE(test_status.report_found, true); + else if (strnstr(buf, "Asynchronous fault: ", len)) + WRITE_ONCE(test_status.async_fault, true); +} -/* - * Temporarily enable multi-shot mode. Otherwise, KASAN would only report the - * first detected bug and panic the kernel if panic_on_warn is enabled. For - * hardware tag-based KASAN also allow tag checking to be reenabled for each - * test, see the comment for KUNIT_EXPECT_KASAN_FAIL(). - */ -static int kasan_test_init(struct kunit *test) +static void register_tracepoints(struct tracepoint *tp, void *ignore) +{ + check_trace_callback_type_console(probe_console); + if (!strcmp(tp->name, "console")) + WARN_ON(tracepoint_probe_register(tp, probe_console, NULL)); +} + +static void unregister_tracepoints(struct tracepoint *tp, void *ignore) +{ + if (!strcmp(tp->name, "console")) + tracepoint_probe_unregister(tp, probe_console, NULL); +} + +static int kasan_suite_init(struct kunit_suite *suite) { if (!kasan_enabled()) { - kunit_err(test, "can't run KASAN tests with KASAN disabled"); + pr_err("Can't run KASAN tests with KASAN disabled"); return -1; } + /* + * Temporarily enable multi-shot mode. Otherwise, KASAN would only + * report the first detected bug and panic the kernel if panic_on_warn + * is enabled. + */ multishot = kasan_save_enable_multi_shot(); - test_status.report_found = false; - test_status.sync_fault = false; - kunit_add_named_resource(test, NULL, NULL, &resource, - "kasan_status", &test_status); + + /* + * Because we want to be able to build the test as a module, we need to + * iterate through all known tracepoints, since the static registration + * won't work here. + */ + for_each_kernel_tracepoint(register_tracepoints, NULL); return 0; } -static void kasan_test_exit(struct kunit *test) +static void kasan_suite_exit(struct kunit_suite *suite) { kasan_restore_multi_shot(multishot); - KUNIT_EXPECT_FALSE(test, test_status.report_found); + for_each_kernel_tracepoint(unregister_tracepoints, NULL); + tracepoint_synchronize_unregister(); +} + +static void kasan_test_exit(struct kunit *test) +{ + KUNIT_EXPECT_FALSE(test, READ_ONCE(test_status.report_found)); } /** @@ -106,11 +143,12 @@ static void kasan_test_exit(struct kunit *test) if (IS_ENABLED(CONFIG_KASAN_HW_TAGS) && \ kasan_sync_fault_possible()) { \ if (READ_ONCE(test_status.report_found) && \ - READ_ONCE(test_status.sync_fault)) \ + !READ_ONCE(test_status.async_fault)) \ kasan_enable_tagging(); \ migrate_enable(); \ } \ WRITE_ONCE(test_status.report_found, false); \ + WRITE_ONCE(test_status.async_fault, false); \ } while (0) #define KASAN_TEST_NEEDS_CONFIG_ON(test, config) do { \ @@ -1447,9 +1485,10 @@ static struct kunit_case kasan_kunit_test_cases[] = { static struct kunit_suite kasan_kunit_test_suite = { .name = "kasan", - .init = kasan_test_init, .test_cases = kasan_kunit_test_cases, .exit = kasan_test_exit, + .suite_init = kasan_suite_init, + .suite_exit = kasan_suite_exit, }; kunit_test_suite(kasan_kunit_test_suite); diff --git a/mm/kasan/report.c b/mm/kasan/report.c index df3602062bfd..31355851a5ec 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -30,8 +30,6 @@ #include <asm/sections.h> -#include <kunit/test.h> - #include "kasan.h" #include "../slab.h" @@ -114,41 +112,12 @@ EXPORT_SYMBOL_GPL(kasan_restore_multi_shot); #endif -#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST) -static void update_kunit_status(bool sync) -{ - struct kunit *test; - struct kunit_resource *resource; - struct kunit_kasan_status *status; - - test = current->kunit_test; - if (!test) - return; - - resource = kunit_find_named_resource(test, "kasan_status"); - if (!resource) { - kunit_set_failure(test); - return; - } - - status = (struct kunit_kasan_status *)resource->data; - WRITE_ONCE(status->report_found, true); - WRITE_ONCE(status->sync_fault, sync); - - kunit_put_resource(resource); -} -#else -static void update_kunit_status(bool sync) { } -#endif - static DEFINE_SPINLOCK(report_lock); static void start_report(unsigned long *flags, bool sync) { /* Respect the /proc/sys/kernel/traceoff_on_warning interface. */ disable_trace_on_warning(); - /* Update status of the currently running KASAN test. */ - update_kunit_status(sync); /* Do not allow LOCKDEP mangling KASAN reports. */ lockdep_off(); /* Make sure we don't end up in loop. */