Re: [LTP] [PATCH v2 2/2] doc: Add ground rules page

Andrea Cervesato via ltp
2025-12-15T14:52:55Z

Hi!

In general I like the idea, but many sentences can be shorten. I didn't
use AI for this review, but usually they work pretty well on spotting
errors and english mistakes and it might be a good idea to review it
first with that.

Some comments below.

On Mon Dec 15, 2025 at 1:44 PM CET, Cyril Hrubis wrote:
> This is a continued effort to write down the unwritten rules we have in
> the project. Feel free to suggest more topics for the page.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> doc/developers/ground_rules.rst | 91 +++++++++++++++++++++++++++++++++
> doc/index.rst | 1 +
> 2 files changed, 92 insertions(+)
> create mode 100644 doc/developers/ground_rules.rst
>
> Changes in v2:
>
> - added two more rules
> - fixes and typos as pointed out by Peter
>
> diff --git a/doc/developers/ground_rules.rst b/doc/developers/ground_rules.rst
> new file mode 100644
> index 000000000..2bef426aa
> --- /dev/null
> +++ b/doc/developers/ground_rules.rst
> @@ -0,0 +1,91 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Ground Rules
> +============
> +
> +Do not work around kernel bugs
> +------------------------------
> +
> +We have decided what we will not work around bugs in upstream LTP sources. If a
> +test fails on your system for a good reason, e.g. patch wasn't backported and
> +the bug is present, work around for this will not be accepted upstream. The
> +main reason for this decision is that this masks the failure for everyone else.
> +
> +
> +Do not synchronize by sleep
> +---------------------------
> +
> +Why is sleep in tests bad then?
> +```````````````````````````````
> +
> +The first problem is that it will likely introduce very rare test failures,
> +that means somebody has to spend time looking into these, which is a wasted
> +effort. Nobody likes tests that will fail rarely for no good reason. Even more
> +so you cannot run such tests with a background load to ensure that everything
> +works correctly on a busy system, because that will increase the likehood of a
> +failure.
> +
> +The second problem is that this wastes resources and slows down a test run. If
> +you think that adding a sleep to a test is not a big deal, lets have a look at
> +the bigger perspective. There is about 1600 syscall tests in Linux Test
> +Project, if 7.5% of them would sleep just for one second, we would end up with
> +two minutes of wasted time per testrun. In practice most of the test, that
> +historically misused sleep for synchronization, waited for much longer just to
> +be sure that things will works even on slower hardware. With sleeps between 2
> +and 5 seconds that puts us somewhere between 4 and 10 minutes which is between
> +13% and 33% of the syscall runtime on an outdated thinkpad, where the run
> +finishes in a bit less than half an hour. It's even worse on newer hardware,
> +because this slowdown will not change no matter how fast your machine is, which
> +is maybe the reason why this was acceptable twenty years ago but it's not now.
> +
> +
> +What to do instead?
> +```````````````````
> +
> +Use proper synchronization.
> +
> +There are different problems and different solutions. Most often test needs to
> +synchronize between child and parent proces.
> +
> +The easiest case is that parent needs to wait for a child to finish, that can
> +be fixed just be adding a :man2:`waitpid` in the parent which ensures that child is
^ by ^ . This ensures
> +finished before parent runs.
> +
> +Commonly child has to execute certain piece of code before parent can continue.
^ ,
> +For that LTP library implements checkpoints with simple

In these cases, LTP library implements..

> +:c:func:`TST_CHECKPOINT_WAIT()` and :c:func:`TST_CHECKPOINT_WAKE()` functions based
> +on futexes on a piece of shared memory set up by the test library.
> +
> +Another common case is where child must sleep in a syscall before parent can
> +continue, for which we have a :c:func:`TST_PROCESS_STATE_WAIT()` helper that
^ . In this case we have..

> +polls `/proc/$PID/stat`.
> +
> +Less often test needs to wait for an action that is done asynchronously, or a
> +kernel resource deallocation is deferred to a later time. In such cases the
> +best we can do is to poll. In LTP we ended up with a macro that polls by
> +calling a piece of code in a loop with exponentially increasing sleeps between
> +retries until it succeeds. Which means that instead of sleeping for a maximal
> +time event can possibly take the sleep is capped by twice of the optimal
> +sleeping time while we avoid polling too aggressively.

Less often, tests need to wait for an action that is done asynchronously
or they need to wait for a kernel resource deallocation. In such cases,
the best we can do is using polling mechanism. In LTP we have a macro
that loops over a piece of code, exponentially increasing sleeps between
retries until succeed.

-> it would be nice to add the link to it.

----
For the entire section I would put a bullet points list.

> +
> +
> +Use runtime checks for kernel features
> +======================================
> +
> +What is and what isn't supported by kernel is determined by the version
> +and configuration of the kernel the systems is currently running on.

Supported kernel features are determined by version and configuration
of the kernel running on the system.

> +That especially means that any checks done during the compilation cannot
> +be used to assume features supported by the kernel the tests end up
> +running on. The compile time checks, done by configure script, are only
> +useful for enabling fallback kernel API definitions when missing, as we
> +do in lapi/ directory.

This means that kernel features can't be always checked at compile-time.
Please keep compile-time checks only for fallback kernel API definitions,
when needed. -> (reference to lapi?)

> +
> +
> +Kernel features and RCs
> +=======================
> +
> +LTP tests or fixes for kernel changes that were not released yet can be posted
> +to the LTP list for a review but will not be be accepted until respective
> +kernel changes are released. Review of such changes is also considered to be
> +lower priority than rest of the changes. This is because kernel changes
> +especially in the early RC phase are volatile and could be changed or reverted.

LTP tests for new kernel features inside the mainline can be sent to the
mailing list, but they will get a low priority in the review process due
to the highly volatile nature of Linux kernel before release.

> diff --git a/doc/index.rst b/doc/index.rst
> index 06b75616f..659549cc3 100644
> --- a/doc/index.rst
> +++ b/doc/index.rst
> @@ -19,6 +19,7 @@
> :hidden:
> :caption: For developers
>
> + developers/ground_rules
> developers/setup_mailinglist
> developers/writing_tests
> developers/test_case_tutorial

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com