All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-09 15:32 ` Andrey Konovalov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2021-02-09 15:32 UTC (permalink / raw)
  To: Andrew Morton, Catalin Marinas, Vincenzo Frascino
  Cc: Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Peter Collingbourne, Evgenii Stepanov,
	Branislav Rankov, Kevin Brodsky, Christoph Hellwig, kasan-dev,
	linux-arm-kernel, linux-mm, linux-kernel, Andrey Konovalov

Only export MTE symbols when KASAN-KUnit tests are enabled.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>

---

Andrew, please squash this into:
"arm64: kasan: export MTE symbols for KASAN tests"
---
 arch/arm64/kernel/mte.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index a66c2806fc4d..788ef0c3a25e 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -113,13 +113,17 @@ void mte_enable_kernel(void)
 	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
 	isb();
 }
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(mte_enable_kernel);
+#endif
 
 void mte_set_report_once(bool state)
 {
 	WRITE_ONCE(report_fault_once, state);
 }
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(mte_set_report_once);
+#endif
 
 bool mte_report_once(void)
 {
-- 
2.30.0.478.g8a0d178c01-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-09 15:32 ` Andrey Konovalov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2021-02-09 15:32 UTC (permalink / raw)
  To: Andrew Morton, Catalin Marinas, Vincenzo Frascino
  Cc: Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Peter Collingbourne, Evgenii Stepanov,
	Branislav Rankov, Kevin Brodsky, Christoph Hellwig, kasan-dev,
	linux-arm-kernel, linux-mm, linux-kernel, Andrey Konovalov

Only export MTE symbols when KASAN-KUnit tests are enabled.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>

---

Andrew, please squash this into:
"arm64: kasan: export MTE symbols for KASAN tests"
---
 arch/arm64/kernel/mte.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index a66c2806fc4d..788ef0c3a25e 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -113,13 +113,17 @@ void mte_enable_kernel(void)
 	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
 	isb();
 }
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(mte_enable_kernel);
+#endif
 
 void mte_set_report_once(bool state)
 {
 	WRITE_ONCE(report_fault_once, state);
 }
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(mte_set_report_once);
+#endif
 
 bool mte_report_once(void)
 {
-- 
2.30.0.478.g8a0d178c01-goog



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-09 15:32 ` Andrey Konovalov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2021-02-09 15:32 UTC (permalink / raw)
  To: Andrew Morton, Catalin Marinas, Vincenzo Frascino
  Cc: linux-arm-kernel, Marco Elver, Andrey Konovalov, Kevin Brodsky,
	Will Deacon, Branislav Rankov, kasan-dev, linux-kernel,
	Christoph Hellwig, linux-mm, Alexander Potapenko,
	Evgenii Stepanov, Andrey Ryabinin, Peter Collingbourne,
	Dmitry Vyukov

Only export MTE symbols when KASAN-KUnit tests are enabled.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>

---

Andrew, please squash this into:
"arm64: kasan: export MTE symbols for KASAN tests"
---
 arch/arm64/kernel/mte.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index a66c2806fc4d..788ef0c3a25e 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -113,13 +113,17 @@ void mte_enable_kernel(void)
 	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
 	isb();
 }
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(mte_enable_kernel);
+#endif
 
 void mte_set_report_once(bool state)
 {
 	WRITE_ONCE(report_fault_once, state);
 }
+#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(mte_set_report_once);
+#endif
 
 bool mte_report_once(void)
 {
-- 
2.30.0.478.g8a0d178c01-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
  2021-02-09 15:32 ` Andrey Konovalov
@ 2021-02-09 17:02   ` Catalin Marinas
  -1 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-02-09 17:02 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrew Morton, Vincenzo Frascino, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Peter Collingbourne, Evgenii Stepanov, Branislav Rankov,
	Kevin Brodsky, Christoph Hellwig, kasan-dev, linux-arm-kernel,
	linux-mm, linux-kernel

On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index a66c2806fc4d..788ef0c3a25e 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
>  	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
>  	isb();
>  }
> +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
>  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> +#endif
>  
>  void mte_set_report_once(bool state)
>  {
>  	WRITE_ONCE(report_fault_once, state);
>  }
> +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
>  EXPORT_SYMBOL_GPL(mte_set_report_once);
> +#endif

Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
looks weird to have these #ifdefs in the arch code. Either the
arch-kasan API requires these symbols to be exported to modules or not.
I'm not keen on such kasan internals trickling down into the arch code.

If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
kasan_test_set_report_once) and conditionally compile them based on
KASAN_KUNIT_TEST.

-- 
Catalin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-09 17:02   ` Catalin Marinas
  0 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2021-02-09 17:02 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: linux-arm-kernel, Marco Elver, Kevin Brodsky, Will Deacon,
	Branislav Rankov, kasan-dev, linux-kernel, Christoph Hellwig,
	linux-mm, Alexander Potapenko, Evgenii Stepanov, Andrey Ryabinin,
	Andrew Morton, Vincenzo Frascino, Peter Collingbourne,
	Dmitry Vyukov

On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index a66c2806fc4d..788ef0c3a25e 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
>  	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
>  	isb();
>  }
> +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
>  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> +#endif
>  
>  void mte_set_report_once(bool state)
>  {
>  	WRITE_ONCE(report_fault_once, state);
>  }
> +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
>  EXPORT_SYMBOL_GPL(mte_set_report_once);
> +#endif

Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
looks weird to have these #ifdefs in the arch code. Either the
arch-kasan API requires these symbols to be exported to modules or not.
I'm not keen on such kasan internals trickling down into the arch code.

