* [PATCH v10 1/1] arm64: Relax Documentation/arm64/tagged-pointers.rst
From: Catalin Marinas @ 2019-08-23 16:37 UTC (permalink / raw)
To: linux-arm-kernel, linux-mm
Cc: Andrew Morton, Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Szabolcs Nagy, Kevin Brodsky, Dave P Martin, Dave Hansen,
linux-doc, linux-arch, Will Deacon
In-Reply-To: <20190823163717.19569-1-catalin.marinas@arm.com>
From: Vincenzo Frascino <vincenzo.frascino@arm.com>
On AArch64 the TCR_EL1.TBI0 bit is set by default, allowing userspace
(EL0) to perform memory accesses through 64-bit pointers with a non-zero
top byte. However, such pointers were not allowed at the user-kernel
syscall ABI boundary.
With the Tagged Address ABI patchset, it is now possible to pass tagged
pointers to the syscalls. Relax the requirements described in
tagged-pointers.rst to be compliant with the behaviours guaranteed by
the AArch64 Tagged Address ABI.
Cc: Will Deacon <will.deacon@arm.com>
Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: Kevin Brodsky <kevin.brodsky@arm.com>
Acked-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
Documentation/arm64/tagged-pointers.rst | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index 2acdec3ebbeb..eab4323609b9 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst
@@ -20,7 +20,9 @@ Passing tagged addresses to the kernel
--------------------------------------
All interpretation of userspace memory addresses by the kernel assumes
-an address tag of 0x00.
+an address tag of 0x00, unless the application enables the AArch64
+Tagged Address ABI explicitly
+(Documentation/arm64/tagged-address-abi.rst).
This includes, but is not limited to, addresses found in:
@@ -33,13 +35,15 @@ This includes, but is not limited to, addresses found in:
- the frame pointer (x29) and frame records, e.g. when interpreting
them to generate a backtrace or call graph.
-Using non-zero address tags in any of these locations may result in an
-error code being returned, a (fatal) signal being raised, or other modes
-of failure.
+Using non-zero address tags in any of these locations when the
+userspace application did not enable the AArch64 Tagged Address ABI may
+result in an error code being returned, a (fatal) signal being raised,
+or other modes of failure.
-For these reasons, passing non-zero address tags to the kernel via
-system calls is forbidden, and using a non-zero address tag for sp is
-strongly discouraged.
+For these reasons, when the AArch64 Tagged Address ABI is disabled,
+passing non-zero address tags to the kernel via system calls is
+forbidden, and using a non-zero address tag for sp is strongly
+discouraged.
Programs maintaining a frame pointer and frame records that use non-zero
address tags may suffer impaired or inaccurate debug and profiling
@@ -59,6 +63,9 @@ be preserved.
The architecture prevents the use of a tagged PC, so the upper byte will
be set to a sign-extension of bit 55 on exception return.
+This behaviour is maintained when the AArch64 Tagged Address ABI is
+enabled.
+
Other considerations
--------------------
^ permalink raw reply related
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: Brendan Higgins @ 2019-08-23 16:48 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg
In-Reply-To: <7f2c8908-75f6-b793-7113-ad57c51777ce@kernel.org>
On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
>
> Hi Brendan,
>
> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> > Add core facilities for defining unit tests; this provides a common way
> > to define test cases, functions that execute code which is under test
> > and determine whether the code under test behaves as expected; this also
> > provides a way to group together related test cases in test suites (here
> > we call them test_modules).
> >
> > Just define test cases and how to execute them for now; setting
> > expectations on code will be defined later.
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
> > kunit/Kconfig | 17 ++++
> > kunit/Makefile | 1 +
> > kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 388 insertions(+)
> > create mode 100644 include/kunit/test.h
> > create mode 100644 kunit/Kconfig
> > create mode 100644 kunit/Makefile
> > create mode 100644 kunit/test.c
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > new file mode 100644
> > index 0000000000000..e0b34acb9ee4e
> > --- /dev/null
> > +++ b/include/kunit/test.h
> > @@ -0,0 +1,179 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Base unit test (KUnit) API.
> > + *
> > + * Copyright (C) 2019, Google LLC.
> > + * Author: Brendan Higgins <brendanhiggins@google.com>
> > + */
> > +
> > +#ifndef _KUNIT_TEST_H
> > +#define _KUNIT_TEST_H
> > +
> > +#include <linux/types.h>
> > +
> > +struct kunit;
> > +
> > +/**
> > + * struct kunit_case - represents an individual test case.
> > + * @run_case: the function representing the actual test case.
> > + * @name: the name of the test case.
> > + *
> > + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> > + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
> > + * test case is associated with a &struct kunit_suite and will be run after the
> > + * suite's init function and followed by the suite's exit function.
> > + *
> > + * A test case should be static and should only be created with the KUNIT_CASE()
> > + * macro; additionally, every array of test cases should be terminated with an
> > + * empty test case.
> > + *
> > + * Example:
>
> Can you fix these line continuations. It makes it very hard to read.
> Sorry for this late comment. These comments lines are longer than 80
> and wrap.
None of the lines in this commit are over 80 characters in column
width. Some are exactly 80 characters (like above).
My guess is that you are seeing the diff added text (+ ), which when
you add that to a line which is exactly 80 char in length ends up
being over 80 char in email. If you apply the patch you will see that
they are only 80 chars.
>
> There are several comment lines in the file that are way too long.
Note that checkpatch also does not complain about any over 80 char
lines in this file.
Sorry if I am misunderstanding what you are trying to tell me. Please
confirm either way.
Thanks
^ permalink raw reply
* Re: [PATCH v14 09/18] kunit: test: add support for test abort
From: Brendan Higgins @ 2019-08-23 16:56 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg
In-Reply-To: <ae6722ce-80ac-5840-5c4b-6f6726e4239d@kernel.org>
On Fri, Aug 23, 2019 at 8:36 AM shuah <shuah@kernel.org> wrote:
>
> Hi Brendan,
>
> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> > Add support for aborting/bailing out of test cases, which is needed for
> > implementing assertions.
> >
> > An assertion is like an expectation, but bails out of the test case
> > early if the assertion is not met. The idea with assertions is that you
> > use them to state all the preconditions for your test. Logically
> > speaking, these are the premises of the test case, so if a premise isn't
> > true, there is no point in continuing the test case because there are no
> > conclusions that can be drawn without the premises. Whereas, the
> > expectation is the thing you are trying to prove.
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > include/kunit/test.h | 2 +
> > include/kunit/try-catch.h | 75 +++++++++++++++++++++
> > kunit/Makefile | 3 +-
> > kunit/test.c | 137 +++++++++++++++++++++++++++++++++-----
> > kunit/try-catch.c | 118 ++++++++++++++++++++++++++++++++
> > 5 files changed, 319 insertions(+), 16 deletions(-)
> > create mode 100644 include/kunit/try-catch.h
> > create mode 100644 kunit/try-catch.c
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 6917b186b737a..390ce02f717b6 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -10,6 +10,7 @@
> > #define _KUNIT_TEST_H
> >
> > #include <kunit/assert.h>
> > +#include <kunit/try-catch.h>
> > #include <linux/kernel.h>
> > #include <linux/slab.h>
> > #include <linux/types.h>
> > @@ -167,6 +168,7 @@ struct kunit {
> >
> > /* private: internal use only. */
> > const char *name; /* Read only after initialization! */
> > + struct kunit_try_catch try_catch;
> > /*
> > * success starts as true, and may only be set to false during a test
> > * case; thus, it is safe to update this across multiple threads using
> > diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
> > new file mode 100644
> > index 0000000000000..404f336cbdc85
> > --- /dev/null
> > +++ b/include/kunit/try-catch.h
> > @@ -0,0 +1,75 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * An API to allow a function, that may fail, to be executed, and recover in a
> > + * controlled manner.
> > + *
> > + * Copyright (C) 2019, Google LLC.
> > + * Author: Brendan Higgins <brendanhiggins@google.com>
> > + */
> > +
> > +#ifndef _KUNIT_TRY_CATCH_H
> > +#define _KUNIT_TRY_CATCH_H
> > +
> > +#include <linux/types.h>
> > +
> > +typedef void (*kunit_try_catch_func_t)(void *);
> > +
> > +struct completion;
> > +struct kunit;
> > +
> > +/**
> > + * struct kunit_try_catch - provides a generic way to run code which might fail.
> > + * @test: The test case that is currently being executed.
> > + * @try_completion: Completion that the control thread waits on while test runs.
> > + * @try_result: Contains any errno obtained while running test case.
> > + * @try: The function, the test case, to attempt to run.
> > + * @catch: The function called if @try bails out.
> > + * @context: used to pass user data to the try and catch functions.
> > + *
> > + * kunit_try_catch provides a generic, architecture independent way to execute
> > + * an arbitrary function of type kunit_try_catch_func_t which may bail out by
> > + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
> > + * is stopped at the site of invocation and @catch is called.
> > + *
> > + * struct kunit_try_catch provides a generic interface for the functionality
> > + * needed to implement kunit->abort() which in turn is needed for implementing
> > + * assertions. Assertions allow stating a precondition for a test simplifying
> > + * how test cases are written and presented.
> > + *
> > + * Assertions are like expectations, except they abort (call
> > + * kunit_try_catch_throw()) when the specified condition is not met. This is
> > + * useful when you look at a test case as a logical statement about some piece
> > + * of code, where assertions are the premises for the test case, and the
> > + * conclusion is a set of predicates, rather expectations, that must all be
> > + * true. If your premises are violated, it does not makes sense to continue.
> > + */
> > +struct kunit_try_catch {
> > + /* private: internal use only. */
> > + struct kunit *test;
> > + struct completion *try_completion;
> > + int try_result;
> > + kunit_try_catch_func_t try;
> > + kunit_try_catch_func_t catch;
> > + void *context;
> > +};
> > +
> > +void kunit_try_catch_init(struct kunit_try_catch *try_catch,
> > + struct kunit *test,
> > + kunit_try_catch_func_t try,
> > + kunit_try_catch_func_t catch);
> > +
> > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
> > +
> > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
> > +
> > +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
> > +{
> > + return try_catch->try_result;
> > +}
> > +
> > +/*
> > + * Exposed for testing only.
> > + */
> > +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
> > +
> > +#endif /* _KUNIT_TRY_CATCH_H */
> > diff --git a/kunit/Makefile b/kunit/Makefile
> > index 4e46450bcb3a8..c9176c9c578c6 100644
> > --- a/kunit/Makefile
> > +++ b/kunit/Makefile
> > @@ -1,6 +1,7 @@
> > obj-$(CONFIG_KUNIT) += test.o \
> > string-stream.o \
> > - assert.o
> > + assert.o \
> > + try-catch.o
> >
> > obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
> >
> > diff --git a/kunit/test.c b/kunit/test.c
> > index 3cbceb34b3b36..ded9895143209 100644
> > --- a/kunit/test.c
> > +++ b/kunit/test.c
> > @@ -7,7 +7,9 @@
> > */
> >
> > #include <kunit/test.h>
> > +#include <kunit/try-catch.h>
> > #include <linux/kernel.h>
> > +#include <linux/sched/debug.h>
> >
> > static void kunit_set_failure(struct kunit *test)
> > {
> > @@ -162,6 +164,19 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert)
> > WARN_ON(string_stream_destroy(stream));
> > }
> >
> > +static void __noreturn kunit_abort(struct kunit *test)
> > +{
> > + kunit_try_catch_throw(&test->try_catch); /* Does not return. */
> > +
> > + /*
> > + * Throw could not abort from test.
> > + *
> > + * XXX: we should never reach this line! As kunit_try_catch_throw is
> > + * marked __noreturn.
> > + */
> > + BUG();
>
>
> I recall discussion on this. What's the point in keeping thie
> BUG() around when it doesn't even reach? It can even be a
> WARN_ON() in that case right?
Originally I had BUG() here, and Frank (I think it was Frank, sorry it
was a while ago) told me it should be WARN_ON(). In v12 Stephen told
me it should be BUG(), and nobody objected so I went back to making it
a BUG() (note I also mentioned this change on the cover letter of v13
and still no one objected).
You can see the most recent discussion with Stephen here:
https://lore.kernel.org/linux-kselftest/20190812182421.141150-1-brendanhiggins@google.com/T/#mb108adc525092dd72fd3368ecae34251bad29edf
Cheers
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: shuah @ 2019-08-23 17:05 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg, shuah
In-Reply-To: <CAFd5g44mRK9t4f58i_YMEt=e9RTxwrrhFY_V2LW_E7bUwR3cdg@mail.gmail.com>
On 8/23/19 10:48 AM, Brendan Higgins wrote:
> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
>>
>> Hi Brendan,
>>
>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
>>> Add core facilities for defining unit tests; this provides a common way
>>> to define test cases, functions that execute code which is under test
>>> and determine whether the code under test behaves as expected; this also
>>> provides a way to group together related test cases in test suites (here
>>> we call them test_modules).
>>>
>>> Just define test cases and how to execute them for now; setting
>>> expectations on code will be defined later.
>>>
>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>>> ---
>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
>>> kunit/Kconfig | 17 ++++
>>> kunit/Makefile | 1 +
>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 388 insertions(+)
>>> create mode 100644 include/kunit/test.h
>>> create mode 100644 kunit/Kconfig
>>> create mode 100644 kunit/Makefile
>>> create mode 100644 kunit/test.c
>>>
>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>> new file mode 100644
>>> index 0000000000000..e0b34acb9ee4e
>>> --- /dev/null
>>> +++ b/include/kunit/test.h
>>> @@ -0,0 +1,179 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Base unit test (KUnit) API.
>>> + *
>>> + * Copyright (C) 2019, Google LLC.
>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
>>> + */
>>> +
>>> +#ifndef _KUNIT_TEST_H
>>> +#define _KUNIT_TEST_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +struct kunit;
>>> +
>>> +/**
>>> + * struct kunit_case - represents an individual test case.
>>> + * @run_case: the function representing the actual test case.
>>> + * @name: the name of the test case.
>>> + *
>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
>>> + * test case is associated with a &struct kunit_suite and will be run after the
>>> + * suite's init function and followed by the suite's exit function.
>>> + *
>>> + * A test case should be static and should only be created with the KUNIT_CASE()
>>> + * macro; additionally, every array of test cases should be terminated with an
>>> + * empty test case.
>>> + *
>>> + * Example:
>>
>> Can you fix these line continuations. It makes it very hard to read.
>> Sorry for this late comment. These comments lines are longer than 80
>> and wrap.
>
> None of the lines in this commit are over 80 characters in column
> width. Some are exactly 80 characters (like above).
>
> My guess is that you are seeing the diff added text (+ ), which when
> you add that to a line which is exactly 80 char in length ends up
> being over 80 char in email. If you apply the patch you will see that
> they are only 80 chars.
>
>>
>> There are several comment lines in the file that are way too long.
>
> Note that checkpatch also does not complain about any over 80 char
> lines in this file.
>
> Sorry if I am misunderstanding what you are trying to tell me. Please
> confirm either way.
>
WARNING: Avoid unnecessary line continuations
#258: FILE: include/kunit/test.h:137:
+ */ \
total: 0 errors, 2 warnings, 388 lines checked
Go ahead fix these. It appears there are few lines that either longer
than 80. In general, I keep them around 75, so it is easier read.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v14 09/18] kunit: test: add support for test abort
From: shuah @ 2019-08-23 17:07 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg, shuah
In-Reply-To: <CAFd5g451bF2-RGZ3wT9gO1FOu+8e8yQO9evvxtQQ9Vkh4UUhVQ@mail.gmail.com>
On 8/23/19 10:56 AM, Brendan Higgins wrote:
> On Fri, Aug 23, 2019 at 8:36 AM shuah <shuah@kernel.org> wrote:
>>
>> Hi Brendan,
>>
>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
>>> Add support for aborting/bailing out of test cases, which is needed for
>>> implementing assertions.
>>>
>>> An assertion is like an expectation, but bails out of the test case
>>> early if the assertion is not met. The idea with assertions is that you
>>> use them to state all the preconditions for your test. Logically
>>> speaking, these are the premises of the test case, so if a premise isn't
>>> true, there is no point in continuing the test case because there are no
>>> conclusions that can be drawn without the premises. Whereas, the
>>> expectation is the thing you are trying to prove.
>>>
>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>>> ---
>>> include/kunit/test.h | 2 +
>>> include/kunit/try-catch.h | 75 +++++++++++++++++++++
>>> kunit/Makefile | 3 +-
>>> kunit/test.c | 137 +++++++++++++++++++++++++++++++++-----
>>> kunit/try-catch.c | 118 ++++++++++++++++++++++++++++++++
>>> 5 files changed, 319 insertions(+), 16 deletions(-)
>>> create mode 100644 include/kunit/try-catch.h
>>> create mode 100644 kunit/try-catch.c
>>>
>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>> index 6917b186b737a..390ce02f717b6 100644
>>> --- a/include/kunit/test.h
>>> +++ b/include/kunit/test.h
>>> @@ -10,6 +10,7 @@
>>> #define _KUNIT_TEST_H
>>>
>>> #include <kunit/assert.h>
>>> +#include <kunit/try-catch.h>
>>> #include <linux/kernel.h>
>>> #include <linux/slab.h>
>>> #include <linux/types.h>
>>> @@ -167,6 +168,7 @@ struct kunit {
>>>
>>> /* private: internal use only. */
>>> const char *name; /* Read only after initialization! */
>>> + struct kunit_try_catch try_catch;
>>> /*
>>> * success starts as true, and may only be set to false during a test
>>> * case; thus, it is safe to update this across multiple threads using
>>> diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
>>> new file mode 100644
>>> index 0000000000000..404f336cbdc85
>>> --- /dev/null
>>> +++ b/include/kunit/try-catch.h
>>> @@ -0,0 +1,75 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * An API to allow a function, that may fail, to be executed, and recover in a
>>> + * controlled manner.
>>> + *
>>> + * Copyright (C) 2019, Google LLC.
>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
>>> + */
>>> +
>>> +#ifndef _KUNIT_TRY_CATCH_H
>>> +#define _KUNIT_TRY_CATCH_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +typedef void (*kunit_try_catch_func_t)(void *);
>>> +
>>> +struct completion;
>>> +struct kunit;
>>> +
>>> +/**
>>> + * struct kunit_try_catch - provides a generic way to run code which might fail.
>>> + * @test: The test case that is currently being executed.
>>> + * @try_completion: Completion that the control thread waits on while test runs.
>>> + * @try_result: Contains any errno obtained while running test case.
>>> + * @try: The function, the test case, to attempt to run.
>>> + * @catch: The function called if @try bails out.
>>> + * @context: used to pass user data to the try and catch functions.
>>> + *
>>> + * kunit_try_catch provides a generic, architecture independent way to execute
>>> + * an arbitrary function of type kunit_try_catch_func_t which may bail out by
>>> + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
>>> + * is stopped at the site of invocation and @catch is called.
>>> + *
>>> + * struct kunit_try_catch provides a generic interface for the functionality
>>> + * needed to implement kunit->abort() which in turn is needed for implementing
>>> + * assertions. Assertions allow stating a precondition for a test simplifying
>>> + * how test cases are written and presented.
>>> + *
>>> + * Assertions are like expectations, except they abort (call
>>> + * kunit_try_catch_throw()) when the specified condition is not met. This is
>>> + * useful when you look at a test case as a logical statement about some piece
>>> + * of code, where assertions are the premises for the test case, and the
>>> + * conclusion is a set of predicates, rather expectations, that must all be
>>> + * true. If your premises are violated, it does not makes sense to continue.
>>> + */
>>> +struct kunit_try_catch {
>>> + /* private: internal use only. */
>>> + struct kunit *test;
>>> + struct completion *try_completion;
>>> + int try_result;
>>> + kunit_try_catch_func_t try;
>>> + kunit_try_catch_func_t catch;
>>> + void *context;
>>> +};
>>> +
>>> +void kunit_try_catch_init(struct kunit_try_catch *try_catch,
>>> + struct kunit *test,
>>> + kunit_try_catch_func_t try,
>>> + kunit_try_catch_func_t catch);
>>> +
>>> +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
>>> +
>>> +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
>>> +
>>> +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
>>> +{
>>> + return try_catch->try_result;
>>> +}
>>> +
>>> +/*
>>> + * Exposed for testing only.
>>> + */
>>> +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
>>> +
>>> +#endif /* _KUNIT_TRY_CATCH_H */
>>> diff --git a/kunit/Makefile b/kunit/Makefile
>>> index 4e46450bcb3a8..c9176c9c578c6 100644
>>> --- a/kunit/Makefile
>>> +++ b/kunit/Makefile
>>> @@ -1,6 +1,7 @@
>>> obj-$(CONFIG_KUNIT) += test.o \
>>> string-stream.o \
>>> - assert.o
>>> + assert.o \
>>> + try-catch.o
>>>
>>> obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
>>>
>>> diff --git a/kunit/test.c b/kunit/test.c
>>> index 3cbceb34b3b36..ded9895143209 100644
>>> --- a/kunit/test.c
>>> +++ b/kunit/test.c
>>> @@ -7,7 +7,9 @@
>>> */
>>>
>>> #include <kunit/test.h>
>>> +#include <kunit/try-catch.h>
>>> #include <linux/kernel.h>
>>> +#include <linux/sched/debug.h>
>>>
>>> static void kunit_set_failure(struct kunit *test)
>>> {
>>> @@ -162,6 +164,19 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert)
>>> WARN_ON(string_stream_destroy(stream));
>>> }
>>>
>>> +static void __noreturn kunit_abort(struct kunit *test)
>>> +{
>>> + kunit_try_catch_throw(&test->try_catch); /* Does not return. */
>>> +
>>> + /*
>>> + * Throw could not abort from test.
>>> + *
>>> + * XXX: we should never reach this line! As kunit_try_catch_throw is
>>> + * marked __noreturn.
>>> + */
>>> + BUG();
>>
>>
>> I recall discussion on this. What's the point in keeping thie
>> BUG() around when it doesn't even reach? It can even be a
>> WARN_ON() in that case right?
>
> Originally I had BUG() here, and Frank (I think it was Frank, sorry it
> was a while ago) told me it should be WARN_ON(). In v12 Stephen told
> me it should be BUG(), and nobody objected so I went back to making it
> a BUG() (note I also mentioned this change on the cover letter of v13
> and still no one objected).
>
Yeah. Sorry for the confusing advice. WARN_ON() or nothing is the right
thing here. I have been cleaning BUG() and WARN_ON() that aren't needed.
I would just delete BUG all together.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: Brendan Higgins @ 2019-08-23 17:27 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg
In-Reply-To: <4513d9f3-a69b-a9a4-768b-86c2962b62e0@kernel.org>
On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
>
> On 8/23/19 10:48 AM, Brendan Higgins wrote:
> > On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
> >>
> >> Hi Brendan,
> >>
> >> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> >>> Add core facilities for defining unit tests; this provides a common way
> >>> to define test cases, functions that execute code which is under test
> >>> and determine whether the code under test behaves as expected; this also
> >>> provides a way to group together related test cases in test suites (here
> >>> we call them test_modules).
> >>>
> >>> Just define test cases and how to execute them for now; setting
> >>> expectations on code will be defined later.
> >>>
> >>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> >>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> >>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> >>> ---
> >>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
> >>> kunit/Kconfig | 17 ++++
> >>> kunit/Makefile | 1 +
> >>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
> >>> 4 files changed, 388 insertions(+)
> >>> create mode 100644 include/kunit/test.h
> >>> create mode 100644 kunit/Kconfig
> >>> create mode 100644 kunit/Makefile
> >>> create mode 100644 kunit/test.c
> >>>
> >>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> >>> new file mode 100644
> >>> index 0000000000000..e0b34acb9ee4e
> >>> --- /dev/null
> >>> +++ b/include/kunit/test.h
> >>> @@ -0,0 +1,179 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +/*
> >>> + * Base unit test (KUnit) API.
> >>> + *
> >>> + * Copyright (C) 2019, Google LLC.
> >>> + * Author: Brendan Higgins <brendanhiggins@google.com>
> >>> + */
> >>> +
> >>> +#ifndef _KUNIT_TEST_H
> >>> +#define _KUNIT_TEST_H
> >>> +
> >>> +#include <linux/types.h>
> >>> +
> >>> +struct kunit;
> >>> +
> >>> +/**
> >>> + * struct kunit_case - represents an individual test case.
> >>> + * @run_case: the function representing the actual test case.
> >>> + * @name: the name of the test case.
> >>> + *
> >>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> >>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
> >>> + * test case is associated with a &struct kunit_suite and will be run after the
> >>> + * suite's init function and followed by the suite's exit function.
> >>> + *
> >>> + * A test case should be static and should only be created with the KUNIT_CASE()
> >>> + * macro; additionally, every array of test cases should be terminated with an
> >>> + * empty test case.
> >>> + *
> >>> + * Example:
> >>
> >> Can you fix these line continuations. It makes it very hard to read.
> >> Sorry for this late comment. These comments lines are longer than 80
> >> and wrap.
> >
> > None of the lines in this commit are over 80 characters in column
> > width. Some are exactly 80 characters (like above).
> >
> > My guess is that you are seeing the diff added text (+ ), which when
> > you add that to a line which is exactly 80 char in length ends up
> > being over 80 char in email. If you apply the patch you will see that
> > they are only 80 chars.
> >
> >>
> >> There are several comment lines in the file that are way too long.
> >
> > Note that checkpatch also does not complain about any over 80 char
> > lines in this file.
> >
> > Sorry if I am misunderstanding what you are trying to tell me. Please
> > confirm either way.
> >
>
> WARNING: Avoid unnecessary line continuations
> #258: FILE: include/kunit/test.h:137:
> + */ \
>
> total: 0 errors, 2 warnings, 388 lines checked
Ah, okay so you don't like the warning about the line continuation.
That's not because it is over 80 char, but because there is a line
continuation after a comment. I don't really see a way to get rid of
it without removing the comment from inside the macro.
I put this TODO there in the first place a Luis' request, and I put it
in the body of the macro because this macro already had a kernel-doc
comment and I didn't think that an implementation detail TODO belonged
in the user documentation.
> Go ahead fix these. It appears there are few lines that either longer
> than 80. In general, I keep them around 75, so it is easier read.
Sorry, the above is the only checkpatch warning other than the
reminder to update the MAINTAINERS file.
Are you saying you want me to go through and make all the lines fit in
75 char column width? I hope not because that is going to be a pretty
substantial change to make.
^ permalink raw reply
* Re: [PATCH v14 09/18] kunit: test: add support for test abort
From: Brendan Higgins @ 2019-08-23 17:29 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg
In-Reply-To: <93cad898-78c8-2bd4-f1c7-5d34fdfbb7cb@kernel.org>
On Fri, Aug 23, 2019 at 10:07 AM shuah <shuah@kernel.org> wrote:
>
> On 8/23/19 10:56 AM, Brendan Higgins wrote:
> > On Fri, Aug 23, 2019 at 8:36 AM shuah <shuah@kernel.org> wrote:
> >>
> >> Hi Brendan,
> >>
> >> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> >>> Add support for aborting/bailing out of test cases, which is needed for
> >>> implementing assertions.
> >>>
> >>> An assertion is like an expectation, but bails out of the test case
> >>> early if the assertion is not met. The idea with assertions is that you
> >>> use them to state all the preconditions for your test. Logically
> >>> speaking, these are the premises of the test case, so if a premise isn't
> >>> true, there is no point in continuing the test case because there are no
> >>> conclusions that can be drawn without the premises. Whereas, the
> >>> expectation is the thing you are trying to prove.
> >>>
> >>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> >>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> >>> ---
> >>> include/kunit/test.h | 2 +
> >>> include/kunit/try-catch.h | 75 +++++++++++++++++++++
> >>> kunit/Makefile | 3 +-
> >>> kunit/test.c | 137 +++++++++++++++++++++++++++++++++-----
> >>> kunit/try-catch.c | 118 ++++++++++++++++++++++++++++++++
> >>> 5 files changed, 319 insertions(+), 16 deletions(-)
> >>> create mode 100644 include/kunit/try-catch.h
> >>> create mode 100644 kunit/try-catch.c
> >>>
> >>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> >>> index 6917b186b737a..390ce02f717b6 100644
> >>> --- a/include/kunit/test.h
> >>> +++ b/include/kunit/test.h
> >>> @@ -10,6 +10,7 @@
> >>> #define _KUNIT_TEST_H
> >>>
> >>> #include <kunit/assert.h>
> >>> +#include <kunit/try-catch.h>
> >>> #include <linux/kernel.h>
> >>> #include <linux/slab.h>
> >>> #include <linux/types.h>
> >>> @@ -167,6 +168,7 @@ struct kunit {
> >>>
> >>> /* private: internal use only. */
> >>> const char *name; /* Read only after initialization! */
> >>> + struct kunit_try_catch try_catch;
> >>> /*
> >>> * success starts as true, and may only be set to false during a test
> >>> * case; thus, it is safe to update this across multiple threads using
> >>> diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
> >>> new file mode 100644
> >>> index 0000000000000..404f336cbdc85
> >>> --- /dev/null
> >>> +++ b/include/kunit/try-catch.h
> >>> @@ -0,0 +1,75 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +/*
> >>> + * An API to allow a function, that may fail, to be executed, and recover in a
> >>> + * controlled manner.
> >>> + *
> >>> + * Copyright (C) 2019, Google LLC.
> >>> + * Author: Brendan Higgins <brendanhiggins@google.com>
> >>> + */
> >>> +
> >>> +#ifndef _KUNIT_TRY_CATCH_H
> >>> +#define _KUNIT_TRY_CATCH_H
> >>> +
> >>> +#include <linux/types.h>
> >>> +
> >>> +typedef void (*kunit_try_catch_func_t)(void *);
> >>> +
> >>> +struct completion;
> >>> +struct kunit;
> >>> +
> >>> +/**
> >>> + * struct kunit_try_catch - provides a generic way to run code which might fail.
> >>> + * @test: The test case that is currently being executed.
> >>> + * @try_completion: Completion that the control thread waits on while test runs.
> >>> + * @try_result: Contains any errno obtained while running test case.
> >>> + * @try: The function, the test case, to attempt to run.
> >>> + * @catch: The function called if @try bails out.
> >>> + * @context: used to pass user data to the try and catch functions.
> >>> + *
> >>> + * kunit_try_catch provides a generic, architecture independent way to execute
> >>> + * an arbitrary function of type kunit_try_catch_func_t which may bail out by
> >>> + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
> >>> + * is stopped at the site of invocation and @catch is called.
> >>> + *
> >>> + * struct kunit_try_catch provides a generic interface for the functionality
> >>> + * needed to implement kunit->abort() which in turn is needed for implementing
> >>> + * assertions. Assertions allow stating a precondition for a test simplifying
> >>> + * how test cases are written and presented.
> >>> + *
> >>> + * Assertions are like expectations, except they abort (call
> >>> + * kunit_try_catch_throw()) when the specified condition is not met. This is
> >>> + * useful when you look at a test case as a logical statement about some piece
> >>> + * of code, where assertions are the premises for the test case, and the
> >>> + * conclusion is a set of predicates, rather expectations, that must all be
> >>> + * true. If your premises are violated, it does not makes sense to continue.
> >>> + */
> >>> +struct kunit_try_catch {
> >>> + /* private: internal use only. */
> >>> + struct kunit *test;
> >>> + struct completion *try_completion;
> >>> + int try_result;
> >>> + kunit_try_catch_func_t try;
> >>> + kunit_try_catch_func_t catch;
> >>> + void *context;
> >>> +};
> >>> +
> >>> +void kunit_try_catch_init(struct kunit_try_catch *try_catch,
> >>> + struct kunit *test,
> >>> + kunit_try_catch_func_t try,
> >>> + kunit_try_catch_func_t catch);
> >>> +
> >>> +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
> >>> +
> >>> +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
> >>> +
> >>> +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
> >>> +{
> >>> + return try_catch->try_result;
> >>> +}
> >>> +
> >>> +/*
> >>> + * Exposed for testing only.
> >>> + */
> >>> +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
> >>> +
> >>> +#endif /* _KUNIT_TRY_CATCH_H */
> >>> diff --git a/kunit/Makefile b/kunit/Makefile
> >>> index 4e46450bcb3a8..c9176c9c578c6 100644
> >>> --- a/kunit/Makefile
> >>> +++ b/kunit/Makefile
> >>> @@ -1,6 +1,7 @@
> >>> obj-$(CONFIG_KUNIT) += test.o \
> >>> string-stream.o \
> >>> - assert.o
> >>> + assert.o \
> >>> + try-catch.o
> >>>
> >>> obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
> >>>
> >>> diff --git a/kunit/test.c b/kunit/test.c
> >>> index 3cbceb34b3b36..ded9895143209 100644
> >>> --- a/kunit/test.c
> >>> +++ b/kunit/test.c
> >>> @@ -7,7 +7,9 @@
> >>> */
> >>>
> >>> #include <kunit/test.h>
> >>> +#include <kunit/try-catch.h>
> >>> #include <linux/kernel.h>
> >>> +#include <linux/sched/debug.h>
> >>>
> >>> static void kunit_set_failure(struct kunit *test)
> >>> {
> >>> @@ -162,6 +164,19 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert)
> >>> WARN_ON(string_stream_destroy(stream));
> >>> }
> >>>
> >>> +static void __noreturn kunit_abort(struct kunit *test)
> >>> +{
> >>> + kunit_try_catch_throw(&test->try_catch); /* Does not return. */
> >>> +
> >>> + /*
> >>> + * Throw could not abort from test.
> >>> + *
> >>> + * XXX: we should never reach this line! As kunit_try_catch_throw is
> >>> + * marked __noreturn.
> >>> + */
> >>> + BUG();
> >>
> >>
> >> I recall discussion on this. What's the point in keeping thie
> >> BUG() around when it doesn't even reach? It can even be a
> >> WARN_ON() in that case right?
> >
> > Originally I had BUG() here, and Frank (I think it was Frank, sorry it
> > was a while ago) told me it should be WARN_ON(). In v12 Stephen told
> > me it should be BUG(), and nobody objected so I went back to making it
> > a BUG() (note I also mentioned this change on the cover letter of v13
> > and still no one objected).
> >
>
> Yeah. Sorry for the confusing advice. WARN_ON() or nothing is the right
> thing here. I have been cleaning BUG() and WARN_ON() that aren't needed.
>
> I would just delete BUG all together.
Alright, that's fine. I will send out a new patch set that removes
this once we get the discussion on 01/18 resolved.
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: shuah @ 2019-08-23 17:34 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg, shuah
In-Reply-To: <CAFd5g446J=cVW4QW+QeZMLDi+ANqshAW6KTrFFBTusPcdr6-GA@mail.gmail.com>
On 8/23/19 11:27 AM, Brendan Higgins wrote:
> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
>>
>> On 8/23/19 10:48 AM, Brendan Higgins wrote:
>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
>>>>
>>>> Hi Brendan,
>>>>
>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
>>>>> Add core facilities for defining unit tests; this provides a common way
>>>>> to define test cases, functions that execute code which is under test
>>>>> and determine whether the code under test behaves as expected; this also
>>>>> provides a way to group together related test cases in test suites (here
>>>>> we call them test_modules).
>>>>>
>>>>> Just define test cases and how to execute them for now; setting
>>>>> expectations on code will be defined later.
>>>>>
>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>>>>> ---
>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
>>>>> kunit/Kconfig | 17 ++++
>>>>> kunit/Makefile | 1 +
>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
>>>>> 4 files changed, 388 insertions(+)
>>>>> create mode 100644 include/kunit/test.h
>>>>> create mode 100644 kunit/Kconfig
>>>>> create mode 100644 kunit/Makefile
>>>>> create mode 100644 kunit/test.c
>>>>>
>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>>>> new file mode 100644
>>>>> index 0000000000000..e0b34acb9ee4e
>>>>> --- /dev/null
>>>>> +++ b/include/kunit/test.h
>>>>> @@ -0,0 +1,179 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>> +/*
>>>>> + * Base unit test (KUnit) API.
>>>>> + *
>>>>> + * Copyright (C) 2019, Google LLC.
>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
>>>>> + */
>>>>> +
>>>>> +#ifndef _KUNIT_TEST_H
>>>>> +#define _KUNIT_TEST_H
>>>>> +
>>>>> +#include <linux/types.h>
>>>>> +
>>>>> +struct kunit;
>>>>> +
>>>>> +/**
>>>>> + * struct kunit_case - represents an individual test case.
>>>>> + * @run_case: the function representing the actual test case.
>>>>> + * @name: the name of the test case.
>>>>> + *
>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
>>>>> + * test case is associated with a &struct kunit_suite and will be run after the
>>>>> + * suite's init function and followed by the suite's exit function.
>>>>> + *
>>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
>>>>> + * macro; additionally, every array of test cases should be terminated with an
>>>>> + * empty test case.
>>>>> + *
>>>>> + * Example:
>>>>
>>>> Can you fix these line continuations. It makes it very hard to read.
>>>> Sorry for this late comment. These comments lines are longer than 80
>>>> and wrap.
>>>
>>> None of the lines in this commit are over 80 characters in column
>>> width. Some are exactly 80 characters (like above).
>>>
>>> My guess is that you are seeing the diff added text (+ ), which when
>>> you add that to a line which is exactly 80 char in length ends up
>>> being over 80 char in email. If you apply the patch you will see that
>>> they are only 80 chars.
>>>
>>>>
>>>> There are several comment lines in the file that are way too long.
>>>
>>> Note that checkpatch also does not complain about any over 80 char
>>> lines in this file.
>>>
>>> Sorry if I am misunderstanding what you are trying to tell me. Please
>>> confirm either way.
>>>
>>
>> WARNING: Avoid unnecessary line continuations
>> #258: FILE: include/kunit/test.h:137:
>> + */ \
>>
>> total: 0 errors, 2 warnings, 388 lines checked
>
> Ah, okay so you don't like the warning about the line continuation.
> That's not because it is over 80 char, but because there is a line
> continuation after a comment. I don't really see a way to get rid of
> it without removing the comment from inside the macro.
>
> I put this TODO there in the first place a Luis' request, and I put it
> in the body of the macro because this macro already had a kernel-doc
> comment and I didn't think that an implementation detail TODO belonged
> in the user documentation.
>
>> Go ahead fix these. It appears there are few lines that either longer
>> than 80. In general, I keep them around 75, so it is easier read.
>
> Sorry, the above is the only checkpatch warning other than the
> reminder to update the MAINTAINERS file.
>
> Are you saying you want me to go through and make all the lines fit in
> 75 char column width? I hope not because that is going to be a pretty
> substantial change to make.
>
There are two things with these comment lines. One is checkpatch
complaining and the other is general readability.
Please go ahead and adjust them.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: Brendan Higgins @ 2019-08-23 17:54 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg
In-Reply-To: <42c6235c-c586-8de1-1913-7cf1962c6066@kernel.org>
On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote:
>
> On 8/23/19 11:27 AM, Brendan Higgins wrote:
> > On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
> >>
> >> On 8/23/19 10:48 AM, Brendan Higgins wrote:
> >>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
> >>>>
> >>>> Hi Brendan,
> >>>>
> >>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> >>>>> Add core facilities for defining unit tests; this provides a common way
> >>>>> to define test cases, functions that execute code which is under test
> >>>>> and determine whether the code under test behaves as expected; this also
> >>>>> provides a way to group together related test cases in test suites (here
> >>>>> we call them test_modules).
> >>>>>
> >>>>> Just define test cases and how to execute them for now; setting
> >>>>> expectations on code will be defined later.
> >>>>>
> >>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> >>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> >>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> >>>>> ---
> >>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
> >>>>> kunit/Kconfig | 17 ++++
> >>>>> kunit/Makefile | 1 +
> >>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
> >>>>> 4 files changed, 388 insertions(+)
> >>>>> create mode 100644 include/kunit/test.h
> >>>>> create mode 100644 kunit/Kconfig
> >>>>> create mode 100644 kunit/Makefile
> >>>>> create mode 100644 kunit/test.c
> >>>>>
> >>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> >>>>> new file mode 100644
> >>>>> index 0000000000000..e0b34acb9ee4e
> >>>>> --- /dev/null
> >>>>> +++ b/include/kunit/test.h
> >>>>> @@ -0,0 +1,179 @@
> >>>>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>>>> +/*
> >>>>> + * Base unit test (KUnit) API.
> >>>>> + *
> >>>>> + * Copyright (C) 2019, Google LLC.
> >>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
> >>>>> + */
> >>>>> +
> >>>>> +#ifndef _KUNIT_TEST_H
> >>>>> +#define _KUNIT_TEST_H
> >>>>> +
> >>>>> +#include <linux/types.h>
> >>>>> +
> >>>>> +struct kunit;
> >>>>> +
> >>>>> +/**
> >>>>> + * struct kunit_case - represents an individual test case.
> >>>>> + * @run_case: the function representing the actual test case.
> >>>>> + * @name: the name of the test case.
> >>>>> + *
> >>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> >>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
> >>>>> + * test case is associated with a &struct kunit_suite and will be run after the
> >>>>> + * suite's init function and followed by the suite's exit function.
> >>>>> + *
> >>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
> >>>>> + * macro; additionally, every array of test cases should be terminated with an
> >>>>> + * empty test case.
> >>>>> + *
> >>>>> + * Example:
> >>>>
> >>>> Can you fix these line continuations. It makes it very hard to read.
> >>>> Sorry for this late comment. These comments lines are longer than 80
> >>>> and wrap.
> >>>
> >>> None of the lines in this commit are over 80 characters in column
> >>> width. Some are exactly 80 characters (like above).
> >>>
> >>> My guess is that you are seeing the diff added text (+ ), which when
> >>> you add that to a line which is exactly 80 char in length ends up
> >>> being over 80 char in email. If you apply the patch you will see that
> >>> they are only 80 chars.
> >>>
> >>>>
> >>>> There are several comment lines in the file that are way too long.
> >>>
> >>> Note that checkpatch also does not complain about any over 80 char
> >>> lines in this file.
> >>>
> >>> Sorry if I am misunderstanding what you are trying to tell me. Please
> >>> confirm either way.
> >>>
> >>
> >> WARNING: Avoid unnecessary line continuations
> >> #258: FILE: include/kunit/test.h:137:
> >> + */ \
> >>
> >> total: 0 errors, 2 warnings, 388 lines checked
> >
> > Ah, okay so you don't like the warning about the line continuation.
> > That's not because it is over 80 char, but because there is a line
> > continuation after a comment. I don't really see a way to get rid of
> > it without removing the comment from inside the macro.
> >
> > I put this TODO there in the first place a Luis' request, and I put it
> > in the body of the macro because this macro already had a kernel-doc
> > comment and I didn't think that an implementation detail TODO belonged
> > in the user documentation.
> >
> >> Go ahead fix these. It appears there are few lines that either longer
> >> than 80. In general, I keep them around 75, so it is easier read.
> >
> > Sorry, the above is the only checkpatch warning other than the
> > reminder to update the MAINTAINERS file.
> >
> > Are you saying you want me to go through and make all the lines fit in
> > 75 char column width? I hope not because that is going to be a pretty
> > substantial change to make.
> >
>
> There are two things with these comment lines. One is checkpatch
> complaining and the other is general readability.
So for the checkpatch warning, do you want me to move the comment out
of the macro body into the kernel-doc comment? I don't really think it
is the right place for a comment of this nature, but I think it is
probably better than dropping it entirely (I don't see how else to do
it without just removing the comment entirely).
As for general readability, are you asking me to readjust all my code
in all 18 patches to fit in 75 chars? Sorry for the confusion, I am
just really surprised by this request. I thought the policy is 80
char, and reflowing all of my code in this patchset to 75 chars is not
a quick and easy thing to do. Additionally, there are some other short
term and long term issues about enforcing a 75 char limit on the KUnit
code.
Sorry, maybe I am just not understanding what you are asking me.
^ permalink raw reply
* Re: [PATCH net-next 03/10] net: sched: refactor block offloads counter usage
From: Jakub Kicinski @ 2019-08-23 18:26 UTC (permalink / raw)
To: Vlad Buslov
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
pablo@netfilter.org, Jiri Pirko, linux-doc
In-Reply-To: <vbfftls17yl.fsf@mellanox.com>
On Fri, 23 Aug 2019 10:39:50 +0000, Vlad Buslov wrote:
> >> +/* Destroy filter and decrement block offload counter, if filter was previously
> >> + * offloaded.
> >> + */
> >> +
> >
> > hm.. is this gap between comment and function it pertains to
> > intentional?
>
> Majority of function comments in cls_api.c have newline after them (not
> all of them though). I don't have any strong opinions regarding this.
> You suggest it is better not to have blank lines after function
> comments?
Ah, you're right. I think it's pretty strange to have a new line after
a comment which pertains only to the function which is immediately
following it. Often the new line is used as a separation, when the
comment describes whole section of the file..
I kind of wish kdoc allowed none of the parameters to be described.
Often you want to document the function but the parameters are kind
of obvious.
Anyway... feel free to leave this as is.
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: shuah @ 2019-08-23 18:32 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg, shuah
In-Reply-To: <CAFd5g44hLgeqPtNw1zQ5k_+apBm=ri_6=wAgHk=oPOvQs6xgNg@mail.gmail.com>
On 8/23/19 11:54 AM, Brendan Higgins wrote:
> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote:
>>
>> On 8/23/19 11:27 AM, Brendan Higgins wrote:
>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
>>>>
>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote:
>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
>>>>>>
>>>>>> Hi Brendan,
>>>>>>
>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
>>>>>>> Add core facilities for defining unit tests; this provides a common way
>>>>>>> to define test cases, functions that execute code which is under test
>>>>>>> and determine whether the code under test behaves as expected; this also
>>>>>>> provides a way to group together related test cases in test suites (here
>>>>>>> we call them test_modules).
>>>>>>>
>>>>>>> Just define test cases and how to execute them for now; setting
>>>>>>> expectations on code will be defined later.
>>>>>>>
>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>>>>>>> ---
>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
>>>>>>> kunit/Kconfig | 17 ++++
>>>>>>> kunit/Makefile | 1 +
>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>> 4 files changed, 388 insertions(+)
>>>>>>> create mode 100644 include/kunit/test.h
>>>>>>> create mode 100644 kunit/Kconfig
>>>>>>> create mode 100644 kunit/Makefile
>>>>>>> create mode 100644 kunit/test.c
>>>>>>>
>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>>>>>> new file mode 100644
>>>>>>> index 0000000000000..e0b34acb9ee4e
>>>>>>> --- /dev/null
>>>>>>> +++ b/include/kunit/test.h
>>>>>>> @@ -0,0 +1,179 @@
>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>>>> +/*
>>>>>>> + * Base unit test (KUnit) API.
>>>>>>> + *
>>>>>>> + * Copyright (C) 2019, Google LLC.
>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
>>>>>>> + */
>>>>>>> +
>>>>>>> +#ifndef _KUNIT_TEST_H
>>>>>>> +#define _KUNIT_TEST_H
>>>>>>> +
>>>>>>> +#include <linux/types.h>
>>>>>>> +
>>>>>>> +struct kunit;
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * struct kunit_case - represents an individual test case.
>>>>>>> + * @run_case: the function representing the actual test case.
>>>>>>> + * @name: the name of the test case.
>>>>>>> + *
>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the
>>>>>>> + * suite's init function and followed by the suite's exit function.
>>>>>>> + *
>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
>>>>>>> + * macro; additionally, every array of test cases should be terminated with an
>>>>>>> + * empty test case.
>>>>>>> + *
>>>>>>> + * Example:
>>>>>>
>>>>>> Can you fix these line continuations. It makes it very hard to read.
>>>>>> Sorry for this late comment. These comments lines are longer than 80
>>>>>> and wrap.
>>>>>
>>>>> None of the lines in this commit are over 80 characters in column
>>>>> width. Some are exactly 80 characters (like above).
>>>>>
>>>>> My guess is that you are seeing the diff added text (+ ), which when
>>>>> you add that to a line which is exactly 80 char in length ends up
>>>>> being over 80 char in email. If you apply the patch you will see that
>>>>> they are only 80 chars.
>>>>>
>>>>>>
>>>>>> There are several comment lines in the file that are way too long.
>>>>>
>>>>> Note that checkpatch also does not complain about any over 80 char
>>>>> lines in this file.
>>>>>
>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please
>>>>> confirm either way.
>>>>>
>>>>
>>>> WARNING: Avoid unnecessary line continuations
>>>> #258: FILE: include/kunit/test.h:137:
>>>> + */ \
>>>>
>>>> total: 0 errors, 2 warnings, 388 lines checked
>>>
>>> Ah, okay so you don't like the warning about the line continuation.
>>> That's not because it is over 80 char, but because there is a line
>>> continuation after a comment. I don't really see a way to get rid of
>>> it without removing the comment from inside the macro.
>>>
>>> I put this TODO there in the first place a Luis' request, and I put it
>>> in the body of the macro because this macro already had a kernel-doc
>>> comment and I didn't think that an implementation detail TODO belonged
>>> in the user documentation.
>>>
>>>> Go ahead fix these. It appears there are few lines that either longer
>>>> than 80. In general, I keep them around 75, so it is easier read.
>>>
>>> Sorry, the above is the only checkpatch warning other than the
>>> reminder to update the MAINTAINERS file.
>>>
>>> Are you saying you want me to go through and make all the lines fit in
>>> 75 char column width? I hope not because that is going to be a pretty
>>> substantial change to make.
>>>
>>
>> There are two things with these comment lines. One is checkpatch
>> complaining and the other is general readability.
>
> So for the checkpatch warning, do you want me to move the comment out
> of the macro body into the kernel-doc comment? I don't really think it
> is the right place for a comment of this nature, but I think it is
> probably better than dropping it entirely (I don't see how else to do
> it without just removing the comment entirely).
>
Don't drop the comments. It makes perfect sense to turn this into a
kernel-doc comment.
We are going back forth on this a lot. I see several lines 81+ in
this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am
fine with it if you want to convert these to kernel-doc comments.
I think it makes perfect sense.
I would like it if you take a look and fix the lines that are longer
than 80. If you think you would rather spend time converting them to
kernel-doc, I am fine with it.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: Brendan Higgins @ 2019-08-23 18:56 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg
In-Reply-To: <54f3c011-d666-e828-5e77-359b7a7374e7@kernel.org>
On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote:
>
> On 8/23/19 11:54 AM, Brendan Higgins wrote:
> > On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote:
> >>
> >> On 8/23/19 11:27 AM, Brendan Higgins wrote:
> >>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
> >>>>
> >>>> On 8/23/19 10:48 AM, Brendan Higgins wrote:
> >>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
> >>>>>>
> >>>>>> Hi Brendan,
> >>>>>>
> >>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> >>>>>>> Add core facilities for defining unit tests; this provides a common way
> >>>>>>> to define test cases, functions that execute code which is under test
> >>>>>>> and determine whether the code under test behaves as expected; this also
> >>>>>>> provides a way to group together related test cases in test suites (here
> >>>>>>> we call them test_modules).
> >>>>>>>
> >>>>>>> Just define test cases and how to execute them for now; setting
> >>>>>>> expectations on code will be defined later.
> >>>>>>>
> >>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> >>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> >>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> >>>>>>> ---
> >>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
> >>>>>>> kunit/Kconfig | 17 ++++
> >>>>>>> kunit/Makefile | 1 +
> >>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
> >>>>>>> 4 files changed, 388 insertions(+)
> >>>>>>> create mode 100644 include/kunit/test.h
> >>>>>>> create mode 100644 kunit/Kconfig
> >>>>>>> create mode 100644 kunit/Makefile
> >>>>>>> create mode 100644 kunit/test.c
> >>>>>>>
> >>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> >>>>>>> new file mode 100644
> >>>>>>> index 0000000000000..e0b34acb9ee4e
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/include/kunit/test.h
> >>>>>>> @@ -0,0 +1,179 @@
> >>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>>>>>> +/*
> >>>>>>> + * Base unit test (KUnit) API.
> >>>>>>> + *
> >>>>>>> + * Copyright (C) 2019, Google LLC.
> >>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> +#ifndef _KUNIT_TEST_H
> >>>>>>> +#define _KUNIT_TEST_H
> >>>>>>> +
> >>>>>>> +#include <linux/types.h>
> >>>>>>> +
> >>>>>>> +struct kunit;
> >>>>>>> +
> >>>>>>> +/**
> >>>>>>> + * struct kunit_case - represents an individual test case.
> >>>>>>> + * @run_case: the function representing the actual test case.
> >>>>>>> + * @name: the name of the test case.
> >>>>>>> + *
> >>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> >>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
> >>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the
> >>>>>>> + * suite's init function and followed by the suite's exit function.
> >>>>>>> + *
> >>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
> >>>>>>> + * macro; additionally, every array of test cases should be terminated with an
> >>>>>>> + * empty test case.
> >>>>>>> + *
> >>>>>>> + * Example:
> >>>>>>
> >>>>>> Can you fix these line continuations. It makes it very hard to read.
> >>>>>> Sorry for this late comment. These comments lines are longer than 80
> >>>>>> and wrap.
> >>>>>
> >>>>> None of the lines in this commit are over 80 characters in column
> >>>>> width. Some are exactly 80 characters (like above).
> >>>>>
> >>>>> My guess is that you are seeing the diff added text (+ ), which when
> >>>>> you add that to a line which is exactly 80 char in length ends up
> >>>>> being over 80 char in email. If you apply the patch you will see that
> >>>>> they are only 80 chars.
> >>>>>
> >>>>>>
> >>>>>> There are several comment lines in the file that are way too long.
> >>>>>
> >>>>> Note that checkpatch also does not complain about any over 80 char
> >>>>> lines in this file.
> >>>>>
> >>>>> Sorry if I am misunderstanding what you are trying to tell me. Please
> >>>>> confirm either way.
> >>>>>
> >>>>
> >>>> WARNING: Avoid unnecessary line continuations
> >>>> #258: FILE: include/kunit/test.h:137:
> >>>> + */ \
> >>>>
> >>>> total: 0 errors, 2 warnings, 388 lines checked
> >>>
> >>> Ah, okay so you don't like the warning about the line continuation.
> >>> That's not because it is over 80 char, but because there is a line
> >>> continuation after a comment. I don't really see a way to get rid of
> >>> it without removing the comment from inside the macro.
> >>>
> >>> I put this TODO there in the first place a Luis' request, and I put it
> >>> in the body of the macro because this macro already had a kernel-doc
> >>> comment and I didn't think that an implementation detail TODO belonged
> >>> in the user documentation.
> >>>
> >>>> Go ahead fix these. It appears there are few lines that either longer
> >>>> than 80. In general, I keep them around 75, so it is easier read.
> >>>
> >>> Sorry, the above is the only checkpatch warning other than the
> >>> reminder to update the MAINTAINERS file.
> >>>
> >>> Are you saying you want me to go through and make all the lines fit in
> >>> 75 char column width? I hope not because that is going to be a pretty
> >>> substantial change to make.
> >>>
> >>
> >> There are two things with these comment lines. One is checkpatch
> >> complaining and the other is general readability.
> >
> > So for the checkpatch warning, do you want me to move the comment out
> > of the macro body into the kernel-doc comment? I don't really think it
> > is the right place for a comment of this nature, but I think it is
> > probably better than dropping it entirely (I don't see how else to do
> > it without just removing the comment entirely).
> >
>
> Don't drop the comments. It makes perfect sense to turn this into a
> kernel-doc comment.
I am fine with that. I will do that in a subsequent revision once we
figure out the column limit issue.
> We are going back forth on this a lot. I see several lines 81+ in
> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am
> fine with it if you want to convert these to kernel-doc comments.
> I think it makes perfect sense.
Okay, so this is interesting. When I look at the applied patches in my
local repo, I don't see any 81+ lines. So it seems that something
interesting is going on here.
To be clear (sorry for the stupid question) you are seeing the issue
after you applied the patch, and not in the patch file itself?
Since we are still at OSS, would you mind if we meet up this afternoon
so I can see this issue you are seeing? I imagine we should get this
figured out pretty quickly.
Cheers
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: shuah @ 2019-08-23 19:04 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg, shuah
In-Reply-To: <CAFd5g44NAs6KK0_sG9itgT5qxujpyx36XV6tT8=zMynG-ZyVhQ@mail.gmail.com>
On 8/23/19 12:56 PM, Brendan Higgins wrote:
> On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote:
>>
>> On 8/23/19 11:54 AM, Brendan Higgins wrote:
>>> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote:
>>>>
>>>> On 8/23/19 11:27 AM, Brendan Higgins wrote:
>>>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
>>>>>>
>>>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote:
>>>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
>>>>>>>>
>>>>>>>> Hi Brendan,
>>>>>>>>
>>>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
>>>>>>>>> Add core facilities for defining unit tests; this provides a common way
>>>>>>>>> to define test cases, functions that execute code which is under test
>>>>>>>>> and determine whether the code under test behaves as expected; this also
>>>>>>>>> provides a way to group together related test cases in test suites (here
>>>>>>>>> we call them test_modules).
>>>>>>>>>
>>>>>>>>> Just define test cases and how to execute them for now; setting
>>>>>>>>> expectations on code will be defined later.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>>>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
>>>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>>>>>>>>> ---
>>>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
>>>>>>>>> kunit/Kconfig | 17 ++++
>>>>>>>>> kunit/Makefile | 1 +
>>>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>> 4 files changed, 388 insertions(+)
>>>>>>>>> create mode 100644 include/kunit/test.h
>>>>>>>>> create mode 100644 kunit/Kconfig
>>>>>>>>> create mode 100644 kunit/Makefile
>>>>>>>>> create mode 100644 kunit/test.c
>>>>>>>>>
>>>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000000000..e0b34acb9ee4e
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/include/kunit/test.h
>>>>>>>>> @@ -0,0 +1,179 @@
>>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>>>>>> +/*
>>>>>>>>> + * Base unit test (KUnit) API.
>>>>>>>>> + *
>>>>>>>>> + * Copyright (C) 2019, Google LLC.
>>>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
>>>>>>>>> + */
>>>>>>>>> +
>>>>>>>>> +#ifndef _KUNIT_TEST_H
>>>>>>>>> +#define _KUNIT_TEST_H
>>>>>>>>> +
>>>>>>>>> +#include <linux/types.h>
>>>>>>>>> +
>>>>>>>>> +struct kunit;
>>>>>>>>> +
>>>>>>>>> +/**
>>>>>>>>> + * struct kunit_case - represents an individual test case.
>>>>>>>>> + * @run_case: the function representing the actual test case.
>>>>>>>>> + * @name: the name of the test case.
>>>>>>>>> + *
>>>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
>>>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
>>>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the
>>>>>>>>> + * suite's init function and followed by the suite's exit function.
>>>>>>>>> + *
>>>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
>>>>>>>>> + * macro; additionally, every array of test cases should be terminated with an
>>>>>>>>> + * empty test case.
>>>>>>>>> + *
>>>>>>>>> + * Example:
>>>>>>>>
>>>>>>>> Can you fix these line continuations. It makes it very hard to read.
>>>>>>>> Sorry for this late comment. These comments lines are longer than 80
>>>>>>>> and wrap.
>>>>>>>
>>>>>>> None of the lines in this commit are over 80 characters in column
>>>>>>> width. Some are exactly 80 characters (like above).
>>>>>>>
>>>>>>> My guess is that you are seeing the diff added text (+ ), which when
>>>>>>> you add that to a line which is exactly 80 char in length ends up
>>>>>>> being over 80 char in email. If you apply the patch you will see that
>>>>>>> they are only 80 chars.
>>>>>>>
>>>>>>>>
>>>>>>>> There are several comment lines in the file that are way too long.
>>>>>>>
>>>>>>> Note that checkpatch also does not complain about any over 80 char
>>>>>>> lines in this file.
>>>>>>>
>>>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please
>>>>>>> confirm either way.
>>>>>>>
>>>>>>
>>>>>> WARNING: Avoid unnecessary line continuations
>>>>>> #258: FILE: include/kunit/test.h:137:
>>>>>> + */ \
>>>>>>
>>>>>> total: 0 errors, 2 warnings, 388 lines checked
>>>>>
>>>>> Ah, okay so you don't like the warning about the line continuation.
>>>>> That's not because it is over 80 char, but because there is a line
>>>>> continuation after a comment. I don't really see a way to get rid of
>>>>> it without removing the comment from inside the macro.
>>>>>
>>>>> I put this TODO there in the first place a Luis' request, and I put it
>>>>> in the body of the macro because this macro already had a kernel-doc
>>>>> comment and I didn't think that an implementation detail TODO belonged
>>>>> in the user documentation.
>>>>>
>>>>>> Go ahead fix these. It appears there are few lines that either longer
>>>>>> than 80. In general, I keep them around 75, so it is easier read.
>>>>>
>>>>> Sorry, the above is the only checkpatch warning other than the
>>>>> reminder to update the MAINTAINERS file.
>>>>>
>>>>> Are you saying you want me to go through and make all the lines fit in
>>>>> 75 char column width? I hope not because that is going to be a pretty
>>>>> substantial change to make.
>>>>>
>>>>
>>>> There are two things with these comment lines. One is checkpatch
>>>> complaining and the other is general readability.
>>>
>>> So for the checkpatch warning, do you want me to move the comment out
>>> of the macro body into the kernel-doc comment? I don't really think it
>>> is the right place for a comment of this nature, but I think it is
>>> probably better than dropping it entirely (I don't see how else to do
>>> it without just removing the comment entirely).
>>>
>>
>> Don't drop the comments. It makes perfect sense to turn this into a
>> kernel-doc comment.
>
> I am fine with that. I will do that in a subsequent revision once we
> figure out the column limit issue.
>
>> We are going back forth on this a lot. I see several lines 81+ in
>> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am
>> fine with it if you want to convert these to kernel-doc comments.
>> I think it makes perfect sense.
>
> Okay, so this is interesting. When I look at the applied patches in my
> local repo, I don't see any 81+ lines. So it seems that something
> interesting is going on here.
>
> To be clear (sorry for the stupid question) you are seeing the issue
> after you applied the patch, and not in the patch file itself?
>
I am using my normal workflow. My pre-commit check is catching this.
Just this patch. All others are good other than the 9/18 BUG() issue.
> Since we are still at OSS, would you mind if we meet up this afternoon
> so I can see this issue you are seeing? I imagine we should get this
> figured out pretty quickly.
>
Yeah. Would have been nice. I am not at oss today.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH v3 5/7] arm64: dts: fsl: Add device tree for S32V234-EVB
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica,
Dan Nica, Larisa Ileana Grigore
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
From: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
Add initial version of device tree for S32V234-EVB, including nodes for the
4 Cortex-A53 cores, AIPS bus with UART modules, ARM architected timer and
Generic Interrupt Controller (GIC).
Keep SoC level separate from board level to let future boards with this SoC
share common properties, while the dts files will keep board-dependent
properties.
Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
Signed-off-by: Mihaela Martinas <Mihaela.Martinas@freescale.com>
Signed-off-by: Dan Nica <dan.nica@nxp.com>
Signed-off-by: Larisa Grigore <Larisa.Grigore@nxp.com>
Signed-off-by: Phu Luu An <phu.luuan@nxp.com>
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
---
arch/arm64/boot/dts/freescale/Makefile | 2 +
arch/arm64/boot/dts/freescale/s32v234-evb.dts | 25 ++++
arch/arm64/boot/dts/freescale/s32v234.dtsi | 139 ++++++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/s32v234-evb.dts
create mode 100644 arch/arm64/boot/dts/freescale/s32v234.dtsi
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index c043aca66572..9aff21324146 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -26,3 +26,5 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-rmb3.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-zest.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
+
+dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
diff --git a/arch/arm64/boot/dts/freescale/s32v234-evb.dts b/arch/arm64/boot/dts/freescale/s32v234-evb.dts
new file mode 100644
index 000000000000..4b802518cefc
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/s32v234-evb.dts
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2015-2016 Freescale Semiconductor, Inc.
+ * Copyright 2016-2017 NXP
+ */
+
+/dts-v1/;
+#include "s32v234.dtsi"
+
+/ {
+ model = "NXP S32V234-EVB2 Board";
+ compatible = "fsl,s32v234-evb", "fsl,s32v234";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
+
+&uart1 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/s32v234.dtsi b/arch/arm64/boot/dts/freescale/s32v234.dtsi
new file mode 100644
index 000000000000..37225191ccbf
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/s32v234.dtsi
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2015-2016 Freescale Semiconductor, Inc.
+ * Copyright 2016-2018 NXP
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/memreserve/ 0x80000000 0x00010000;
+
+/ {
+ compatible = "fsl,s32v234";
+ interrupt-parent = <&gic>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ aliases {
+ serial0 = &uart0;
+ serial1 = &uart1;
+ };
+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0 0x0>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x80000000>;
+ next-level-cache = <&cluster0_l2_cache>;
+ };
+
+ cpu1: cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0 0x1>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x80000000>;
+ next-level-cache = <&cluster0_l2_cache>;
+ };
+
+ cpu2: cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0 0x100>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x80000000>;
+ next-level-cache = <&cluster1_l2_cache>;
+ };
+
+ cpu3: cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0 0x101>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x80000000>;
+ next-level-cache = <&cluster1_l2_cache>;
+ };
+
+ cluster0_l2_cache: l2-cache0 {
+ compatible = "cache";
+ };
+
+ cluster1_l2_cache: l2-cache1 {
+ compatible = "cache";
+ };
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>;
+ /* clock-frequency might be modified by u-boot, depending on the
+ * chip version.
+ */
+ clock-frequency = <10000000>;
+ };
+
+ gic: interrupt-controller@7d001000 {
+ compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
+ #interrupt-cells = <3>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0 0x7d001000 0 0x1000>,
+ <0 0x7d002000 0 0x2000>,
+ <0 0x7d004000 0 0x2000>,
+ <0 0x7d006000 0 0x2000>;
+ interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_HIGH)>;
+ };
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ compatible = "simple-bus";
+ interrupt-parent = <&gic>;
+ ranges;
+
+ aips0: aips-bus@40000000 {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ interrupt-parent = <&gic>;
+ reg = <0x0 0x40000000 0x0 0x7D000>;
+ ranges;
+
+ uart0: serial@40053000 {
+ compatible = "fsl,s32v234-linflexuart";
+ reg = <0x0 0x40053000 0x0 0x1000>;
+ interrupts = <GIC_SPI 59 IRQ_TYPE_EDGE_RISING>;
+ status = "disabled";
+ };
+ };
+
+ aips1: aips-bus@40080000 {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ interrupt-parent = <&gic>;
+ reg = <0x0 0x40080000 0x0 0x70000>;
+ ranges;
+
+ uart1: serial@400bc000 {
+ compatible = "fsl,s32v234-linflexuart";
+ reg = <0x0 0x400bc000 0x0 0x1000>;
+ interrupts = <GIC_SPI 60 IRQ_TYPE_EDGE_RISING>;
+ status = "disabled";
+ };
+ };
+ };
+};
--
2.22.0
^ permalink raw reply related
* [PATCH v3 7/7] arm64: defconfig: Enable configs for S32V234
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
From: Mihaela Martinas <Mihaela.Martinas@freescale.com>
Enable support for the S32V234 SoC, including the previously added UART
driver.
Signed-off-by: Mihaela Martinas <Mihaela.Martinas@freescale.com>
Signed-off-by: Adrian.Nitu <adrian.nitu@freescale.com>
Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
---
arch/arm64/configs/defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0e58ef02880c..bb5aa95a8455 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -48,6 +48,7 @@ CONFIG_ARCH_MXC=y
CONFIG_ARCH_QCOM=y
CONFIG_ARCH_RENESAS=y
CONFIG_ARCH_ROCKCHIP=y
+CONFIG_ARCH_S32=y
CONFIG_ARCH_SEATTLE=y
CONFIG_ARCH_STRATIX10=y
CONFIG_ARCH_SYNQUACER=y
@@ -347,6 +348,8 @@ CONFIG_SERIAL_XILINX_PS_UART=y
CONFIG_SERIAL_XILINX_PS_UART_CONSOLE=y
CONFIG_SERIAL_FSL_LPUART=y
CONFIG_SERIAL_FSL_LPUART_CONSOLE=y
+CONFIG_SERIAL_FSL_LINFLEXUART=y
+CONFIG_SERIAL_FSL_LINFLEXUART_CONSOLE=y
CONFIG_SERIAL_MVEBU_UART=y
CONFIG_SERIAL_DEV_BUS=y
CONFIG_VIRTIO_CONSOLE=y
--
2.22.0
^ permalink raw reply related
* [PATCH v3 0/7] Add initial support for S32V234-EVB
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Hello,
NXP's S32V234[1] ("Treerunner") vision microprocessors are targeted for
high-performance, computationally intensive vision and sensor fusion
applications that require automotive safety levels. They include leading
edge Camera Vision modules like APEX-2, ISP and GPU. The S32V234-EVB and
S32V234-SBC boards are available for customer evaluation.
The following patch series introduces minimal enablement support for the
NXP S32V234-EVB2[2] board, which leverages most of the SoC capabilities.
Up to v2, this series also included the fsl_linflexuart driver, which has
been included in Greg Kroah-Hartman's tty git tree[3].
In the future, we aim to submit multiple drivers upstream, which can be
found in the kernel of our Auto Linux BSP[4] ("ALB"), starting with basic
pinmuxing, clock and uSDHC drivers.
For validation, you can use the U-Boot bootloader in the ALB[5], which we
build and test with our patched version of the Linaro GCC 6.3.1 2017.05
toolchain for ARM 64-bit, with sources available on [6].
Changes in v3:
* Remove the patch 'tty: serial: Add linflexuart driver for S32V234'
following its acceptance[3];
* Replace 'Freescale' with 'NXP' in the ARCH_S32 config definition and the
'model' property from the device tree;
* Remove the 'fsl-' prefixes from the dtsi and dts file names;
* Move the 'model' property from (fsl-)s32v234.dtsi to s32v234-evb.dts;
* Add newlines between the cpu nodes in s32v234.dtsi;
* Make use of GIC_SPI, GIC_PPI, GIC_CPU_MASK_SIMPLE and IRQ_TYPE_* in the
'interrupts' tuples;
* Move the 'timer' and 'interrupt-controller' nodes before 'soc' in
s32v234.dtsi;
* Be consistent with the 'LINFlexD' spelling in documentation, strings and
comments; add new patch 'serial: fsl_linflexuart: Be consistent with the
name' to update the LINFlexD driver as well;
* Remove from fsl,s32-linflexuart.txt a statement regarding the limitation
to UART mode;
* Make the compatible string SoC specific ("fsl,s32v234-linflexuart"); add
new patch 'serial: fsl_linflexuart: Update compatible string' to update
the LINFlexD driver as well;
* In the LINFlexD binding documentation, insert a space between label and
node name and remove the 'status' property.
Changes in v2:
* Update the entry in fsl.yaml to apply to all S32V234 based boards;
* Add chosen node to dts, with a 'stdout-path' property for earlycon;
* Remove linflex_verify_port(), because it was only called from
uart_set_info(), which was going to always fail at the "baud_base < 9600"
check, as we are not using uartclk from uart_port yet;
* Fix compatible string used in OF_EARLYCON_DECLARE.
[1] https://www.nxp.com/products/processors-and-microcontrollers/arm-based-processors-and-mcus/s32-automotive-platform/vision-processor-for-front-and-surround-view-camera-machine-learning-and-sensor-fusion:S32V234
[2] https://www.nxp.com/support/developer-resources/evaluation-and-development-boards/ultra-reliable-dev-platforms/s32v-mpus-platforms/s32v-vision-and-sensor-fusion-evaluation-system:S32V234EVB
[3] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next&id=b953815b819b0f327b5538feba3639d52db70172
[4] https://source.codeaurora.org/external/autobsps32/linux/
[5] https://source.codeaurora.org/external/autobsps32/u-boot/
[6] https://source.codeaurora.org/external/s32ds/compiler/gcc/
Eddy Petrișor (1):
dt-bindings: arm: fsl: Add the S32V234-EVB board
Mihaela Martinas (2):
arm64: Introduce config for S32
arm64: defconfig: Enable configs for S32V234
Stefan-Gabriel Mirea (2):
serial: fsl_linflexuart: Update compatible string
serial: fsl_linflexuart: Be consistent with the name
Stoica Cosmin-Stefan (2):
arm64: dts: fsl: Add device tree for S32V234-EVB
dt-bindings: serial: Document Freescale LINFlexD UART
.../admin-guide/kernel-parameters.txt | 2 +-
.../devicetree/bindings/arm/fsl.yaml | 6 +
.../bindings/serial/fsl,s32-linflexuart.txt | 22 +++
arch/arm64/Kconfig.platforms | 5 +
arch/arm64/boot/dts/freescale/Makefile | 2 +
arch/arm64/boot/dts/freescale/s32v234-evb.dts | 25 ++++
arch/arm64/boot/dts/freescale/s32v234.dtsi | 139 ++++++++++++++++++
arch/arm64/configs/defconfig | 3 +
drivers/tty/serial/Kconfig | 8 +-
drivers/tty/serial/fsl_linflexuart.c | 8 +-
include/uapi/linux/serial_core.h | 4 +-
11 files changed, 213 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
create mode 100644 arch/arm64/boot/dts/freescale/s32v234-evb.dts
create mode 100644 arch/arm64/boot/dts/freescale/s32v234.dtsi
--
2.22.0
^ permalink raw reply
* [PATCH v3 1/7] dt-bindings: arm: fsl: Add the S32V234-EVB board
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Eddy Petrisor
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
From: Eddy Petrișor <eddy.petrisor@nxp.com>
Add entry for the NXP S32V234 Customer Evaluation Board to the board/SoC
bindings.
Signed-off-by: Eddy Petrișor <eddy.petrisor@nxp.com>
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 7294ac36f4c0..597c563bdec9 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -309,4 +309,10 @@ properties:
- fsl,ls2088a-rdb
- const: fsl,ls2088a
+ - description: S32V234 based Boards
+ items:
+ - enum:
+ - fsl,s32v234-evb # S32V234-EVB2 Customer Evaluation Board
+ - const: fsl,s32v234
+
...
--
2.22.0
^ permalink raw reply related
* [PATCH v3 2/7] arm64: Introduce config for S32
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
From: Mihaela Martinas <Mihaela.Martinas@freescale.com>
Add configuration option for the NXP S32 platform family in
Kconfig.platforms. For starters, the only SoC supported will be Treerunner
(S32V234), with a single execution target: the S32V234-EVB (rev 29288)
board.
Signed-off-by: Mihaela Martinas <Mihaela.Martinas@freescale.com>
Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
---
arch/arm64/Kconfig.platforms | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 4778c775de1b..64bbe2d0b04f 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -210,6 +210,11 @@ config ARCH_ROCKCHIP
This enables support for the ARMv8 based Rockchip chipsets,
like the RK3368.
+config ARCH_S32
+ bool "NXP S32 SoC Family"
+ help
+ This enables support for the NXP S32 family of processors.
+
config ARCH_SEATTLE
bool "AMD Seattle SoC Family"
help
--
2.22.0
^ permalink raw reply related
* [PATCH v3 3/7] serial: fsl_linflexuart: Update compatible string
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
The "fsl,s32-linflexuart" compatible string is too generic. Make it SoC
specific.
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
---
drivers/tty/serial/fsl_linflexuart.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c
index 26b9601a0952..e54c65830e5e 100644
--- a/drivers/tty/serial/fsl_linflexuart.c
+++ b/drivers/tty/serial/fsl_linflexuart.c
@@ -127,7 +127,7 @@
static const struct of_device_id linflex_dt_ids[] = {
{
- .compatible = "fsl,s32-linflexuart",
+ .compatible = "fsl,s32v234-linflexuart",
},
{ /* sentinel */ }
};
@@ -801,7 +801,7 @@ static int __init linflex_early_console_setup(struct earlycon_device *device,
return 0;
}
-OF_EARLYCON_DECLARE(linflex, "fsl,s32-linflexuart",
+OF_EARLYCON_DECLARE(linflex, "fsl,s32v234-linflexuart",
linflex_early_console_setup);
#define LINFLEX_CONSOLE (&linflex_console)
--
2.22.0
^ permalink raw reply related
* [PATCH v3 4/7] serial: fsl_linflexuart: Be consistent with the name
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
For consistency reasons, spell the controller name as "LINFlexD" in
comments and documentation.
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
drivers/tty/serial/Kconfig | 8 ++++----
drivers/tty/serial/fsl_linflexuart.c | 4 ++--
include/uapi/linux/serial_core.h | 4 ++--
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4d545732aadc..81773425fb31 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1091,7 +1091,7 @@
mapped with the correct attributes.
linflex,<addr>
- Use early console provided by Freescale LinFlex UART
+ Use early console provided by Freescale LINFlexD UART
serial driver for NXP S32V234 SoCs. A valid base
address must be provided, and the serial port must
already be setup and configured.
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index b4fa2f7c96bd..3039c1ff68a9 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1453,18 +1453,18 @@ config SERIAL_FSL_LPUART_CONSOLE
you can make it the console by answering Y to this option.
config SERIAL_FSL_LINFLEXUART
- tristate "Freescale linflexuart serial port support"
+ tristate "Freescale LINFlexD UART serial port support"
select SERIAL_CORE
help
- Support for the on-chip linflexuart on some Freescale SOCs.
+ Support for the on-chip LINFlexD UART on some Freescale SOCs.
config SERIAL_FSL_LINFLEXUART_CONSOLE
- bool "Console on Freescale linflexuart serial port"
+ bool "Console on Freescale LINFlexD UART serial port"
depends on SERIAL_FSL_LINFLEXUART=y
select SERIAL_CORE_CONSOLE
select SERIAL_EARLYCON
help
- If you have enabled the linflexuart serial port on the Freescale
+ If you have enabled the LINFlexD UART serial port on the Freescale
SoCs, you can make it the console by answering Y to this option.
config SERIAL_CONEXANT_DIGICOLOR
diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c
index e54c65830e5e..b5889f8d80cb 100644
--- a/drivers/tty/serial/fsl_linflexuart.c
+++ b/drivers/tty/serial/fsl_linflexuart.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * Freescale linflexuart serial port driver
+ * Freescale LINFlexD UART serial port driver
*
* Copyright 2012-2016 Freescale Semiconductor, Inc.
* Copyright 2017-2018 NXP
@@ -938,5 +938,5 @@ static void __exit linflex_serial_exit(void)
module_init(linflex_serial_init);
module_exit(linflex_serial_exit);
-MODULE_DESCRIPTION("Freescale linflex serial port driver");
+MODULE_DESCRIPTION("Freescale LINFlexD serial port driver");
MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index f9e829416dcc..33bd9c411a31 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -296,7 +296,7 @@
/* Sunix UART */
#define PORT_SUNIX 121
-/* Freescale Linflex UART */
-#define PORT_LINFLEXUART 121
+/* Freescale LINFlexD UART */
+#define PORT_LINFLEXUART 122
#endif /* _UAPILINUX_SERIAL_CORE_H */
--
2.22.0
^ permalink raw reply related
* [PATCH v3 6/7] dt-bindings: serial: Document Freescale LINFlexD UART
From: Stefan-gabriel Mirea @ 2019-08-23 19:11 UTC (permalink / raw)
To: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, shawnguo@kernel.org, Leo Li
Cc: jslaby@suse.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica,
Larisa Ileana Grigore
In-Reply-To: <20190823191115.18490-1-stefan-gabriel.mirea@nxp.com>
From: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
Add documentation for the serial communication interface module (LINFlexD),
found in two instances on S32V234.
Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
Signed-off-by: Larisa Grigore <Larisa.Grigore@nxp.com>
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
---
.../bindings/serial/fsl,s32-linflexuart.txt | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
new file mode 100644
index 000000000000..f1bbe0826be5
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.txt
@@ -0,0 +1,22 @@
+* Freescale LINFlexD UART
+
+The LINFlexD controller implements several LIN protocol versions, as well as
+support for full-duplex UART communication through 8-bit and 9-bit frames.
+
+See chapter 47 ("LINFlexD") in the reference manual[1].
+
+Required properties:
+- compatible :
+ - "fsl,s32v234-linflexuart" for LINFlexD configured in UART mode, which
+ is compatible with the one integrated on S32V234 SoC
+- reg : Address and length of the register set for the device
+- interrupts : Should contain uart interrupt
+
+Example:
+uart0: serial@40053000 {
+ compatible = "fsl,s32v234-linflexuart";
+ reg = <0x0 0x40053000 0x0 0x1000>;
+ interrupts = <0 59 4>;
+};
+
+[1] https://www.nxp.com/webapp/Download?colCode=S32V234RM
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: Brendan Higgins @ 2019-08-23 19:20 UTC (permalink / raw)
To: shuah
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg
In-Reply-To: <bb9384cd-bd62-2190-e0da-ed3537aff171@kernel.org>
On Fri, Aug 23, 2019 at 12:04 PM shuah <shuah@kernel.org> wrote:
>
> On 8/23/19 12:56 PM, Brendan Higgins wrote:
> > On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote:
> >>
> >> On 8/23/19 11:54 AM, Brendan Higgins wrote:
> >>> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote:
> >>>>
> >>>> On 8/23/19 11:27 AM, Brendan Higgins wrote:
> >>>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
> >>>>>>
> >>>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote:
> >>>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
> >>>>>>>>
> >>>>>>>> Hi Brendan,
> >>>>>>>>
> >>>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
> >>>>>>>>> Add core facilities for defining unit tests; this provides a common way
> >>>>>>>>> to define test cases, functions that execute code which is under test
> >>>>>>>>> and determine whether the code under test behaves as expected; this also
> >>>>>>>>> provides a way to group together related test cases in test suites (here
> >>>>>>>>> we call them test_modules).
> >>>>>>>>>
> >>>>>>>>> Just define test cases and how to execute them for now; setting
> >>>>>>>>> expectations on code will be defined later.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >>>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> >>>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> >>>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> >>>>>>>>> ---
> >>>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
> >>>>>>>>> kunit/Kconfig | 17 ++++
> >>>>>>>>> kunit/Makefile | 1 +
> >>>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
> >>>>>>>>> 4 files changed, 388 insertions(+)
> >>>>>>>>> create mode 100644 include/kunit/test.h
> >>>>>>>>> create mode 100644 kunit/Kconfig
> >>>>>>>>> create mode 100644 kunit/Makefile
> >>>>>>>>> create mode 100644 kunit/test.c
> >>>>>>>>>
> >>>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> >>>>>>>>> new file mode 100644
> >>>>>>>>> index 0000000000000..e0b34acb9ee4e
> >>>>>>>>> --- /dev/null
> >>>>>>>>> +++ b/include/kunit/test.h
> >>>>>>>>> @@ -0,0 +1,179 @@
> >>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>>>>>>>> +/*
> >>>>>>>>> + * Base unit test (KUnit) API.
> >>>>>>>>> + *
> >>>>>>>>> + * Copyright (C) 2019, Google LLC.
> >>>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
> >>>>>>>>> + */
> >>>>>>>>> +
> >>>>>>>>> +#ifndef _KUNIT_TEST_H
> >>>>>>>>> +#define _KUNIT_TEST_H
> >>>>>>>>> +
> >>>>>>>>> +#include <linux/types.h>
> >>>>>>>>> +
> >>>>>>>>> +struct kunit;
> >>>>>>>>> +
> >>>>>>>>> +/**
> >>>>>>>>> + * struct kunit_case - represents an individual test case.
> >>>>>>>>> + * @run_case: the function representing the actual test case.
> >>>>>>>>> + * @name: the name of the test case.
> >>>>>>>>> + *
> >>>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> >>>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
> >>>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the
> >>>>>>>>> + * suite's init function and followed by the suite's exit function.
> >>>>>>>>> + *
> >>>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
> >>>>>>>>> + * macro; additionally, every array of test cases should be terminated with an
> >>>>>>>>> + * empty test case.
> >>>>>>>>> + *
> >>>>>>>>> + * Example:
> >>>>>>>>
> >>>>>>>> Can you fix these line continuations. It makes it very hard to read.
> >>>>>>>> Sorry for this late comment. These comments lines are longer than 80
> >>>>>>>> and wrap.
> >>>>>>>
> >>>>>>> None of the lines in this commit are over 80 characters in column
> >>>>>>> width. Some are exactly 80 characters (like above).
> >>>>>>>
> >>>>>>> My guess is that you are seeing the diff added text (+ ), which when
> >>>>>>> you add that to a line which is exactly 80 char in length ends up
> >>>>>>> being over 80 char in email. If you apply the patch you will see that
> >>>>>>> they are only 80 chars.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> There are several comment lines in the file that are way too long.
> >>>>>>>
> >>>>>>> Note that checkpatch also does not complain about any over 80 char
> >>>>>>> lines in this file.
> >>>>>>>
> >>>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please
> >>>>>>> confirm either way.
> >>>>>>>
> >>>>>>
> >>>>>> WARNING: Avoid unnecessary line continuations
> >>>>>> #258: FILE: include/kunit/test.h:137:
> >>>>>> + */ \
> >>>>>>
> >>>>>> total: 0 errors, 2 warnings, 388 lines checked
> >>>>>
> >>>>> Ah, okay so you don't like the warning about the line continuation.
> >>>>> That's not because it is over 80 char, but because there is a line
> >>>>> continuation after a comment. I don't really see a way to get rid of
> >>>>> it without removing the comment from inside the macro.
> >>>>>
> >>>>> I put this TODO there in the first place a Luis' request, and I put it
> >>>>> in the body of the macro because this macro already had a kernel-doc
> >>>>> comment and I didn't think that an implementation detail TODO belonged
> >>>>> in the user documentation.
> >>>>>
> >>>>>> Go ahead fix these. It appears there are few lines that either longer
> >>>>>> than 80. In general, I keep them around 75, so it is easier read.
> >>>>>
> >>>>> Sorry, the above is the only checkpatch warning other than the
> >>>>> reminder to update the MAINTAINERS file.
> >>>>>
> >>>>> Are you saying you want me to go through and make all the lines fit in
> >>>>> 75 char column width? I hope not because that is going to be a pretty
> >>>>> substantial change to make.
> >>>>>
> >>>>
> >>>> There are two things with these comment lines. One is checkpatch
> >>>> complaining and the other is general readability.
> >>>
> >>> So for the checkpatch warning, do you want me to move the comment out
> >>> of the macro body into the kernel-doc comment? I don't really think it
> >>> is the right place for a comment of this nature, but I think it is
> >>> probably better than dropping it entirely (I don't see how else to do
> >>> it without just removing the comment entirely).
> >>>
> >>
> >> Don't drop the comments. It makes perfect sense to turn this into a
> >> kernel-doc comment.
> >
> > I am fine with that. I will do that in a subsequent revision once we
> > figure out the column limit issue.
> >
> >> We are going back forth on this a lot. I see several lines 81+ in
> >> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am
> >> fine with it if you want to convert these to kernel-doc comments.
> >> I think it makes perfect sense.
> >
> > Okay, so this is interesting. When I look at the applied patches in my
> > local repo, I don't see any 81+ lines. So it seems that something
> > interesting is going on here.
> >
> > To be clear (sorry for the stupid question) you are seeing the issue
> > after you applied the patch, and not in the patch file itself?
> >
>
> I am using my normal workflow. My pre-commit check is catching this.
> Just this patch.
Okay, *that* is super strange!
So I have lines in this patch (01/18) that are exactly 80 char wide
and I was thinking that it might be an off by one issue on either my
workflow or your workflow, but I have lines in other patches that are
exactly 80 char wide and our setups agree that they are fine, so I
really am not sure what's going on here.
It sounds like you are only seeing the issue in only a couple places,
do you mind calling out the specific lines that are problematic?
> All others are good other than the 9/18 BUG() issue.
> > Since we are still at OSS, would you mind if we meet up this afternoon
> > so I can see this issue you are seeing? I imagine we should get this
> > figured out pretty quickly.
> >
>
> Yeah. Would have been nice. I am not at oss today.
Dang.
^ permalink raw reply
* Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core
From: shuah @ 2019-08-23 19:43 UTC (permalink / raw)
To: Brendan Higgins
Cc: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Petr Mladek, Randy Dunlap, Richard Weinberger,
David Rientjes, Steven Rostedt, wfg, shuah
In-Reply-To: <CAFd5g47bJjp94bbCRmho8yUXNWx3PpQ4Cu6Y1UnErKVKWuedNw@mail.gmail.com>
On 8/23/19 1:20 PM, Brendan Higgins wrote:
> On Fri, Aug 23, 2019 at 12:04 PM shuah <shuah@kernel.org> wrote:
>>
>> On 8/23/19 12:56 PM, Brendan Higgins wrote:
>>> On Fri, Aug 23, 2019 at 11:32 AM shuah <shuah@kernel.org> wrote:
>>>>
>>>> On 8/23/19 11:54 AM, Brendan Higgins wrote:
>>>>> On Fri, Aug 23, 2019 at 10:34 AM shuah <shuah@kernel.org> wrote:
>>>>>>
>>>>>> On 8/23/19 11:27 AM, Brendan Higgins wrote:
>>>>>>> On Fri, Aug 23, 2019 at 10:05 AM shuah <shuah@kernel.org> wrote:
>>>>>>>>
>>>>>>>> On 8/23/19 10:48 AM, Brendan Higgins wrote:
>>>>>>>>> On Fri, Aug 23, 2019 at 8:33 AM shuah <shuah@kernel.org> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Brendan,
>>>>>>>>>>
>>>>>>>>>> On 8/20/19 5:20 PM, Brendan Higgins wrote:
>>>>>>>>>>> Add core facilities for defining unit tests; this provides a common way
>>>>>>>>>>> to define test cases, functions that execute code which is under test
>>>>>>>>>>> and determine whether the code under test behaves as expected; this also
>>>>>>>>>>> provides a way to group together related test cases in test suites (here
>>>>>>>>>>> we call them test_modules).
>>>>>>>>>>>
>>>>>>>>>>> Just define test cases and how to execute them for now; setting
>>>>>>>>>>> expectations on code will be defined later.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>>>>>>>>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>>>>>>> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>>>>>>>>>>> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
>>>>>>>>>>> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>>>>>>>>>>> ---
>>>>>>>>>>> include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>> kunit/Kconfig | 17 ++++
>>>>>>>>>>> kunit/Makefile | 1 +
>>>>>>>>>>> kunit/test.c | 191 +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>> 4 files changed, 388 insertions(+)
>>>>>>>>>>> create mode 100644 include/kunit/test.h
>>>>>>>>>>> create mode 100644 kunit/Kconfig
>>>>>>>>>>> create mode 100644 kunit/Makefile
>>>>>>>>>>> create mode 100644 kunit/test.c
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>>>>>>>>>> new file mode 100644
>>>>>>>>>>> index 0000000000000..e0b34acb9ee4e
>>>>>>>>>>> --- /dev/null
>>>>>>>>>>> +++ b/include/kunit/test.h
>>>>>>>>>>> @@ -0,0 +1,179 @@
>>>>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>>>>>>>> +/*
>>>>>>>>>>> + * Base unit test (KUnit) API.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Copyright (C) 2019, Google LLC.
>>>>>>>>>>> + * Author: Brendan Higgins <brendanhiggins@google.com>
>>>>>>>>>>> + */
>>>>>>>>>>> +
>>>>>>>>>>> +#ifndef _KUNIT_TEST_H
>>>>>>>>>>> +#define _KUNIT_TEST_H
>>>>>>>>>>> +
>>>>>>>>>>> +#include <linux/types.h>
>>>>>>>>>>> +
>>>>>>>>>>> +struct kunit;
>>>>>>>>>>> +
>>>>>>>>>>> +/**
>>>>>>>>>>> + * struct kunit_case - represents an individual test case.
>>>>>>>>>>> + * @run_case: the function representing the actual test case.
>>>>>>>>>>> + * @name: the name of the test case.
>>>>>>>>>>> + *
>>>>>>>>>>> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
>>>>>>>>>>> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
>>>>>>>>>>> + * test case is associated with a &struct kunit_suite and will be run after the
>>>>>>>>>>> + * suite's init function and followed by the suite's exit function.
>>>>>>>>>>> + *
>>>>>>>>>>> + * A test case should be static and should only be created with the KUNIT_CASE()
>>>>>>>>>>> + * macro; additionally, every array of test cases should be terminated with an
>>>>>>>>>>> + * empty test case.
>>>>>>>>>>> + *
>>>>>>>>>>> + * Example:
>>>>>>>>>>
>>>>>>>>>> Can you fix these line continuations. It makes it very hard to read.
>>>>>>>>>> Sorry for this late comment. These comments lines are longer than 80
>>>>>>>>>> and wrap.
>>>>>>>>>
>>>>>>>>> None of the lines in this commit are over 80 characters in column
>>>>>>>>> width. Some are exactly 80 characters (like above).
>>>>>>>>>
>>>>>>>>> My guess is that you are seeing the diff added text (+ ), which when
>>>>>>>>> you add that to a line which is exactly 80 char in length ends up
>>>>>>>>> being over 80 char in email. If you apply the patch you will see that
>>>>>>>>> they are only 80 chars.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> There are several comment lines in the file that are way too long.
>>>>>>>>>
>>>>>>>>> Note that checkpatch also does not complain about any over 80 char
>>>>>>>>> lines in this file.
>>>>>>>>>
>>>>>>>>> Sorry if I am misunderstanding what you are trying to tell me. Please
>>>>>>>>> confirm either way.
>>>>>>>>>
>>>>>>>>
>>>>>>>> WARNING: Avoid unnecessary line continuations
>>>>>>>> #258: FILE: include/kunit/test.h:137:
>>>>>>>> + */ \
>>>>>>>>
>>>>>>>> total: 0 errors, 2 warnings, 388 lines checked
>>>>>>>
>>>>>>> Ah, okay so you don't like the warning about the line continuation.
>>>>>>> That's not because it is over 80 char, but because there is a line
>>>>>>> continuation after a comment. I don't really see a way to get rid of
>>>>>>> it without removing the comment from inside the macro.
>>>>>>>
>>>>>>> I put this TODO there in the first place a Luis' request, and I put it
>>>>>>> in the body of the macro because this macro already had a kernel-doc
>>>>>>> comment and I didn't think that an implementation detail TODO belonged
>>>>>>> in the user documentation.
>>>>>>>
>>>>>>>> Go ahead fix these. It appears there are few lines that either longer
>>>>>>>> than 80. In general, I keep them around 75, so it is easier read.
>>>>>>>
>>>>>>> Sorry, the above is the only checkpatch warning other than the
>>>>>>> reminder to update the MAINTAINERS file.
>>>>>>>
>>>>>>> Are you saying you want me to go through and make all the lines fit in
>>>>>>> 75 char column width? I hope not because that is going to be a pretty
>>>>>>> substantial change to make.
>>>>>>>
>>>>>>
>>>>>> There are two things with these comment lines. One is checkpatch
>>>>>> complaining and the other is general readability.
>>>>>
>>>>> So for the checkpatch warning, do you want me to move the comment out
>>>>> of the macro body into the kernel-doc comment? I don't really think it
>>>>> is the right place for a comment of this nature, but I think it is
>>>>> probably better than dropping it entirely (I don't see how else to do
>>>>> it without just removing the comment entirely).
>>>>>
>>>>
>>>> Don't drop the comments. It makes perfect sense to turn this into a
>>>> kernel-doc comment.
>>>
>>> I am fine with that. I will do that in a subsequent revision once we
>>> figure out the column limit issue.
>>>
>>>> We are going back forth on this a lot. I see several lines 81+ in
>>>> this file. I am at 5.3-rc5 and my commit hooks aren't happy. I am
>>>> fine with it if you want to convert these to kernel-doc comments.
>>>> I think it makes perfect sense.
>>>
>>> Okay, so this is interesting. When I look at the applied patches in my
>>> local repo, I don't see any 81+ lines. So it seems that something
>>> interesting is going on here.
>>>
>>> To be clear (sorry for the stupid question) you are seeing the issue
>>> after you applied the patch, and not in the patch file itself?
>>>
>>
>> I am using my normal workflow. My pre-commit check is catching this.
>> Just this patch.
>
> Okay, *that* is super strange!
>
> So I have lines in this patch (01/18) that are exactly 80 char wide
> and I was thinking that it might be an off by one issue on either my
> workflow or your workflow, but I have lines in other patches that are
> exactly 80 char wide and our setups agree that they are fine, so I
> really am not sure what's going on here.
>
> It sounds like you are only seeing the issue in only a couple places,
> do you mind calling out the specific lines that are problematic?
Take a look at the comment blocks. That is where the problem are.
>
>> All others are good other than the 9/18 BUG() issue.
>>> Since we are still at OSS, would you mind if we meet up this afternoon
>>> so I can see this issue you are seeing? I imagine we should get this
>>> figured out pretty quickly.
>>>
>>
>> Yeah. Would have been nice. I am not at oss today.
>
> Dang.
>
thanks,
-- Shuah
^ permalink raw reply
* [PATCH v15 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework
From: Brendan Higgins @ 2019-08-24 1:34 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
## TL;DR
This revision addresses comments from Shuah by fixing a couple
checkpatch warnings and fixing some comment readability issues. No API
or major structual changes have been made since v13.
## Background
This patch set proposes KUnit, a lightweight unit testing and mocking
framework for the Linux kernel.
Unlike Autotest and kselftest, KUnit is a true unit testing framework;
it does not require installing the kernel on a test machine or in a VM
(however, KUnit still allows you to run tests on test machines or in VMs
if you want[1]) and does not require tests to be written in userspace
running on a host kernel. Additionally, KUnit is fast: From invocation
to completion KUnit can run several dozen tests in about a second.
Currently, the entire KUnit test suite for KUnit runs in under a second
from the initial invocation (build time excluded).
KUnit is heavily inspired by JUnit, Python's unittest.mock, and
Googletest/Googlemock for C++. KUnit provides facilities for defining
unit test cases, grouping related test cases into test suites, providing
common infrastructure for running tests, mocking, spying, and much more.
### What's so special about unit testing?
A unit test is supposed to test a single unit of code in isolation,
hence the name. There should be no dependencies outside the control of
the test; this means no external dependencies, which makes tests orders
of magnitudes faster. Likewise, since there are no external dependencies,
there are no hoops to jump through to run the tests. Additionally, this
makes unit tests deterministic: a failing unit test always indicates a
problem. Finally, because unit tests necessarily have finer granularity,
they are able to test all code paths easily solving the classic problem
of difficulty in exercising error handling code.
### Is KUnit trying to replace other testing frameworks for the kernel?
No. Most existing tests for the Linux kernel are end-to-end tests, which
have their place. A well tested system has lots of unit tests, a
reasonable number of integration tests, and some end-to-end tests. KUnit
is just trying to address the unit test space which is currently not
being addressed.
### More information on KUnit
There is a bunch of documentation near the end of this patch set that
describes how to use KUnit and best practices for writing unit tests.
For convenience I am hosting the compiled docs here[2].
Additionally for convenience, I have applied these patches to a
branch[3]. The repo may be cloned with:
git clone https://kunit.googlesource.com/linux
This patchset is on the kunit/rfc/v5.3/v15 branch.
## Changes Since Last Version
- Moved comment from inline in macro to kernel-doc to address checkpatch
warning.
- Demoted BUG() to WARN_ON.
- Formatted some kernel-doc comments to make them more readible.
[1] https://google.github.io/kunit-docs/third_party/kernel/docs/usage.html#kunit-on-non-uml-architectures
[2] https://google.github.io/kunit-docs/third_party/kernel/docs/
[3] https://kunit.googlesource.com/linux/+/kunit/rfc/v5.3/v15
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply
* [PATCH v15 02/18] kunit: test: add test resource management API
From: Brendan Higgins @ 2019-08-24 1:34 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190824013425.175645-1-brendanhiggins@google.com>
Create a common API for test managed resources like memory and test
objects. A lot of times a test will want to set up infrastructure to be
used in test cases; this could be anything from just wanting to allocate
some memory to setting up a driver stack; this defines facilities for
creating "test resources" which are managed by the test infrastructure
and are automatically cleaned up at the conclusion of the test.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
---
include/kunit/test.h | 187 +++++++++++++++++++++++++++++++++++++++++++
kunit/test.c | 163 +++++++++++++++++++++++++++++++++++++
2 files changed, 350 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index e30d1bf2fb68..6781c756f11b 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -9,8 +9,72 @@
#ifndef _KUNIT_TEST_H
#define _KUNIT_TEST_H
+#include <linux/slab.h>
#include <linux/types.h>
+struct kunit_resource;
+
+typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *);
+typedef void (*kunit_resource_free_t)(struct kunit_resource *);
+
+/**
+ * struct kunit_resource - represents a *test managed resource*
+ * @allocation: for the user to store arbitrary data.
+ * @free: a user supplied function to free the resource. Populated by
+ * kunit_alloc_resource().
+ *
+ * Represents a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * struct kunit_kmalloc_params {
+ * size_t size;
+ * gfp_t gfp;
+ * };
+ *
+ * static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+ * {
+ * struct kunit_kmalloc_params *params = context;
+ * res->allocation = kmalloc(params->size, params->gfp);
+ *
+ * if (!res->allocation)
+ * return -ENOMEM;
+ *
+ * return 0;
+ * }
+ *
+ * static void kunit_kmalloc_free(struct kunit_resource *res)
+ * {
+ * kfree(res->allocation);
+ * }
+ *
+ * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+ * {
+ * struct kunit_kmalloc_params params;
+ * struct kunit_resource *res;
+ *
+ * params.size = size;
+ * params.gfp = gfp;
+ *
+ * res = kunit_alloc_resource(test, kunit_kmalloc_init,
+ * kunit_kmalloc_free, ¶ms);
+ * if (res)
+ * return res->allocation;
+ *
+ * return NULL;
+ * }
+ */
+struct kunit_resource {
+ void *allocation;
+ kunit_resource_free_t free;
+
+ /* private: internal use only. */
+ struct list_head node;
+};
+
struct kunit;
/**
@@ -114,6 +178,13 @@ struct kunit {
* with the test case have terminated.
*/
bool success; /* Read only after test_case finishes! */
+ spinlock_t lock; /* Guards all mutable test state. */
+ /*
+ * Because resources is a list that may be updated multiple times (with
+ * new resources) from any thread associated with a test case, we must
+ * protect it with some type of lock.
+ */
+ struct list_head resources; /* Protected by lock. */
};
void kunit_init_test(struct kunit *test, const char *name);
@@ -147,6 +218,122 @@ int kunit_run_tests(struct kunit_suite *suite);
} \
late_initcall(kunit_suite_init##suite)
+/*
+ * Like kunit_alloc_resource() below, but returns the struct kunit_resource
+ * object that contains the allocation. This is mostly for testing purposes.
+ */
+struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ gfp_t internal_gfp,
+ void *context);
+
+/**
+ * kunit_alloc_resource() - Allocates a *test managed resource*.
+ * @test: The test context object.
+ * @init: a user supplied function to initialize the resource.
+ * @free: a user supplied function to free the resource.
+ * @internal_gfp: gfp to use for internal allocations, if unsure, use GFP_KERNEL
+ * @context: for the user to pass in arbitrary data to the init function.
+ *
+ * Allocates a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case. See &struct kunit_resource for an
+ * example.
+ *
+ * NOTE: KUnit needs to allocate memory for each kunit_resource object. You must
+ * specify an @internal_gfp that is compatible with the use context of your
+ * resource.
+ */
+static inline void *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ gfp_t internal_gfp,
+ void *context)
+{
+ struct kunit_resource *res;
+
+ res = kunit_alloc_and_get_resource(test, init, free, internal_gfp,
+ context);
+
+ if (res)
+ return res->allocation;
+
+ return NULL;
+}
+
+typedef bool (*kunit_resource_match_t)(struct kunit *test,
+ const void *res,
+ void *match_data);
+
+/**
+ * kunit_resource_instance_match() - Match a resource with the same instance.
+ * @test: Test case to which the resource belongs.
+ * @res: The data stored in kunit_resource->allocation.
+ * @match_data: The resource pointer to match against.
+ *
+ * An instance of kunit_resource_match_t that matches a resource whose
+ * allocation matches @match_data.
+ */
+static inline bool kunit_resource_instance_match(struct kunit *test,
+ const void *res,
+ void *match_data)
+{
+ return res == match_data;
+}
+
+/**
+ * kunit_resource_destroy() - Find a kunit_resource and destroy it.
+ * @test: Test case to which the resource belongs.
+ * @match: Match function. Returns whether a given resource matches @match_data.
+ * @free: Must match free on the kunit_resource to free.
+ * @match_data: Data passed into @match.
+ *
+ * Free the latest kunit_resource of @test for which @free matches the
+ * kunit_resource_free_t associated with the resource and for which @match
+ * returns true.
+ *
+ * RETURNS:
+ * 0 if kunit_resource is found and freed, -ENOENT if not found.
+ */
+int kunit_resource_destroy(struct kunit *test,
+ kunit_resource_match_t match,
+ kunit_resource_free_t free,
+ void *match_data);
+
+/**
+ * kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * Just like `kmalloc(...)`, except the allocation is managed by the test case
+ * and is automatically cleaned up after the test case concludes. See &struct
+ * kunit_resource for more information.
+ */
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp);
+
+/**
+ * kunit_kfree() - Like kfree except for allocations managed by KUnit.
+ * @test: The test case to which the resource belongs.
+ * @ptr: The memory allocation to free.
+ */
+void kunit_kfree(struct kunit *test, const void *ptr);
+
+/**
+ * kunit_kzalloc() - Just like kunit_kmalloc(), but zeroes the allocation.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * See kzalloc() and kunit_kmalloc() for more information.
+ */
+static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ return kunit_kmalloc(test, size, gfp | __GFP_ZERO);
+}
+
+void kunit_cleanup(struct kunit *test);
+
void __printf(3, 4) kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...);
diff --git a/kunit/test.c b/kunit/test.c
index d3dda359f99b..68b1037ab74d 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -122,6 +122,8 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
void kunit_init_test(struct kunit *test, const char *name)
{
+ spin_lock_init(&test->lock);
+ INIT_LIST_HEAD(&test->resources);
test->name = name;
test->success = true;
}
@@ -153,6 +155,8 @@ static void kunit_run_case(struct kunit_suite *suite,
if (suite->exit)
suite->exit(&test);
+ kunit_cleanup(&test);
+
test_case->success = test.success;
}
@@ -173,6 +177,165 @@ int kunit_run_tests(struct kunit_suite *suite)
return 0;
}
+struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ gfp_t internal_gfp,
+ void *context)
+{
+ struct kunit_resource *res;
+ int ret;
+
+ res = kzalloc(sizeof(*res), internal_gfp);
+ if (!res)
+ return NULL;
+
+ ret = init(res, context);
+ if (ret)
+ return NULL;
+
+ res->free = free;
+ spin_lock(&test->lock);
+ list_add_tail(&res->node, &test->resources);
+ spin_unlock(&test->lock);
+
+ return res;
+}
+
+static void kunit_resource_free(struct kunit *test, struct kunit_resource *res)
+{
+ res->free(res);
+ kfree(res);
+}
+
+static struct kunit_resource *kunit_resource_find(struct kunit *test,
+ kunit_resource_match_t match,
+ kunit_resource_free_t free,
+ void *match_data)
+{
+ struct kunit_resource *resource;
+
+ lockdep_assert_held(&test->lock);
+
+ list_for_each_entry_reverse(resource, &test->resources, node) {
+ if (resource->free != free)
+ continue;
+ if (match(test, resource->allocation, match_data))
+ return resource;
+ }
+
+ return NULL;
+}
+
+static struct kunit_resource *kunit_resource_remove(
+ struct kunit *test,
+ kunit_resource_match_t match,
+ kunit_resource_free_t free,
+ void *match_data)
+{
+ struct kunit_resource *resource;
+
+ spin_lock(&test->lock);
+ resource = kunit_resource_find(test, match, free, match_data);
+ if (resource)
+ list_del(&resource->node);
+ spin_unlock(&test->lock);
+
+ return resource;
+}
+
+int kunit_resource_destroy(struct kunit *test,
+ kunit_resource_match_t match,
+ kunit_resource_free_t free,
+ void *match_data)
+{
+ struct kunit_resource *resource;
+
+ resource = kunit_resource_remove(test, match, free, match_data);
+
+ if (!resource)
+ return -ENOENT;
+
+ kunit_resource_free(test, resource);
+ return 0;
+}
+
+struct kunit_kmalloc_params {
+ size_t size;
+ gfp_t gfp;
+};
+
+static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_kmalloc_params *params = context;
+
+ res->allocation = kmalloc(params->size, params->gfp);
+ if (!res->allocation)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kunit_kmalloc_free(struct kunit_resource *res)
+{
+ kfree(res->allocation);
+}
+
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ struct kunit_kmalloc_params params = {
+ .size = size,
+ .gfp = gfp
+ };
+
+ return kunit_alloc_resource(test,
+ kunit_kmalloc_init,
+ kunit_kmalloc_free,
+ gfp,
+ ¶ms);
+}
+
+void kunit_kfree(struct kunit *test, const void *ptr)
+{
+ int rc;
+
+ rc = kunit_resource_destroy(test,
+ kunit_resource_instance_match,
+ kunit_kmalloc_free,
+ (void *)ptr);
+
+ WARN_ON(rc);
+}
+
+void kunit_cleanup(struct kunit *test)
+{
+ struct kunit_resource *resource;
+
+ /*
+ * test->resources is a stack - each allocation must be freed in the
+ * reverse order from which it was added since one resource may depend
+ * on another for its entire lifetime.
+ * Also, we cannot use the normal list_for_each constructs, even the
+ * safe ones because *arbitrary* nodes may be deleted when
+ * kunit_resource_free is called; the list_for_each_safe variants only
+ * protect against the current node being deleted, not the next.
+ */
+ while (true) {
+ spin_lock(&test->lock);
+ if (list_empty(&test->resources)) {
+ spin_unlock(&test->lock);
+ break;
+ }
+ resource = list_last_entry(&test->resources,
+ struct kunit_resource,
+ node);
+ list_del(&resource->node);
+ spin_unlock(&test->lock);
+
+ kunit_resource_free(test, resource);
+ }
+}
+
void kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...)
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox