Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Marco Elver]

+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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Steven Rostedt]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Peter Collingbourne]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Peter Collingbourne]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Nick Desaulniers]

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.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Steven Rostedt]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Andrey Konovalov]

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!

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Andrey Konovalov]

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.

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Marco Elver]

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.

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Andrey Konovalov]

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.

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Marco Elver]

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

Re: Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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

Re: [PATCH v3 1/3] kasan: switch kunit tests to console tracepoints

[Paul Heidekrüger]

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