If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
kasan_test_set_report_once) and conditionally compile them based on
KASAN_KUNIT_TEST.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
  2021-02-09 17:02   ` Catalin Marinas
@ 2021-02-09 18:45     ` Andrew Morton
  -1 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2021-02-09 18:45 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Andrey Konovalov, Vincenzo Frascino, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Peter Collingbourne, Evgenii Stepanov, Branislav Rankov,
	Kevin Brodsky, Christoph Hellwig, kasan-dev, linux-arm-kernel,
	linux-mm, linux-kernel

On Tue, 9 Feb 2021 17:02:56 +0000 Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > index a66c2806fc4d..788ef0c3a25e 100644
> > --- a/arch/arm64/kernel/mte.c
> > +++ b/arch/arm64/kernel/mte.c
> > @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
> >  	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> >  	isb();
> >  }
> > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> >  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> > +#endif
> >  
> >  void mte_set_report_once(bool state)
> >  {
> >  	WRITE_ONCE(report_fault_once, state);
> >  }
> > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> >  EXPORT_SYMBOL_GPL(mte_set_report_once);
> > +#endif
> 
> Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
> looks weird to have these #ifdefs in the arch code. Either the
> arch-kasan API requires these symbols to be exported to modules or not.
> I'm not keen on such kasan internals trickling down into the arch code.
> 
> If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
> wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
> kasan_test_set_report_once) and conditionally compile them based on
> KASAN_KUNIT_TEST.

In other words, the patch's changelog was poor!  It told us what the
patch does (which is often obvious from the code) but it failed to
explain why the patch does what it does.

