[PATCH] kasan: default to inline instrumentation

[PATCH] kasan: default to inline instrumentation

[Paul Heidekrüger]

KASan inline instrumentation can yield up to a 2x performance gain at
the cost of a larger binary.

Make inline instrumentation the default, as suggested in the bug report
below.

When an architecture does not support inline instrumentation, it should
set ARCH_DISABLE_KASAN_INLINE, as done by PowerPC, for instance.

CC: Dmitry Vyukov <dvyukov@google.com>
Reported-by: Andrey Konovalov <andreyknvl@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=203495
Signed-off-by: Paul Heidekrüger <paul.heidekrueger@tum.de>
---
 lib/Kconfig.kasan | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index fdca89c05745..935eda08b1e1 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -134,7 +134,7 @@ endchoice
 choice
 	prompt "Instrumentation type"
 	depends on KASAN_GENERIC || KASAN_SW_TAGS
-	default KASAN_OUTLINE
+	default KASAN_INLINE if !ARCH_DISABLE_KASAN_INLINE
 
 config KASAN_OUTLINE
 	bool "Outline instrumentation"
-- 
2.40.1

Re: [PATCH] kasan: default to inline instrumentation

[Andrey Konovalov]

On Thu, Nov 9, 2023 at 4:51 PM Paul Heidekrüger
<paul.heidekrueger@tum.de> wrote:
>
> KASan inline instrumentation can yield up to a 2x performance gain at
> the cost of a larger binary.
>
> Make inline instrumentation the default, as suggested in the bug report
> below.
>
> When an architecture does not support inline instrumentation, it should
> set ARCH_DISABLE_KASAN_INLINE, as done by PowerPC, for instance.
>
> CC: Dmitry Vyukov <dvyukov@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=203495
> Signed-off-by: Paul Heidekrüger <paul.heidekrueger@tum.de>
> ---
>  lib/Kconfig.kasan | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index fdca89c05745..935eda08b1e1 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -134,7 +134,7 @@ endchoice
>  choice
>         prompt "Instrumentation type"
>         depends on KASAN_GENERIC || KASAN_SW_TAGS
> -       default KASAN_OUTLINE
> +       default KASAN_INLINE if !ARCH_DISABLE_KASAN_INLINE
>
>  config KASAN_OUTLINE
>         bool "Outline instrumentation"
> --
> 2.40.1
>

Acked-by: Andrey Konovalov <andreyknvl@gmail.com>

Thank you for taking care of this!

Re: [PATCH] kasan: default to inline instrumentation

[Marco Elver]

On Thu, 9 Nov 2023 at 22:08, Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> On Thu, Nov 9, 2023 at 4:51 PM Paul Heidekrüger
> <paul.heidekrueger@tum.de> wrote:
> >
> > KASan inline instrumentation can yield up to a 2x performance gain at
> > the cost of a larger binary.
> >
> > Make inline instrumentation the default, as suggested in the bug report
> > below.
> >
> > When an architecture does not support inline instrumentation, it should
> > set ARCH_DISABLE_KASAN_INLINE, as done by PowerPC, for instance.
> >
> > CC: Dmitry Vyukov <dvyukov@google.com>
> > Reported-by: Andrey Konovalov <andreyknvl@gmail.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=203495
> > Signed-off-by: Paul Heidekrüger <paul.heidekrueger@tum.de>
> > ---
> >  lib/Kconfig.kasan | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> > index fdca89c05745..935eda08b1e1 100644
> > --- a/lib/Kconfig.kasan
> > +++ b/lib/Kconfig.kasan
> > @@ -134,7 +134,7 @@ endchoice
> >  choice
> >         prompt "Instrumentation type"
> >         depends on KASAN_GENERIC || KASAN_SW_TAGS
> > -       default KASAN_OUTLINE
> > +       default KASAN_INLINE if !ARCH_DISABLE_KASAN_INLINE
> >
> >  config KASAN_OUTLINE
> >         bool "Outline instrumentation"
> > --
> > 2.40.1
> >
>
> Acked-by: Andrey Konovalov <andreyknvl@gmail.com>
>
> Thank you for taking care of this!

Reviewed-by: Marco Elver <elver@google.com>

+Cc Andrew (get_maintainers.pl doesn't add Andrew automatically for
KASAN sources in lib/)

Thanks,
-- Marco

Re: [PATCH] kasan: default to inline instrumentation

[Andrew Morton]

On Tue, 14 Nov 2023 12:00:49 +0100 Marco Elver <elver@google.com> wrote:

> +Cc Andrew (get_maintainers.pl doesn't add Andrew automatically for
> KASAN sources in lib/)

Did I do this right?


