Date:   Thu, 15 Apr 2021 09:19:36 +0800
From:   David Gow <davidgow@...gle.com>
To:     Daniel Latypov <dlatypov@...gle.com>
Cc:     Jonathan Corbet <corbet@....net>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Marco Elver <elver@...gle.com>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        KUnit Development <kunit-dev@...glegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] Documentation: dev-tools: Add Testing Overview

On Thu, Apr 15, 2021 at 12:30 AM Daniel Latypov <dlatypov@...gle.com> wrote:
>
> On Wed, Apr 14, 2021 at 1:15 AM David Gow <davidgow@...gle.com> wrote:
> >
> > The kernel now has a number of testing and debugging tools, and we've
> > seen a bit of confusion about what the differences between them are.
> >
> > Add a basic documentation outlining the testing tools, when to use each,
> > and how they interact.
> >
> > This is a pretty quick overview rather than the idealised "kernel
> > testing guide" that'd probably be optimal, but given the number of times
> > questions like "When do you use KUnit and when do you use Kselftest?"
> > are being asked, it seemed worth at least having something. Hopefully
> > this can form the basis for more detailed documentation later.
> >
> > Signed-off-by: David Gow <davidgow@...gle.com>
>
> Reviewed-by: Daniel Latypov <dlatypov@...gle.com>
>
> Looks good to me. Some minor typos and nits about wording here and there.
>

Thanks: I'll send out v3 with some fixes to your suggestions soon.

Cheers,
-- David

> > ---
> > Thanks, everyone, for the comments on the doc. I've made a few of the
> > suggested changes. Please let me know what you think!
> >
> > -- David
> >
> > Changes since v1:
> > https://lore.kernel.org/linux-kselftest/20210410070529.4113432-1-davidgow@google.com/
> > - Note KUnit's speed and that one should provide selftests for syscalls
> > - Mention lockdep as a Dynamic Analysis Tool
> > - Refer to "Dynamic Analysis Tools" instead of "Sanitizers"
> > - A number of minor formatting tweaks and rewordings for clarity.
> >
> > Not changed:
> > - I haven't included an exhaustive list of differences, advantages, etc,
> >   between KUnit and kselftest: for now, the doc continues to focus on
> >   the difference between 'in-kernel' and 'userspace' testing here.
> > - Similarly, I'm not linking out to docs defining and describing "Unit"
> >   tests versus "End-to-end" tests. None of the existing documentation
> >   elsewhere quite matches what we do in the kernel perfectly, so it
> >   seems less confusing to focus on the 'in-kernel'/'userspace'
> >   distinction, and leave other definitions as a passing mention for
> >   those who are already familiar with the concepts.
> > - I haven't linked to any talk videos here: a few of them are linked on
> >   (e.g.) the KUnit webpage, but I wanted to keep the Kernel documentation
> >   more self-contained for now. No objection to adding them in a follow-up
> >   patch if people feel strongly about it, though.
> > - The link from index.rst to this doc is unchanged. I personally think
> >   that the link is prominent enough there: it's the first link, and
> >   shows up a few times. One possibility if people disagreed would be to
> >   merge this page with the index, but given not all dev-tools are going
> >   to be testing-related, it seemed a bit arrogant. :-)
> >
> >  Documentation/dev-tools/index.rst            |   3 +
> >  Documentation/dev-tools/testing-overview.rst | 117 +++++++++++++++++++
> >  2 files changed, 120 insertions(+)
> >  create mode 100644 Documentation/dev-tools/testing-overview.rst
> >
> > diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
> > index 1b1cf4f5c9d9..f590e5860794 100644
> > --- a/Documentation/dev-tools/index.rst
> > +++ b/Documentation/dev-tools/index.rst
> > @@ -7,6 +7,8 @@ be used to work on the kernel. For now, the documents have been pulled
> >  together without any significant effort to integrate them into a coherent
> >  whole; patches welcome!
> >
> > +A brief overview of testing-specific tools can be found in :doc:`testing-overview`.
> > +
> >  .. class:: toc-title
> >
> >            Table of contents
> > @@ -14,6 +16,7 @@ whole; patches welcome!
> >  .. toctree::
> >     :maxdepth: 2
> >
> > +   testing-overview
> >     coccinelle
> >     sparse
> >     kcov
> > diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst
> > new file mode 100644
> > index 000000000000..ce36a8cdf6b5
> > --- /dev/null
> > +++ b/Documentation/dev-tools/testing-overview.rst
> > @@ -0,0 +1,117 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +====================
> > +Kernel Testing Guide
> > +====================
> > +
> > +
> > +There are a number of different tools for testing the Linux kernel, so knowing
> > +when to use each of them can be a challenge. This document provides a rough
> > +overview of their differences, and how they fit together.
> > +
> > +
> > +Writing and Running Tests
> > +=========================
> > +
> > +The bulk of kernel tests are written using either the kselftest or KUnit
> > +frameworks. These both provide infrastructure to help make running tests and
> > +groups of tests easier, as well as providing helpers to aid in writing new
> > +tests.
> > +
> > +If you're looking to verify the behaviour of the Kernel — particularly specific
> > +parts of the kernel — then you'll want to use KUnit or kselftest.
> > +
> > +
> > +The Difference Between KUnit and kselftest
> > +------------------------------------------
> > +
> > +KUnit (Documentation/dev-tools/kunit/index.rst) is an entirely in-kernel system
> > +for "white box" testing: because test code is part of the kernel, it can access
> > +internal structures and functions which aren't exposed to userspace.
> > +
> > +KUnit tests therefore are best written against small, self-contained parts
> > +of the kernel, which can be tested in isolation. This aligns well with the
> > +concept of 'unit' testing.
> > +
> > +For example, a KUnit test might test an individual kernel function (or even a
> > +single codepath through a function, such as an error handling case), rather
> > +than a feature as a whole.
> > +
> > +This also makes KUnit tests very fast to build and run, allowing them to be
> > +run frequently as part of the development process.
> > +
> > +There is a KUnit test style guide which may give further pointers in
> > +Documentation/dev-tools/kunit/style.rst
> > +
> > +
> > +kselftest (Documentation/dev-tools/kselftest.rst), on the other hand, is
> > +largely implemented in userspace, and tests are normal userspace scripts or
> > +programs.
> > +
> > +This makes it easier to write more complicated tests, or tests which need to
> > +manipulate the overall system state more (e.g., spawning processes, etc.).
> > +However, it's not possible to call kernel functions directly from kselftest.
> > +This means that only kernel functionality which is exposed to userspace somhow
>
> *s/somhow/somehow
>