The same goes for code comments, folks - please explain "why it does
this" rather than "what it does".


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-09 18:45     ` Andrew Morton
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2021-02-09 18:45 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, Marco Elver, Andrey Konovalov, Kevin Brodsky,
	Will Deacon, Branislav Rankov, kasan-dev, linux-kernel,
	Christoph Hellwig, linux-mm, Alexander Potapenko,
	Evgenii Stepanov, Andrey Ryabinin, Vincenzo Frascino,
	Peter Collingbourne, Dmitry Vyukov

On Tue, 9 Feb 2021 17:02:56 +0000 Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > index a66c2806fc4d..788ef0c3a25e 100644
> > --- a/arch/arm64/kernel/mte.c
> > +++ b/arch/arm64/kernel/mte.c
> > @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
> >  	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> >  	isb();
> >  }
> > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> >  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> > +#endif
> >  
> >  void mte_set_report_once(bool state)
> >  {
> >  	WRITE_ONCE(report_fault_once, state);
> >  }
> > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> >  EXPORT_SYMBOL_GPL(mte_set_report_once);
> > +#endif
> 
> Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
> looks weird to have these #ifdefs in the arch code. Either the
> arch-kasan API requires these symbols to be exported to modules or not.
> I'm not keen on such kasan internals trickling down into the arch code.
> 
> If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
> wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
> kasan_test_set_report_once) and conditionally compile them based on
> KASAN_KUNIT_TEST.

In other words, the patch's changelog was poor!  It told us what the
patch does (which is often obvious from the code) but it failed to
explain why the patch does what it does.

The same goes for code comments, folks - please explain "why it does
this" rather than "what it does".


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
  2021-02-09 18:45     ` Andrew Morton
  (?)
@ 2021-02-11 17:30       ` Andrey Konovalov
  -1 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2021-02-11 17:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Peter Collingbourne, Evgenii Stepanov, Branislav Rankov,
	Kevin Brodsky, Christoph Hellwig, kasan-dev, Linux ARM,
	Linux Memory Management List, LKML

On Tue, Feb 9, 2021 at 7:45 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 9 Feb 2021 17:02:56 +0000 Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> > On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> > > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > > index a66c2806fc4d..788ef0c3a25e 100644
> > > --- a/arch/arm64/kernel/mte.c
> > > +++ b/arch/arm64/kernel/mte.c
> > > @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
> > >     sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> > >     isb();
> > >  }
> > > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> > > +#endif
> > >
> > >  void mte_set_report_once(bool state)
> > >  {
> > >     WRITE_ONCE(report_fault_once, state);
> > >  }
> > > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >  EXPORT_SYMBOL_GPL(mte_set_report_once);
> > > +#endif
> >
> > Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
> > looks weird to have these #ifdefs in the arch code. Either the
> > arch-kasan API requires these symbols to be exported to modules or not.
> > I'm not keen on such kasan internals trickling down into the arch code.

Understood.

> > If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
> > wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
> > kasan_test_set_report_once) and conditionally compile them based on
> > KASAN_KUNIT_TEST.

This might be a better approach indeed.

> In other words, the patch's changelog was poor!  It told us what the
> patch does (which is often obvious from the code) but it failed to
> explain why the patch does what it does.
>
> The same goes for code comments, folks - please explain "why it does
> this" rather than "what it does".

I'm sorry, Andrew.

Could you please drop the "arm64: kasan: export MTE symbols for KASAN
tests" patch from the mm tree (but keep the rest of that series)?

I'll post a separate patch with a fix.

Thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-11 17:30       ` Andrey Konovalov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2021-02-11 17:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Peter Collingbourne, Evgenii Stepanov, Branislav Rankov,
	Kevin Brodsky, Christoph Hellwig, kasan-dev, Linux ARM,
	Linux Memory Management List, LKML

On Tue, Feb 9, 2021 at 7:45 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 9 Feb 2021 17:02:56 +0000 Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> > On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> > > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > > index a66c2806fc4d..788ef0c3a25e 100644
> > > --- a/arch/arm64/kernel/mte.c
> > > +++ b/arch/arm64/kernel/mte.c
> > > @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
> > >     sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> > >     isb();
> > >  }
> > > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> > > +#endif
> > >
> > >  void mte_set_report_once(bool state)
> > >  {
> > >     WRITE_ONCE(report_fault_once, state);
> > >  }
> > > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >  EXPORT_SYMBOL_GPL(mte_set_report_once);
> > > +#endif
> >
> > Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
> > looks weird to have these #ifdefs in the arch code. Either the
> > arch-kasan API requires these symbols to be exported to modules or not.
> > I'm not keen on such kasan internals trickling down into the arch code.