From: Andrew Morton <akpm@linux-foundation.org>
Subject: MAINTAINERS: add Andrew Morton for lib/*
Date: Tue Nov 14 03:02:04 PM PST 2023

Add myself as the fallthough maintainer for material under lib/

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 MAINTAINERS |    7 +++++++
 1 file changed, 7 insertions(+)

--- a/MAINTAINERS~a
+++ a/MAINTAINERS
@@ -12209,6 +12209,13 @@ F:	include/linux/nd.h
 F:	include/uapi/linux/ndctl.h
 F:	tools/testing/nvdimm/
 
+LIBRARY CODE
+M:	Andrew Morton <akpm@linux-foundation.org>
+L:	linux-kernel@vger.kernel.org
+S:	Supported
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable
+F:	lib/*
+
 LICENSES and SPDX stuff
 M:	Thomas Gleixner <tglx@linutronix.de>
 M:	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
_

Re: [PATCH] kasan: default to inline instrumentation

[Joe Perches]

On Tue, 2023-11-14 at 15:11 -0800, Andrew Morton wrote:
> On Tue, 14 Nov 2023 12:00:49 +0100 Marco Elver <elver@google.com> wrote:
> 
> > +Cc Andrew (get_maintainers.pl doesn't add Andrew automatically for
> > KASAN sources in lib/)
> 
> Did I do this right?
> 
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: MAINTAINERS: add Andrew Morton for lib/*
> Date: Tue Nov 14 03:02:04 PM PST 2023
> 
> Add myself as the fallthough maintainer for material under lib/
> 
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  MAINTAINERS |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> --- a/MAINTAINERS~a
> +++ a/MAINTAINERS
> @@ -12209,6 +12209,13 @@ F:	include/linux/nd.h
>  F:	include/uapi/linux/ndctl.h
>  F:	tools/testing/nvdimm/
>  
> +LIBRARY CODE
> +M:	Andrew Morton <akpm@linux-foundation.org>
> +L:	linux-kernel@vger.kernel.org
> +S:	Supported

Dunno.

There are a lot of already specifically maintained or
supported files in lib/

Maybe be a reviewer?

Re: [PATCH] kasan: default to inline instrumentation

[Marco Elver]

On Wed, 15 Nov 2023 at 06:38, Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2023-11-14 at 15:11 -0800, Andrew Morton wrote:
> > On Tue, 14 Nov 2023 12:00:49 +0100 Marco Elver <elver@google.com> wrote:
> >
> > > +Cc Andrew (get_maintainers.pl doesn't add Andrew automatically for
> > > KASAN sources in lib/)
> >
> > Did I do this right?

If the signal to noise ratio is acceptable, something like that could
be helpful. New contributors like Paul in this case may have an easier
time, if none of the reviewers spot the missing Cc.

However, folks familiar with subsystems that also have bits in lib/
(or elsewhere) know to Cc you. It worked in this case.

Thanks,
-- Marco

> > From: Andrew Morton <akpm@linux-foundation.org>
> > Subject: MAINTAINERS: add Andrew Morton for lib/*
> > Date: Tue Nov 14 03:02:04 PM PST 2023
> >
> > Add myself as the fallthough maintainer for material under lib/
> >
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >
> >  MAINTAINERS |    7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > --- a/MAINTAINERS~a
> > +++ a/MAINTAINERS
> > @@ -12209,6 +12209,13 @@ F:   include/linux/nd.h
> >  F:   include/uapi/linux/ndctl.h
> >  F:   tools/testing/nvdimm/
> >
> > +LIBRARY CODE
> > +M:   Andrew Morton <akpm@linux-foundation.org>
> > +L:   linux-kernel@vger.kernel.org
> > +S:   Supported
>
> Dunno.
>
> There are a lot of already specifically maintained or
> supported files in lib/
>
> Maybe be a reviewer?
>

Re: [PATCH] kasan: default to inline instrumentation

[Andrew Morton]

On Tue, 14 Nov 2023 21:38:50 -0800 Joe Perches <joe@perches.com> wrote:

> > +LIBRARY CODE
> > +M:	Andrew Morton <akpm@linux-foundation.org>
> > +L:	linux-kernel@vger.kernel.org
> > +S:	Supported
> 
> Dunno.
> 
> There are a lot of already specifically maintained or
> supported files in lib/

That's OK.  I'll get printed out along with the existing list of
maintainers, if any.

> Maybe be a reviewer?

Would that alter the get_maintainer output in any way?

I suppose I could list each file individually, but I'm not sure what
that would gain.

btw, I see MAINTAINERS lists non-existent file[s] (lib/fw_table.c). 
Maybe someone has a script to check...

Re: [PATCH] kasan: default to inline instrumentation

[Joe Perches]

On Wed, 2023-11-15 at 14:34 -0800, Andrew Morton wrote:
> On Tue, 14 Nov 2023 21:38:50 -0800 Joe Perches <joe@perches.com> wrote:
> 
> > > +LIBRARY CODE
> > > +M:	Andrew Morton <akpm@linux-foundation.org>
> > > +L:	linux-kernel@vger.kernel.org
> > > +S:	Supported
> > 
> > Dunno.
> > 
> > There are a lot of already specifically maintained or
> > supported files in lib/
> 
> That's OK.  I'll get printed out along with the existing list of
> maintainers, if any.
> 
> > Maybe be a reviewer?
> 
> Would that alter the get_maintainer output in any way?

Not really.  It would allow someone to avoid cc'ing reviewers
and not maintainers though.

Perhaps change the
	S:	Supported
to something like
	S:	Supported for the files otherwise not supported

> I suppose I could list each file individually, but I'm not sure what
> that would gain.
> 
> btw, I see MAINTAINERS lists non-existent file[s] (lib/fw_table.c). 
> Maybe someone has a script to check...

--self-test works

$ ./scripts/get_maintainer.pl --self-test=patterns
./MAINTAINERS:3653: warning: no file matches	F:	Documentation/devicetree/bindings/iio/imu/bosch,bma400.yaml
./MAINTAINERS:6126: warning: no file matches	F:	Documentation/devicetree/bindings/watchdog/da90??-wdt.txt
./MAINTAINERS:10342: warning: no file matches	F:	drivers/iio/light/gain-time-scale-helper.c
./MAINTAINERS:10343: warning: no file matches	F:	drivers/iio/light/gain-time-scale-helper.h
./MAINTAINERS:22062: warning: no file matches	F:	arch/arm/boot/dts/imx*mba*.dts*
./MAINTAINERS:22063: warning: no file matches	F:	arch/arm/boot/dts/imx*tqma*.dts*
./MAINTAINERS:22064: warning: no file matches	F:	arch/arm/boot/dts/mba*.dtsi

and: see commit a103f46633fdcddc2aaca506420f177e8803a2bd

$ git log --stat -1 a103f46633fdcddc2aaca506420f177e8803a2bd
commit a103f46633fdcddc2aaca506420f177e8803a2bd
Author: Dave Jiang <dave.jiang@intel.com>
Date:   Thu Oct 12 11:53:54 2023 -0700

    acpi: Move common tables helper functions to common lib
    
    Some of the routines in ACPI driver/acpi/tables.c can be shared with
    parsing CDAT. CDAT is a device-provided data structure that is formatted
    similar to a platform provided ACPI table. CDAT is used by CXL and can
    exist on platforms that do not use ACPI. Split out the common routine
    from ACPI to accommodate platforms that do not support ACPI and move that
    to /lib. The common routines can be built outside of ACPI if
    FIRMWARE_TABLES is selected.
    
    Link: https://lore.kernel.org/linux-cxl/CAJZ5v0jipbtTNnsA0-o5ozOk8ZgWnOg34m34a9pPenTyRLj=6A@mail.gmail.com/
    Suggested-by: "Rafael J. Wysocki" <rafael@kernel.org>
    Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
    Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
    Signed-off-by: Dave Jiang <dave.jiang@intel.com>
    Acked-by: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
    Link: https://lore.kernel.org/r/169713683430.2205276.17899451119920103445.stgit@djiang5-mobl3
    Signed-off-by: Dan Williams <dan.j.williams@intel.com>

 MAINTAINERS              |   2 ++
 drivers/acpi/Kconfig     |   1 +
 drivers/acpi/tables.c    | 173 -------------------------------------------------------------------------------------------------------
 include/linux/acpi.h     |  42 +++++++------------------
 include/linux/fw_table.h |  43 ++++++++++++++++++++++++++
 lib/Kconfig              |   3 ++
 lib/Makefile             |   2 ++
 lib/fw_table.c           | 189 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 251 insertions(+), 204 deletions(-)

Re: [PATCH] kasan: default to inline instrumentation

[Andrew Morton]

On Wed, 15 Nov 2023 14:48:38 -0800 Joe Perches <joe@perches.com> wrote:

> > Would that alter the get_maintainer output in any way?
> 
> Not really.  It would allow someone to avoid cc'ing reviewers
> and not maintainers though.
> 
> Perhaps change the
> 	S:	Supported
> to something like
> 	S:	Supported for the files otherwise not supported

That's OK.  I actually like to see what's going on in lib/.  Sometimes
I discover things in there that surprise me...