Message ID | ae666d8946f586cfc250205cea4ae0b729d818fa.1609871239.git.andreyknvl@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | kasan: HW_TAGS tests support and fixes | expand |
On Tue, Jan 5, 2021 at 7:28 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST. > > This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f Reviewed-by: Alexander Potapenko <glider@google.com> > KASAN tests consist on two parts: While at it: "consist of".
On Tue, Jan 05, 2021 at 07:27PM +0100, Andrey Konovalov wrote: > Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST. > > This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f Reviewed-by: Marco Elver <elver@google.com> For this patch, as-is. But we could potentially do better in future -- see below. > --- > Documentation/dev-tools/kasan.rst | 6 +++--- > lib/Kconfig.kasan | 2 +- > lib/Makefile | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst > index 26c99852a852..72535816145d 100644 > --- a/Documentation/dev-tools/kasan.rst > +++ b/Documentation/dev-tools/kasan.rst > @@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code. > This allows ``VMAP_STACK`` support on x86, and can simplify support of > architectures that do not have a fixed module region. > > -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE > --------------------------------------------------- > +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST > +---------------------------------------------------- > > KASAN tests consist on two parts: > > @@ -384,7 +384,7 @@ KASAN tests consist on two parts: > automatically in a few different ways, see the instructions below. > > 2. Tests that are currently incompatible with KUnit. Enabled with > -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can > +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can > only be verified manually, by loading the kernel module and inspecting the > kernel log for KASAN reports. > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > index 3091432acb0a..624ae1df7984 100644 > --- a/lib/Kconfig.kasan > +++ b/lib/Kconfig.kasan > @@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST > For more information on KUnit and unit tests in general, please refer > to the KUnit documentation in Documentation/dev-tools/kunit. > > -config TEST_KASAN_MODULE > +config KASAN_MODULE_TEST > tristate "KUnit-incompatible tests of KASAN bug detection capabilities" > depends on m && KASAN && !KASAN_HW_TAGS > help > diff --git a/lib/Makefile b/lib/Makefile > index afeff05fa8c5..122f25d6407e 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o > obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o > CFLAGS_test_kasan.o += -fno-builtin > CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) > -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o > +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o > CFLAGS_test_kasan_module.o += -fno-builtin [1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names Do we eventually want to rename the tests to follow the style recommendation more closely? Option 1: Rename the KUnit test to kasan_test.c? And then also rename test_kasan_module.c -> kasan_module_test.c? Then the file names would be mostly consistent with the config names. Option 2: The style guide [1] also mentions where there are non-KUnit tests around to use _kunit for KUnit test, and _test (or similar) for the non-KUnit test. So here we'd end up with kasan_kunit.c and kasan_test.c. That would get rid of the confusing "module" part. The config variable could either remain CONFIG_KASAN_MODULE_TEST, or simply become CONFIG_KASAN_TEST, since we already have CONFIG_KASAN_KUNIT_TEST to distinguish. But I won't bikeshed further. If you do a v2, I leave it to your judgement to decide what is most appropriate. Thanks, -- Marco
On Tue, Jan 12, 2021 at 9:10 AM Alexander Potapenko <glider@google.com> wrote: > > On Tue, Jan 5, 2021 at 7:28 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > > > Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST. > > > > This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f > Reviewed-by: Alexander Potapenko <glider@google.com> > > > > KASAN tests consist on two parts: > > While at it: "consist of". Will do in v2, thanks!
On Tue, Jan 12, 2021 at 2:33 PM Marco Elver <elver@google.com> wrote: > > On Tue, Jan 05, 2021 at 07:27PM +0100, Andrey Konovalov wrote: > > Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST. > > > > This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f > > Reviewed-by: Marco Elver <elver@google.com> > > For this patch, as-is. But we could potentially do better in future -- > see below. > > > --- > > Documentation/dev-tools/kasan.rst | 6 +++--- > > lib/Kconfig.kasan | 2 +- > > lib/Makefile | 2 +- > > 3 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst > > index 26c99852a852..72535816145d 100644 > > --- a/Documentation/dev-tools/kasan.rst > > +++ b/Documentation/dev-tools/kasan.rst > > @@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code. > > This allows ``VMAP_STACK`` support on x86, and can simplify support of > > architectures that do not have a fixed module region. > > > > -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE > > --------------------------------------------------- > > +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST > > +---------------------------------------------------- > > > > KASAN tests consist on two parts: > > > > @@ -384,7 +384,7 @@ KASAN tests consist on two parts: > > automatically in a few different ways, see the instructions below. > > > > 2. Tests that are currently incompatible with KUnit. Enabled with > > -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can > > +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can > > only be verified manually, by loading the kernel module and inspecting the > > kernel log for KASAN reports. > > > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > > index 3091432acb0a..624ae1df7984 100644 > > --- a/lib/Kconfig.kasan > > +++ b/lib/Kconfig.kasan > > @@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST > > For more information on KUnit and unit tests in general, please refer > > to the KUnit documentation in Documentation/dev-tools/kunit. > > > > -config TEST_KASAN_MODULE > > +config KASAN_MODULE_TEST > > tristate "KUnit-incompatible tests of KASAN bug detection capabilities" > > depends on m && KASAN && !KASAN_HW_TAGS > > help > > diff --git a/lib/Makefile b/lib/Makefile > > index afeff05fa8c5..122f25d6407e 100644 > > --- a/lib/Makefile > > +++ b/lib/Makefile > > @@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o > > obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o > > CFLAGS_test_kasan.o += -fno-builtin > > CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) > > -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o > > +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o > > CFLAGS_test_kasan_module.o += -fno-builtin > > [1] https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-file-and-module-names > > Do we eventually want to rename the tests to follow the style > recommendation more closely? > > Option 1: Rename the KUnit test to kasan_test.c? And then > also rename test_kasan_module.c -> kasan_module_test.c? Then the file > names would be mostly consistent with the config names. > > Option 2: The style guide [1] also mentions where there are non-KUnit > tests around to use _kunit for KUnit test, and _test (or similar) for > the non-KUnit test. So here we'd end up with kasan_kunit.c and > kasan_test.c. That would get rid of the confusing "module" part. The > config variable could either remain CONFIG_KASAN_MODULE_TEST, or simply > become CONFIG_KASAN_TEST, since we already have CONFIG_KASAN_KUNIT_TEST > to distinguish. > > But I won't bikeshed further. If you do a v2, I leave it to your > judgement to decide what is most appropriate. Most tests in lib/ start with test_, so not using that pattern for KASAN tests could be confusing. Maybe we can move them to mm/kasan. Anyway, I won't look into this right now. Thanks!
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst index 26c99852a852..72535816145d 100644 --- a/Documentation/dev-tools/kasan.rst +++ b/Documentation/dev-tools/kasan.rst @@ -374,8 +374,8 @@ unmapped. This will require changes in arch-specific code. This allows ``VMAP_STACK`` support on x86, and can simplify support of architectures that do not have a fixed module region. -CONFIG_KASAN_KUNIT_TEST & CONFIG_TEST_KASAN_MODULE --------------------------------------------------- +CONFIG_KASAN_KUNIT_TEST and CONFIG_KASAN_MODULE_TEST +---------------------------------------------------- KASAN tests consist on two parts: @@ -384,7 +384,7 @@ KASAN tests consist on two parts: automatically in a few different ways, see the instructions below. 2. Tests that are currently incompatible with KUnit. Enabled with -``CONFIG_TEST_KASAN_MODULE`` and can only be run as a module. These tests can +``CONFIG_KASAN_MODULE_TEST`` and can only be run as a module. These tests can only be verified manually, by loading the kernel module and inspecting the kernel log for KASAN reports. diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan index 3091432acb0a..624ae1df7984 100644 --- a/lib/Kconfig.kasan +++ b/lib/Kconfig.kasan @@ -192,7 +192,7 @@ config KASAN_KUNIT_TEST For more information on KUnit and unit tests in general, please refer to the KUnit documentation in Documentation/dev-tools/kunit. -config TEST_KASAN_MODULE +config KASAN_MODULE_TEST tristate "KUnit-incompatible tests of KASAN bug detection capabilities" depends on m && KASAN && !KASAN_HW_TAGS help diff --git a/lib/Makefile b/lib/Makefile index afeff05fa8c5..122f25d6407e 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -68,7 +68,7 @@ obj-$(CONFIG_TEST_IDA) += test_ida.o obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o CFLAGS_test_kasan.o += -fno-builtin CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) -obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o +obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o CFLAGS_test_kasan_module.o += -fno-builtin obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
Rename CONFIG_TEST_KASAN_MODULE to CONFIG_KASAN_MODULE_TEST. This naming is more consistent with the existing CONFIG_KASAN_KUNIT_TEST. Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Link: https://linux-review.googlesource.com/id/Id347dfa5fe8788b7a1a189863e039f409da0ae5f --- Documentation/dev-tools/kasan.rst | 6 +++--- lib/Kconfig.kasan | 2 +- lib/Makefile | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)