linux-m68k.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).