From: Tamir Duberstein <tamird@gmail.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: David Gow <davidgow@google.com>,
John Hubbard <jhubbard@nvidia.com>,
Andrew Morton <akpm@linux-foundation.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Madhavan Srinivasan <maddy@linux.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Nicholas Piggin <npiggin@gmail.com>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
Naveen N Rao <naveen@kernel.org>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Shuah Khan <shuah@kernel.org>, Kees Cook <kees@kernel.org>,
Muhammad Usama Anjum <usama.anjum@collabora.com>,
linux-kernel@vger.kernel.org, linux-m68k@lists.linux-m68k.org,
linuxppc-dev@lists.ozlabs.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 0/3] bitmap: convert self-test to KUnit
Date: Sat, 8 Feb 2025 13:42:02 -0500 [thread overview]
Message-ID: <CAJ-ks9=f8d7jh=15bHc28Z37p9rA-Kg4J2mQ++VBcsesVvezUA@mail.gmail.com> (raw)
In-Reply-To: <Z6eaDuXnT_rjVSNS@thinkpad>
On Sat, Feb 8, 2025 at 12:53 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> [...]
>
> Take over means that you'd at least add the Co-developed-by tag.
I didn't use their code - the thing being "taken over" is the work of
having these debates with the maintainers.
> [...]
>
> KUNIT is disabled in defconfig, at least on x86_64. It is also disabled
> on my Ubuntu 24.04 machine. If I take your patches, I'll be unable to
> boot-test bitmaps. Even worse, I'll be unable to build the standalone
> test from sources as a module and load it later.
>
> Or I misunderstand it, and there's a way to build some particular KUNIT
> test without enabling KUNIT in config and/or re-compiling the whole kernel?
> Please teach me, if so
>
> Unless you give me a way to build and run the test in true
> production environment, I'm not going with KUNITs. Sorry.
This is a question for David -- I don't know if this is possible.
> [...]
>
> This is my evidence: sometimes people report performance or whatever
> issues on their systems, suspecting bitmaps guilty. I ask them to run
> the bitmap or find_bit test to narrow the problem. Sometimes I need to
> test a hardware I have no access to, and I have to (kindly!) ask people
> to build a small test and run it. I don't want to ask them to rebuild
> the whole kernel, or even to build something else.
>
> https://lore.kernel.org/all/YuWk3titnOiQACzC@yury-laptop/
This is compelling evidence, and it was not previously raised. Thank you.
I notice that two things are true about the performance test part of
test_bitmap.c:
- It's a minority of the code in the file (48 lines out of 1462).
- There are no assertions in it.
Do you also find value in running the testing portion on other
people's machines, to which you don't have access?
> [...]
>
> Nice summary for the discussion. Unfortunately you missed my concerns.
> Which are:
>
> Pros:
> - Now we switch to KUNITs because KUNITs are so good
>
> Cons:
> - Wipes git history;
I was very careful to minimize churn, and the result is 249 lines on
which I'd now own the blame (228 with `-w`). Still, it's a valid con.
> - Bloats the test's source code;
The test is 74 lines shorter after this series.
> - Adds dependencies;
> - Doesn't run on most popular distros and defconfig;
Yep, I understand your concerns much better now - and I'm grateful for
your having taken the time to explain and show receipts. Still, I
wonder if we can get the best of both worlds - either by finding what
you need in KUnit, or by moving the testing bit to KUnit and keeping
the performance bit where it is.
Thanks.
Tamir
next prev parent reply other threads:[~2025-02-08 18:42 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-07 20:14 [PATCH 0/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-07 20:14 ` [PATCH 1/3] bitmap: remove _check_eq_u32_array Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-08 18:00 ` Yury Norov
2025-02-10 5:22 ` Muhammad Usama Anjum
2025-02-07 20:14 ` [PATCH 2/3] bitmap: convert self-test to KUnit Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-10 5:22 ` Muhammad Usama Anjum
2025-02-10 7:42 ` Geert Uytterhoeven
2025-02-07 20:14 ` [PATCH 3/3] bitmap: break kunit into test cases Tamir Duberstein
2025-02-08 9:07 ` David Gow
2025-02-08 12:33 ` Tamir Duberstein
2025-02-10 5:23 ` Muhammad Usama Anjum
2025-02-08 9:07 ` [PATCH 0/3] bitmap: convert self-test to KUnit David Gow
2025-02-08 17:53 ` Yury Norov
2025-02-08 18:42 ` Tamir Duberstein [this message]
2025-02-10 7:54 ` Geert Uytterhoeven
2025-02-10 19:35 ` distro support for CONFIG_KUNIT: " John Hubbard
2025-02-10 19:46 ` David Hildenbrand
2025-02-10 23:10 ` Nico Pache
2025-02-10 23:41 ` John Hubbard
2025-02-10 20:20 ` Yury Norov
2025-02-10 22:01 ` Tamir Duberstein
2025-02-11 7:51 ` David Gow
2025-03-21 16:53 ` Tamir Duberstein
2025-03-21 18:32 ` Yury Norov
2025-03-21 19:47 ` Tamir Duberstein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJ-ks9=f8d7jh=15bHc28Z37p9rA-Kg4J2mQ++VBcsesVvezUA@mail.gmail.com' \
--to=tamird@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=christophe.leroy@csgroup.eu \
--cc=davidgow@google.com \
--cc=geert@linux-m68k.org \
--cc=jhubbard@nvidia.com \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux@rasmusvillemoes.dk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.ibm.com \
--cc=mpe@ellerman.id.au \
--cc=naveen@kernel.org \
--cc=npiggin@gmail.com \
--cc=shuah@kernel.org \
--cc=usama.anjum@collabora.com \
--cc=yury.norov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).