Understood.

> > If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
> > wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
> > kasan_test_set_report_once) and conditionally compile them based on
> > KASAN_KUNIT_TEST.

This might be a better approach indeed.

> In other words, the patch's changelog was poor!  It told us what the
> patch does (which is often obvious from the code) but it failed to
> explain why the patch does what it does.
>
> The same goes for code comments, folks - please explain "why it does
> this" rather than "what it does".

I'm sorry, Andrew.

Could you please drop the "arm64: kasan: export MTE symbols for KASAN
tests" patch from the mm tree (but keep the rest of that series)?

I'll post a separate patch with a fix.

Thanks!


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH mm] arm64: kasan: fix MTE symbols exports
@ 2021-02-11 17:30       ` Andrey Konovalov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2021-02-11 17:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linux ARM, Marco Elver, Catalin Marinas, Kevin Brodsky,
	Will Deacon, Branislav Rankov, kasan-dev, LKML,
	Christoph Hellwig, Linux Memory Management List,
	Alexander Potapenko, Evgenii Stepanov, Andrey Ryabinin,
	Vincenzo Frascino, Peter Collingbourne, Dmitry Vyukov

On Tue, Feb 9, 2021 at 7:45 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 9 Feb 2021 17:02:56 +0000 Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> > On Tue, Feb 09, 2021 at 04:32:30PM +0100, Andrey Konovalov wrote:
> > > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > > index a66c2806fc4d..788ef0c3a25e 100644
> > > --- a/arch/arm64/kernel/mte.c
> > > +++ b/arch/arm64/kernel/mte.c
> > > @@ -113,13 +113,17 @@ void mte_enable_kernel(void)
> > >     sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> > >     isb();
> > >  }
> > > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >  EXPORT_SYMBOL_GPL(mte_enable_kernel);
> > > +#endif
> > >
> > >  void mte_set_report_once(bool state)
> > >  {
> > >     WRITE_ONCE(report_fault_once, state);
> > >  }
> > > +#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >  EXPORT_SYMBOL_GPL(mte_set_report_once);
> > > +#endif
> >
> > Do we actually care about exporting them when KASAN_KUNIT_TEST=n? It
> > looks weird to have these #ifdefs in the arch code. Either the
> > arch-kasan API requires these symbols to be exported to modules or not.
> > I'm not keen on such kasan internals trickling down into the arch code.

Understood.

> > If you don't want to export them in the KASAN_KUNIT_TEST=n case, add a
> > wrapper in the kasan built-in code (e.g. kasan_test_enable_tagging,
> > kasan_test_set_report_once) and conditionally compile them based on
> > KASAN_KUNIT_TEST.

This might be a better approach indeed.

> In other words, the patch's changelog was poor!  It told us what the
> patch does (which is often obvious from the code) but it failed to
> explain why the patch does what it does.
>
> The same goes for code comments, folks - please explain "why it does
> this" rather than "what it does".

I'm sorry, Andrew.

Could you please drop the "arm64: kasan: export MTE symbols for KASAN
tests" patch from the mm tree (but keep the rest of that series)?

I'll post a separate patch with a fix.

Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-02-11 18:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 15:32 [PATCH mm] arm64: kasan: fix MTE symbols exports Andrey Konovalov
2021-02-09 15:32 ` Andrey Konovalov
2021-02-09 15:32 ` Andrey Konovalov
2021-02-09 17:02 ` Catalin Marinas
2021-02-09 17:02   ` Catalin Marinas
2021-02-09 18:45   ` Andrew Morton
2021-02-09 18:45     ` Andrew Morton
2021-02-11 17:30     ` Andrey Konovalov
2021-02-11 17:30       ` Andrey Konovalov
2021-02-11 17:30       ` Andrey Konovalov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.