Re: [PATCH v2 2/2] Documentation: Add debugfs docs with run after boot

From: David Gow <davidgow@google.com>
To: Rae Moar <rmoar@google.com>
Cc: shuah@kernel.org, dlatypov@google.com, brendan.higgins@linux.dev,
	sadiyakazi@google.com, linux-kselftest@vger.kernel.org,
	kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] Documentation: Add debugfs docs with run after boot
Date: Thu, 14 Sep 2023 17:06:43 +0800	[thread overview]
Message-ID: <CABVgOS=0coPxNE12EsOb-y-VGQ4hafSiNiVj8sLfCCG8LLz8Vw@mail.gmail.com> (raw)
In-Reply-To: <20230908213148.3610108-2-rmoar@google.com>

[-- Attachment #1: Type: text/plain, Size: 3745 bytes --]

On Sat, 9 Sept 2023 at 05:32, Rae Moar <rmoar@google.com> wrote:
>
> Expand the documentation on the KUnit debugfs filesystem on the
> run_manual.rst page.
>
> Add section describing how to access results using debugfs.
>
> Add section describing how to run tests after boot using debugfs.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> Co-developed-by: Sadiya Kazi <sadiyakazi@google.com>
> Signed-off-by: Sadiya Kazi <sadiyakazi@google.com>
> ---

Looks good to me, a few nitpicks, and the fact that we'll probably
need to add something about init section suites when those are
implemented.

(Also, since you sent the email, your sign off should be at the bottom
of the list above.)

>  Documentation/dev-tools/kunit/run_manual.rst | 45 ++++++++++++++++++--
>  1 file changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst
> index e7b46421f247..613385c5ba5b 100644
> --- a/Documentation/dev-tools/kunit/run_manual.rst
> +++ b/Documentation/dev-tools/kunit/run_manual.rst
> @@ -49,9 +49,46 @@ loaded.
>
>  The results will appear in TAP format in ``dmesg``.
>
> +debugfs
> +=======
> +
> +``debugfs`` is a file system that enables user interaction with the files to
> +make kernel information available to user space. A user can interact with
> +the debugfs filesystem using a variety of file operations, such as open,
> +read, and write.
> +
> +By default, only the root user has access to the debugfs directory.

These two paragraphs are probably a bit excessive: we want to focus on
what KUnit can do with debugfs, not describing what debugfs is as a
whole (which is best left to pages like
Documentation/filesystems/debugfs.rst )

> +
> +If ``CONFIG_KUNIT_DEBUGFS`` is enabled, you can use KUnit debugfs
> +filesystem to perform the following actions.
> +
> +Retrieve Test Results
> +=====================
> +
> +You can use debugfs to retrieve KUnit test results. The test results are
> +accessible from the debugfs filesystem in the following read-only file:
> +
> +.. code-block :: bash
> +
> +       /sys/kernel/debug/kunit/<test_suite>/results
> +
> +The test results are available in KTAP format.

We _could_ mention that this is a separate KTAP document (i.e., the
numbering starts at 1), though it may be obvious.

> +
> +Run Tests After Kernel Has Booted
> +=================================
> +
> +You can use the debugfs filesystem to trigger built-in tests to run after
> +boot. To run the test suite, you can use the following command to write to
> +the ``/sys/kernel/debug/kunit/<test_suite>/run`` file:
> +
> +.. code-block :: bash
> +
> +       echo "any string" > /sys/kernel/debugfs/kunit/<test_suite>/run
> +
> +As a result, the test suite runs and the results are printed to the kernel
> +log.
> +
>  .. note ::
>
> -       If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results will
> -       be accessible from the ``debugfs`` filesystem (if mounted).
> -       They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, in
> -       TAP format.
> +       The contents written to the debugfs file
> +       ``/sys/kernel/debug/kunit/<test_suite>/run`` are not saved.

This is possibly a bit obvious. Maybe it'd be more useful with a bit
more context, e.g., "The contents written to the file ... are
discarded; it is the act of writing which triggers the test, not the
specific contents written."?

It might be worth having a note that tests cannot run concurrently, so
this may block or fail.

Equally, it may be worth having a note for test authors, that their
tests will need to correctly initialise and/or clean up any data, so
the test runs correctly a second time.

> --
> 2.42.0.283.g2d96d420d3-goog
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]