Whoops: fixed.

> > +(e.g. by a syscall, device, filesystem, etc.) can be tested with kselftest.  To
> > +work around this, some tests include a companion kernel module which exposes
> > +more information or functionality. If a test runs mostly or entirely within the
> > +kernel, however,  KUnit may be the more appropriate tool.
>
> I like this slightly tweaked wording better, thanks.
> Still might be a bit confusing for a reader to see "it's not possible"
> => "it's possible if you have a companion module," but I'm happy
> enough with it as-is.
>

Yeah: I experimented a bit with a couple of other ways to word this,
but it's difficult to find a succinct way of describing that it isn't
possible from a purely-userspace test, but that a workaround exists
and is used. This is what I ended up with, but I'm happy to change it
further (or see it changed in a follow-up) if someone has a brilliant
turn of phrase for it. :-)

> > +
> > +kselftest is therefore suited well to tests of whole features, as these will
> > +expose an interface to userspace, which can be tested, but not implementation
> > +details. This aligns well with 'system' or 'end-to-end' testing.
> > +
> > +For example, all new system calls should be accompanied by kselftest tests.
> > +
> > +Code Coverage Tools
> > +===================
> > +
> > +The Linux Kernel supports two different code coverage measurement tools. These
> > +can be used to verify that a test is executing particular functions or lines
> > +of code. This is useful for determining how much of the kernel is being tested,
> > +and for finding corner-cases which are not covered by the appropriate test.
> > +
> > +:doc:`gcov` is GCC's coverage testing tool, which can be used with the kernel
> > +to get global or per-module coverage. Unlike KCOV, it does not record per-task
> > +coverage. Coverage data can be read from debugfs, and interpreted using the
> > +usual gcov tooling.
> > +
> > +:doc:`kcov` is a feature which can be built in to the kernel to allow
> > +capturing coverage on a per-task level. It's therefore useful for fuzzing and
> > +other situations where information about code executed during, for example, a
> > +single syscall is useful.
> > +
> > +
> > +Dynamic Analysis Tools
> > +======================
> > +
> > +The kernel also supports a number of dynamic analysis tools, which attempt to
> > +detect classes of issues when the occur in a running kernel. These typically
>
> *s/the occur/they occur
>

Whoops: fixed.

> > +look for undefined behaviour of some kind, such as invalid memory accesses,
>
> nit: "look for undefined behaviour of some kind"
> Given that I think most readers will interpret UB in the sense that C
> uses it, this might be a bit misleading. E.g. lockdep errors aren't UB
> in that sense.
>
> Perhaps we can reword this to "look for invalid behaviour" or even
> just "look for bugs"
>

I've reworded this to say that each tool "looks for a different class of bugs".

> > +concurrency issues such as data races, or other undefined behaviour like
> > +integer overflows.
> > +
> > +Some of these tools are listed below:
> > +
> > +* kmemleak detects possible memory leaks. See
> > +  Documentation/dev-tools/kmemleak.rst
> > +* KASAN detects invalid memory accesses such as out-of-bounds and
> > +  use-after-free errors. See Documentation/dev-tools/kasan.rst
> > +* UBSAN detects behaviour that is undefined by the C standard, like integer
> > +  overflows. See Documentation/dev-tools/ubsan.rst
> > +* KCSAN detects data races. See Documentation/dev-tools/kcsan.rst
> > +* KFENCE is a low-overhead detector of memory issues, which is much faster than
> > +  KASAN and can be used in production. See Documentation/dev-tools/kfence.rst
> > +* lockdep is a locking correctness validator. See
> > +  Documentation/locking/lockdep-design.rst
> > +* There are several other pieces of debug instrumentation in the kernel, many
> > +  of which can be found in lib/Kconfig.debug
> > +
> > +These tools tend to test the kernel as a whole, and do not "pass" like
> > +kselftest or KUnit tests. They can be combined with KUnit or kselftest by
> > +running tests on a kernel with a sanitizer enabled: you can then be sure
>
> nit: we refer to "sanitizers" again, I assume this needs to be updated as well?
>

Yeah: I missed this one: it now just refers to "tools".

> > +that none of these errors are occurring during the test.
> > +
> > +Some of these tools integrate with KUnit or kselftest and will
> > +automatically fail tests if an issue is detected.
> > +
> > --
> > 2.31.1.295.g9ea45b61b8-goog
> >

Powered by blists - more mailing lists