linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] xarray: port tests to kunit
       [not found] ` <07cf896e-adf8-414f-a629-a808fc26014a@oracle.com>
@ 2025-01-29 21:26   ` Liam R. Howlett
  2025-01-29 21:28     ` Tamir Duberstein
  0 siblings, 1 reply; 20+ messages in thread
From: Liam R. Howlett @ 2025-01-29 21:26 UTC (permalink / raw)
  To: Sidhartha Kumar
  Cc: tamird, akpm, christophe.leroy, geert, justinstitt, linux-kernel,
	linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe, nathan,
	naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

* Sidhartha Kumar <sidhartha.kumar@oracle.com> [250129 16:02]:
> + Liam, Matthew

+ linux-mm

Thank you Sid.

> 
> Hello,
> 
> I believe this patch needs to be reverted for now as it breaks the
> user-space build of /tools/testing/radix-tree with:
> 
> In file included from xarray.c:11:
> ../../../lib/test_xarray.c:9:10: fatal error: kunit/test.h: No such file
> or directory
>      9 | #include <kunit/test.h>
>        |          ^~~~~~~~~~~~~~
> compilation terminated.
> make: *** [<builtin>: xarray.o] Error 1
> make: *** Waiting for unfinished jobs....
> 
> this then prevents the maple tree test suite from building.

How are grammar corrections going to the right person (but not the
mailing list) while an entire conversion to kunit is not [1]?

Does the patch really need to drop the module testing too?

What exactly is the point of converting one testing system to another
besides disruption of actual work?  Who asked for this?  What is the
point?

Is anyone doing work on the xarray running the kunit tests?

This should be reverted as it should never have been merged.

Regards,
Liam

[1]. https://lore.kernel.org/all/20241009193602.41797-2-tamird@gmail.com/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 21:26   ` [PATCH] xarray: port tests to kunit Liam R. Howlett
@ 2025-01-29 21:28     ` Tamir Duberstein
  2025-01-29 22:26       ` Liam R. Howlett
  2025-01-31  0:22       ` Andrew Morton
  0 siblings, 2 replies; 20+ messages in thread
From: Tamir Duberstein @ 2025-01-29 21:28 UTC (permalink / raw)
  To: Liam R. Howlett, Sidhartha Kumar, tamird, akpm, christophe.leroy,
	geert, justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm,
	maddy, morbo, mpe, nathan, naveen, ndesaulniers, npiggin,
	Matthew Wilcox, linux-mm

On Wed, Jan 29, 2025 at 4:26 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Sidhartha Kumar <sidhartha.kumar@oracle.com> [250129 16:02]:
> > + Liam, Matthew
>
> + linux-mm
>
> Thank you Sid.
>
> >
> > Hello,
> >
> > I believe this patch needs to be reverted for now as it breaks the
> > user-space build of /tools/testing/radix-tree with:
> >
> > In file included from xarray.c:11:
> > ../../../lib/test_xarray.c:9:10: fatal error: kunit/test.h: No such file
> > or directory
> >      9 | #include <kunit/test.h>
> >        |          ^~~~~~~~~~~~~~
> > compilation terminated.
> > make: *** [<builtin>: xarray.o] Error 1
> > make: *** Waiting for unfinished jobs....
> >
> > this then prevents the maple tree test suite from building.
>
> How are grammar corrections going to the right person (but not the
> mailing list) while an entire conversion to kunit is not [1]?

Very simple: the tests are not properly included in MAINTAINERS. I
sent https://lore.kernel.org/all/20250129-xarray-test-maintainer-v1-1-482e31f30f47@gmail.com/
a few minutes ago for this reason.

> Does the patch really need to drop the module testing too?
>
> What exactly is the point of converting one testing system to another
> besides disruption of actual work?  Who asked for this?  What is the
> point?

All this is described in the commit message.

> Is anyone doing work on the xarray running the kunit tests?

I was doing work on xarray and I was running the kunit tests.

> This should be reverted as it should never have been merged.
>
> Regards,
> Liam
>
> [1]. https://lore.kernel.org/all/20241009193602.41797-2-tamird@gmail.com/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 21:28     ` Tamir Duberstein
@ 2025-01-29 22:26       ` Liam R. Howlett
  2025-01-29 22:33         ` Tamir Duberstein
  2025-01-30  8:21         ` Geert Uytterhoeven
  2025-01-31  0:22       ` Andrew Morton
  1 sibling, 2 replies; 20+ messages in thread
From: Liam R. Howlett @ 2025-01-29 22:26 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Sidhartha Kumar, akpm, christophe.leroy, geert, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

* Tamir Duberstein <tamird@gmail.com> [250129 16:29]:
> On Wed, Jan 29, 2025 at 4:26 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > * Sidhartha Kumar <sidhartha.kumar@oracle.com> [250129 16:02]:
> > > + Liam, Matthew
> >
> > + linux-mm
> >
> > Thank you Sid.
> >
> > >
> > > Hello,
> > >
> > > I believe this patch needs to be reverted for now as it breaks the
> > > user-space build of /tools/testing/radix-tree with:
> > >
> > > In file included from xarray.c:11:
> > > ../../../lib/test_xarray.c:9:10: fatal error: kunit/test.h: No such file
> > > or directory
> > >      9 | #include <kunit/test.h>
> > >        |          ^~~~~~~~~~~~~~
> > > compilation terminated.
> > > make: *** [<builtin>: xarray.o] Error 1
> > > make: *** Waiting for unfinished jobs....
> > >
> > > this then prevents the maple tree test suite from building.
> >
> > How are grammar corrections going to the right person (but not the
> > mailing list) while an entire conversion to kunit is not [1]?
> 
> Very simple: the tests are not properly included in MAINTAINERS. I
> sent https://lore.kernel.org/all/20250129-xarray-test-maintainer-v1-1-482e31f30f47@gmail.com/
> a few minutes ago for this reason.

Fair enough, but from the patch:

@@ -6,11 +6,10 @@
  * Author: Matthew Wilcox <willy@infradead.org>
  */
 
-#include <linux/xarray.h>
-#include <linux/module.h>
+#include <kunit/test.h>

...


-module_init(xarray_checks);
-module_exit(xarray_exit);
 MODULE_AUTHOR("Matthew Wilcox <willy@infradead.org>");
 MODULE_DESCRIPTION("XArray API test module");
 MODULE_LICENSE("GPL");

I don't get why the huge list of Cc's didn't include the author who is
in the git commit signers:
 $ ./scripts/get_maintainer.pl --git lib/xarray.c
Matthew Wilcox <willy@infradead.org> (supporter:XARRAY,commit_signer:1/3=33%,authored:1/3=33%,added_lines:19/52=37%,removed_lines:4/23=17%)
Andrew Morton <akpm@linux-foundation.org> (supporter:LIBRARY CODE,commit_signer:3/3=100%)
...

> 
> > Does the patch really need to drop the module testing too?
> >
> > What exactly is the point of converting one testing system to another
> > besides disruption of actual work?  Who asked for this?  What is the
> > point?
> 
> All this is described in the commit message.

The commit message says you like the output more and implies you like
the command better.

I've never used the kunit testing of xarray and have used the userspace
testing instead, so I can't speak to the obscure invocation as both
commands seem insanely long and obscure to me.

> 
> > Is anyone doing work on the xarray running the kunit tests?
> 
> I was doing work on xarray and I was running the kunit tests.

...

You should look at the userspace testing (that this broke) as it has
been really useful in certain scenarios.

Thanks,
Liam



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 22:26       ` Liam R. Howlett
@ 2025-01-29 22:33         ` Tamir Duberstein
  2025-01-29 23:02           ` Matthew Wilcox
  2025-01-30  8:21         ` Geert Uytterhoeven
  1 sibling, 1 reply; 20+ messages in thread
From: Tamir Duberstein @ 2025-01-29 22:33 UTC (permalink / raw)
  To: Liam R. Howlett, Tamir Duberstein, Sidhartha Kumar, akpm,
	christophe.leroy, geert, justinstitt, linux-kernel, linux-m68k,
	linuxppc-dev, llvm, maddy, morbo, mpe, nathan, naveen,
	ndesaulniers, npiggin, Matthew Wilcox, linux-mm

On Wed, Jan 29, 2025 at 5:26 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Tamir Duberstein <tamird@gmail.com> [250129 16:29]:
> > On Wed, Jan 29, 2025 at 4:26 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > >
> > > * Sidhartha Kumar <sidhartha.kumar@oracle.com> [250129 16:02]:
> > > > + Liam, Matthew
> > >
> > > + linux-mm
> > >
> > > Thank you Sid.
> > >
> > > >
> > > > Hello,
> > > >
> > > > I believe this patch needs to be reverted for now as it breaks the
> > > > user-space build of /tools/testing/radix-tree with:
> > > >
> > > > In file included from xarray.c:11:
> > > > ../../../lib/test_xarray.c:9:10: fatal error: kunit/test.h: No such file
> > > > or directory
> > > >      9 | #include <kunit/test.h>
> > > >        |          ^~~~~~~~~~~~~~
> > > > compilation terminated.
> > > > make: *** [<builtin>: xarray.o] Error 1
> > > > make: *** Waiting for unfinished jobs....
> > > >
> > > > this then prevents the maple tree test suite from building.
> > >
> > > How are grammar corrections going to the right person (but not the
> > > mailing list) while an entire conversion to kunit is not [1]?
> >
> > Very simple: the tests are not properly included in MAINTAINERS. I
> > sent https://lore.kernel.org/all/20250129-xarray-test-maintainer-v1-1-482e31f30f47@gmail.com/
> > a few minutes ago for this reason.
>
> Fair enough, but from the patch:
>
> @@ -6,11 +6,10 @@
>   * Author: Matthew Wilcox <willy@infradead.org>
>   */
>
> -#include <linux/xarray.h>
> -#include <linux/module.h>
> +#include <kunit/test.h>
>
> ...
>
>
> -module_init(xarray_checks);
> -module_exit(xarray_exit);
>  MODULE_AUTHOR("Matthew Wilcox <willy@infradead.org>");
>  MODULE_DESCRIPTION("XArray API test module");
>  MODULE_LICENSE("GPL");
>
> I don't get why the huge list of Cc's didn't include the author who is
> in the git commit signers:
>  $ ./scripts/get_maintainer.pl --git lib/xarray.c
> Matthew Wilcox <willy@infradead.org> (supporter:XARRAY,commit_signer:1/3=33%,authored:1/3=33%,added_lines:19/52=37%,removed_lines:4/23=17%)
> Andrew Morton <akpm@linux-foundation.org> (supporter:LIBRARY CODE,commit_signer:3/3=100%)

I'm not sure what you're asking. I used `b4 prep --auto-to-cc`. It
doesn't know that test_xarray.c and xarray.c have the same maintainer.

> ...
>
> >
> > > Does the patch really need to drop the module testing too?
> > >
> > > What exactly is the point of converting one testing system to another
> > > besides disruption of actual work?  Who asked for this?  What is the
> > > point?
> >
> > All this is described in the commit message.
>
> The commit message says you like the output more and implies you like
> the command better.
>
> I've never used the kunit testing of xarray and have used the userspace
> testing instead, so I can't speak to the obscure invocation as both
> commands seem insanely long and obscure to me.
>
> >
> > > Is anyone doing work on the xarray running the kunit tests?
> >
> > I was doing work on xarray and I was running the kunit tests.
>
> ...
>
> You should look at the userspace testing (that this broke) as it has
> been really useful in certain scenarios.
>
> Thanks,
> Liam

For what it's worth the kunit invocation, while obscure, is
self-documenting. There's usage information that's reasonably
understandable embedded in the tool itself. I looked for the userspace
testing initially but failed to find
tools/testing/radix-tree/xarray.c. Even now, I'm not sure how I'm
meant to compile this.

Tamir


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 22:33         ` Tamir Duberstein
@ 2025-01-29 23:02           ` Matthew Wilcox
  2025-01-29 23:08             ` Tamir Duberstein
  0 siblings, 1 reply; 20+ messages in thread
From: Matthew Wilcox @ 2025-01-29 23:02 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy, geert,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin, linux-mm

On Wed, Jan 29, 2025 at 05:33:06PM -0500, Tamir Duberstein wrote:
> >  $ ./scripts/get_maintainer.pl --git lib/xarray.c
> > Matthew Wilcox <willy@infradead.org> (supporter:XARRAY,commit_signer:1/3=33%,authored:1/3=33%,added_lines:19/52=37%,removed_lines:4/23=17%)
> > Andrew Morton <akpm@linux-foundation.org> (supporter:LIBRARY CODE,commit_signer:3/3=100%)
> 
> I'm not sure what you're asking. I used `b4 prep --auto-to-cc`. It
> doesn't know that test_xarray.c and xarray.c have the same maintainer.

You need to use your brain.  You can't just say "I used the tool".
Tools are just tools.  Sometimes they're wrong.  My email address is
listed as the Author: of test_xarray.c.  You should have noticed that.

> For what it's worth the kunit invocation, while obscure, is
> self-documenting. There's usage information that's reasonably
> understandable embedded in the tool itself. I looked for the userspace
> testing initially but failed to find
> tools/testing/radix-tree/xarray.c. Even now, I'm not sure how I'm
> meant to compile this.

kunit is useless.  The test_xarray.c module is useless.  If you break
xarray, the kernel won't boot far enough to load any modules.  You
haven't thought about this AT ALL.

cd tools/testing/radix-tree
make

Then you can run the tests, whichever ones make sense for you to run.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 23:02           ` Matthew Wilcox
@ 2025-01-29 23:08             ` Tamir Duberstein
  2025-01-29 23:11               ` Matthew Wilcox
  0 siblings, 1 reply; 20+ messages in thread
From: Tamir Duberstein @ 2025-01-29 23:08 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy, geert,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin, linux-mm

On Wed, Jan 29, 2025 at 6:02 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jan 29, 2025 at 05:33:06PM -0500, Tamir Duberstein wrote:
> > >  $ ./scripts/get_maintainer.pl --git lib/xarray.c
> > > Matthew Wilcox <willy@infradead.org> (supporter:XARRAY,commit_signer:1/3=33%,authored:1/3=33%,added_lines:19/52=37%,removed_lines:4/23=17%)
> > > Andrew Morton <akpm@linux-foundation.org> (supporter:LIBRARY CODE,commit_signer:3/3=100%)
> >
> > I'm not sure what you're asking. I used `b4 prep --auto-to-cc`. It
> > doesn't know that test_xarray.c and xarray.c have the same maintainer.
>
> You need to use your brain.  You can't just say "I used the tool".
> Tools are just tools.  Sometimes they're wrong.  My email address is
> listed as the Author: of test_xarray.c.  You should have noticed that.

The whole point of tools is to liberate stupid humans' brains like
mine from mundane tasks like working out who to email. The tool wasn't
wrong; it did exactly what you told it to do in your MAINTAINERS
entry.

> > For what it's worth the kunit invocation, while obscure, is
> > self-documenting. There's usage information that's reasonably
> > understandable embedded in the tool itself. I looked for the userspace
> > testing initially but failed to find
> > tools/testing/radix-tree/xarray.c. Even now, I'm not sure how I'm
> > meant to compile this.
>
> kunit is useless.  The test_xarray.c module is useless.  If you break
> xarray, the kernel won't boot far enough to load any modules.  You
> haven't thought about this AT ALL.

I don't understand what you're saying here.

> cd tools/testing/radix-tree
> make
>
> Then you can run the tests, whichever ones make sense for you to run.

Thanks.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 23:08             ` Tamir Duberstein
@ 2025-01-29 23:11               ` Matthew Wilcox
  2025-01-29 23:17                 ` Tamir Duberstein
  0 siblings, 1 reply; 20+ messages in thread
From: Matthew Wilcox @ 2025-01-29 23:11 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy, geert,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin, linux-mm

On Wed, Jan 29, 2025 at 06:08:22PM -0500, Tamir Duberstein wrote:
> The whole point of tools is to liberate stupid humans' brains like
> mine from mundane tasks like working out who to email. The tool wasn't
> wrong; it did exactly what you told it to do in your MAINTAINERS
> entry.

Tools do get things wrong.  So do humans.  When you take your hands off
the steering wheel and the car crashes, it's still your fault.

> > > For what it's worth the kunit invocation, while obscure, is
> > > self-documenting. There's usage information that's reasonably
> > > understandable embedded in the tool itself. I looked for the userspace
> > > testing initially but failed to find
> > > tools/testing/radix-tree/xarray.c. Even now, I'm not sure how I'm
> > > meant to compile this.
> >
> > kunit is useless.  The test_xarray.c module is useless.  If you break
> > xarray, the kernel won't boot far enough to load any modules.  You
> > haven't thought about this AT ALL.
> 
> I don't understand what you're saying here.

Then I don't want to see any more patches from you until you do.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 23:11               ` Matthew Wilcox
@ 2025-01-29 23:17                 ` Tamir Duberstein
  0 siblings, 0 replies; 20+ messages in thread
From: Tamir Duberstein @ 2025-01-29 23:17 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy, geert,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin, linux-mm

On Wed, Jan 29, 2025 at 6:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jan 29, 2025 at 06:08:22PM -0500, Tamir Duberstein wrote:
> > The whole point of tools is to liberate stupid humans' brains like
> > mine from mundane tasks like working out who to email. The tool wasn't
> > wrong; it did exactly what you told it to do in your MAINTAINERS
> > entry.
>
> Tools do get things wrong.  So do humans.  When you take your hands off
> the steering wheel and the car crashes, it's still your fault.

Which one of us is driving in this analogy?

> > > > For what it's worth the kunit invocation, while obscure, is
> > > > self-documenting. There's usage information that's reasonably
> > > > understandable embedded in the tool itself. I looked for the userspace
> > > > testing initially but failed to find
> > > > tools/testing/radix-tree/xarray.c. Even now, I'm not sure how I'm
> > > > meant to compile this.
> > >
> > > kunit is useless.  The test_xarray.c module is useless.  If you break
> > > xarray, the kernel won't boot far enough to load any modules.  You
> > > haven't thought about this AT ALL.
> >
> > I don't understand what you're saying here.
>
> Then I don't want to see any more patches from you until you do.

Yes, I see now. But it's not quite true; the patch was itself
motivated by the difficult output of the tests, so I did run this with
kunit in a state that produced a failure. Your point is taken.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 22:26       ` Liam R. Howlett
  2025-01-29 22:33         ` Tamir Duberstein
@ 2025-01-30  8:21         ` Geert Uytterhoeven
  2025-01-30 12:51           ` Liam R. Howlett
  2025-01-30 14:22           ` Matthew Wilcox
  1 sibling, 2 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2025-01-30  8:21 UTC (permalink / raw)
  To: Liam R. Howlett
  Cc: Sidhartha Kumar, akpm, christophe.leroy, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

Hi Liam,

On Wed, 29 Jan 2025 at 23:26, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> I've never used the kunit testing of xarray and have used the userspace
> testing instead, so I can't speak to the obscure invocation as both
> commands seem insanely long and obscure to me.

The long and obscure command line is a red herring: a simple
"modprobe test_xarray" is all it takes...

> You should look at the userspace testing (that this broke) as it has
> been really useful in certain scenarios.

BTW, how do I even build tools/testing/radix-tree?
"make tools/help" doesn't show the radix-tree test.
"make tools/all" doesn't seem to try to build it.
Same for "make kselftest-all".
When trying the above, and ignoring failures due to missing packages
on my host:
  - there are several weird build errors,
  - this doesn't play well with O=,
  - lots of scary warnings when building for 32-bit,
  - ...

At least the kunit tests build (and run[1] ;-) most of the time...

[1] test_xarray started failing on m68k recently
    https://lore.kernel.org/all/CAMuHMdU_bfadUO=0OZ=AoQ9EAmQPA4wsLCBqohXR+QCeCKRn4A@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30  8:21         ` Geert Uytterhoeven
@ 2025-01-30 12:51           ` Liam R. Howlett
  2025-01-30 13:25             ` Geert Uytterhoeven
  2025-01-30 14:22           ` Matthew Wilcox
  1 sibling, 1 reply; 20+ messages in thread
From: Liam R. Howlett @ 2025-01-30 12:51 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sidhartha Kumar, akpm, christophe.leroy, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

* Geert Uytterhoeven <geert@linux-m68k.org> [250130 03:21]:
> Hi Liam,
> 
> On Wed, 29 Jan 2025 at 23:26, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > I've never used the kunit testing of xarray and have used the userspace
> > testing instead, so I can't speak to the obscure invocation as both
> > commands seem insanely long and obscure to me.
> 
> The long and obscure command line is a red herring: a simple
> "modprobe test_xarray" is all it takes...

That command worked before too...

> 
> > You should look at the userspace testing (that this broke) as it has
> > been really useful in certain scenarios.
> 
> BTW, how do I even build tools/testing/radix-tree?
> "make tools/help" doesn't show the radix-tree test.
> "make tools/all" doesn't seem to try to build it.
> Same for "make kselftest-all".

make

Or look at the make file and stop guessing.  Considering how difficult
it is to get m68k to build, you should probably know how to read a
makefile.

> When trying the above, and ignoring failures due to missing packages
> on my host:
>   - there are several weird build errors,
>   - this doesn't play well with O=,
>   - lots of scary warnings when building for 32-bit,
>   - ...
> 
> At least the kunit tests build (and run[1] ;-) most of the time...

Do they?  How about you break something in xarray and then try to boot
the kunit, or try to boot to load that module.


This exchange is a good argument against doing any kunit work on my
tests.

> 
> [1] test_xarray started failing on m68k recently
>     https://lore.kernel.org/all/CAMuHMdU_bfadUO=0OZ=AoQ9EAmQPA4wsLCBqohXR+QCeCKRn4A@mail.gmail.com/
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 12:51           ` Liam R. Howlett
@ 2025-01-30 13:25             ` Geert Uytterhoeven
  2025-01-30 14:05               ` Liam R. Howlett
  2025-01-30 14:09               ` Lorenzo Stoakes
  0 siblings, 2 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2025-01-30 13:25 UTC (permalink / raw)
  To: Liam R. Howlett
  Cc: Sidhartha Kumar, akpm, christophe.leroy, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

Hi Liam,

On Thu, 30 Jan 2025 at 13:52, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> * Geert Uytterhoeven <geert@linux-m68k.org> [250130 03:21]:
> > On Wed, 29 Jan 2025 at 23:26, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > I've never used the kunit testing of xarray and have used the userspace
> > > testing instead, so I can't speak to the obscure invocation as both
> > > commands seem insanely long and obscure to me.
> >
> > The long and obscure command line is a red herring: a simple
> > "modprobe test_xarray" is all it takes...
>
> That command worked before too...

Exactly, great!

> > > You should look at the userspace testing (that this broke) as it has
> > > been really useful in certain scenarios.
> >
> > BTW, how do I even build tools/testing/radix-tree?
> > "make tools/help" doesn't show the radix-tree test.
> > "make tools/all" doesn't seem to try to build it.
> > Same for "make kselftest-all".
>
> make

Where?

> Or look at the make file and stop guessing.  Considering how difficult

There is no Makefile referencing tools/testing/radix-tree or the
radix-tree subdir. That's why I asked...

Oh, I am supposed to run make in tools/testing/radix-tree/?
What a surprise!

Which is a pain when building in a separate output directory, as you
cannot just do "make -C tools/testing/radix-tree" there, but have to
type the full "make -C tools/testing/radix-tree O=..." (and optionally
ARCH=... and CROSS_COMPILE=...; oh wait, these are ignored :-( in the
source directory instead...

If these tests are not integrated into the normal build system (see
also [1]), I am not so surprised the auto-builders don't build them,
and breakages are introduced...

> it is to get m68k to build, you should probably know how to read a
> makefile.

Like all other kernel cross-compilation? Usually you don't even have
to know where your cross-compiler is living:

    make ARCH=m68k

> > When trying the above, and ignoring failures due to missing packages
> > on my host:
> >   - there are several weird build errors,
> >   - this doesn't play well with O=,
> >   - lots of scary warnings when building for 32-bit,
> >   - ...
> >
> > At least the kunit tests build (and run[1] ;-) most of the time...
>
> Do they?  How about you break something in xarray and then try to boot
> the kunit, or try to boot to load that module.

If you break the kernel beyond the point of booting, you can indeed
not run any test modules...

Which does _not_ mean the userspace tests are not useful, and that I
approve breaking the userspace tests...

[1] https://lore.kernel.org/all/CAK7LNASdA+5_pdTjr1dY-cKGSDq804Huc_CX_8-Gg+ypFCmajQ@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 13:25             ` Geert Uytterhoeven
@ 2025-01-30 14:05               ` Liam R. Howlett
  2025-01-30 14:24                 ` Geert Uytterhoeven
  2025-01-30 14:09               ` Lorenzo Stoakes
  1 sibling, 1 reply; 20+ messages in thread
From: Liam R. Howlett @ 2025-01-30 14:05 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sidhartha Kumar, akpm, christophe.leroy, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

* Geert Uytterhoeven <geert@linux-m68k.org> [250130 08:26]:
> Hi Liam,
> 
> On Thu, 30 Jan 2025 at 13:52, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > * Geert Uytterhoeven <geert@linux-m68k.org> [250130 03:21]:
> > > On Wed, 29 Jan 2025 at 23:26, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > > I've never used the kunit testing of xarray and have used the userspace
> > > > testing instead, so I can't speak to the obscure invocation as both
> > > > commands seem insanely long and obscure to me.
> > >
> > > The long and obscure command line is a red herring: a simple
> > > "modprobe test_xarray" is all it takes...
> >
> > That command worked before too...
> 
> Exactly, great!
> 
> > > > You should look at the userspace testing (that this broke) as it has
> > > > been really useful in certain scenarios.
> > >
> > > BTW, how do I even build tools/testing/radix-tree?
> > > "make tools/help" doesn't show the radix-tree test.
> > > "make tools/all" doesn't seem to try to build it.
> > > Same for "make kselftest-all".
> >
> > make
> 
> Where?
> > > BTW, how do I even build tools/testing/radix-tree?
                                ^^^^^^^^^^^^^^^^^^^^^^^
> 
> > Or look at the make file and stop guessing.  Considering how difficult
> 
> There is no Makefile referencing tools/testing/radix-tree or the
> radix-tree subdir. That's why I asked...
> 
> Oh, I am supposed to run make in tools/testing/radix-tree/?
> What a surprise!
> 
> Which is a pain when building in a separate output directory, as you
> cannot just do "make -C tools/testing/radix-tree" there, but have to
> type the full "make -C tools/testing/radix-tree O=..." (and optionally
> ARCH=... and CROSS_COMPILE=...; oh wait, these are ignored :-( in the
> source directory instead...

I'll await your patch to link all this together.  Please Cc the authors.


> 
> If these tests are not integrated into the normal build system (see
> also [1]), I am not so surprised the auto-builders don't build them,
> and breakages are introduced...
> 
> > it is to get m68k to build, you should probably know how to read a
> > makefile.
> 
> Like all other kernel cross-compilation? Usually you don't even have
> to know where your cross-compiler is living:
> 
>     make ARCH=m68k

Ignoring that I had to make a config - which asked challenging
questions...

And ignoring the steps to get m68k compiler...

> > > When trying the above, and ignoring failures due to missing packages
> > > on my host:
> > >   - there are several weird build errors,
> > >   - this doesn't play well with O=,
> > >   - lots of scary warnings when building for 32-bit,
> > >   - ...
> > >

In file included from ./include/linux/sched.h:12,
                 from arch/m68k/kernel/asm-offsets.c:15:
./arch/m68k/include/asm/current.h:7:30: error: invalid register name for ‘current’
    7 | register struct task_struct *current __asm__("%a2");


> 
> > > At least the kunit tests build (and run[1] ;-) most of the time...
> >
> > Do they?  How about you break something in xarray and then try to boot
> > the kunit, or try to boot to load that module.
> 
> If you break the kernel beyond the point of booting, you can indeed
> not run any test modules...

Which is extremely easy when you are changing code that runs so early in
the boot.

My code found a compiler issue because it's the first function that
returns a boolean.  This is stupid.

> 
> Which does _not_ mean the userspace tests are not useful, and that I
> approve breaking the userspace tests...

Perfect, let's revert the patch then.

This is such a waste of time.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 13:25             ` Geert Uytterhoeven
  2025-01-30 14:05               ` Liam R. Howlett
@ 2025-01-30 14:09               ` Lorenzo Stoakes
  2025-01-30 14:38                 ` Geert Uytterhoeven
  1 sibling, 1 reply; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-01-30 14:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin,
	Matthew Wilcox, linux-mm

Geert,

Having written a ton of test code, I've unfortunately encountered a lot of
this sort of push-back and it's HUGELY off-putting. Writing test code
should be ENCOURAGED not litigated against.

The truth is far too little kernel code is tested to any degree, and this
is part of why.

On kunit collaboration, I attended an in-person talk at LPC on kunit
userland testing where it was broadly agreed that at this point in time,
the xarray/radix tree tests weren't really suited to the framework.

Therefore I think the healthy means of pushing forward with integration is
in sensible discussion and if patches, RFC patches in collaboration with
authors.

The unhealthy approach is to needle one of the biggest contributors to core
test code in the kernel on a thread because you don't seem to want to cd to
a directory and run make.

Why is this relevant to me? I am the author of the VMA test suite, on which
I spent countless hours + relied heavily on Liam's work to do so, and
equally there you have to cd to a directory and run make.

But at the same time in both cases, testability of key internal components
is ENORMOUSLY improved and allows for REALLY exciting possibilities in test
coverage, really isolating functions for unit testing, enormously fast
iteration speed, etc. etc.

I ask you to weigh up the desire to enumerate your misgivings about the
testing approach used here vs. all of the above.

I would respectfully suggest that here is neither the time nor the place
for splitting hairs.

Thanks, Lorenzo

On Thu, Jan 30, 2025 at 02:25:57PM +0100, Geert Uytterhoeven wrote:
> Hi Liam,
>
> On Thu, 30 Jan 2025 at 13:52, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > * Geert Uytterhoeven <geert@linux-m68k.org> [250130 03:21]:
> > > On Wed, 29 Jan 2025 at 23:26, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > > I've never used the kunit testing of xarray and have used the userspace
> > > > testing instead, so I can't speak to the obscure invocation as both
> > > > commands seem insanely long and obscure to me.
> > >
> > > The long and obscure command line is a red herring: a simple
> > > "modprobe test_xarray" is all it takes...
> >
> > That command worked before too...
>
> Exactly, great!
>
> > > > You should look at the userspace testing (that this broke) as it has
> > > > been really useful in certain scenarios.
> > >
> > > BTW, how do I even build tools/testing/radix-tree?
> > > "make tools/help" doesn't show the radix-tree test.
> > > "make tools/all" doesn't seem to try to build it.
> > > Same for "make kselftest-all".
> >
> > make
>
> Where?
>
> > Or look at the make file and stop guessing.  Considering how difficult
>
> There is no Makefile referencing tools/testing/radix-tree or the
> radix-tree subdir. That's why I asked...
>
> Oh, I am supposed to run make in tools/testing/radix-tree/?
> What a surprise!
>
> Which is a pain when building in a separate output directory, as you
> cannot just do "make -C tools/testing/radix-tree" there, but have to
> type the full "make -C tools/testing/radix-tree O=..." (and optionally
> ARCH=... and CROSS_COMPILE=...; oh wait, these are ignored :-( in the
> source directory instead...
>
> If these tests are not integrated into the normal build system (see
> also [1]), I am not so surprised the auto-builders don't build them,
> and breakages are introduced...
>
> > it is to get m68k to build, you should probably know how to read a
> > makefile.
>
> Like all other kernel cross-compilation? Usually you don't even have
> to know where your cross-compiler is living:
>
>     make ARCH=m68k
>
> > > When trying the above, and ignoring failures due to missing packages
> > > on my host:
> > >   - there are several weird build errors,
> > >   - this doesn't play well with O=,
> > >   - lots of scary warnings when building for 32-bit,
> > >   - ...
> > >
> > > At least the kunit tests build (and run[1] ;-) most of the time...
> >
> > Do they?  How about you break something in xarray and then try to boot
> > the kunit, or try to boot to load that module.
>
> If you break the kernel beyond the point of booting, you can indeed
> not run any test modules...
>
> Which does _not_ mean the userspace tests are not useful, and that I
> approve breaking the userspace tests...
>
> [1] https://lore.kernel.org/all/CAK7LNASdA+5_pdTjr1dY-cKGSDq804Huc_CX_8-Gg+ypFCmajQ@mail.gmail.com/
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30  8:21         ` Geert Uytterhoeven
  2025-01-30 12:51           ` Liam R. Howlett
@ 2025-01-30 14:22           ` Matthew Wilcox
  1 sibling, 0 replies; 20+ messages in thread
From: Matthew Wilcox @ 2025-01-30 14:22 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin, linux-mm

On Thu, Jan 30, 2025 at 09:21:12AM +0100, Geert Uytterhoeven wrote:
> The long and obscure command line is a red herring: a simple
> "modprobe test_xarray" is all it takes...

That's all I've ever done.  I'm confused/annoyed by all this "let's
wrap everything up in some complex and obscure new thing" practice.

> > You should look at the userspace testing (that this broke) as it has
> > been really useful in certain scenarios.
> 
> BTW, how do I even build tools/testing/radix-tree?
> "make tools/help" doesn't show the radix-tree test.
> "make tools/all" doesn't seem to try to build it.
> Same for "make kselftest-all".

I've never heard of any of those things.  So I didn't try to make any of
them work.

> When trying the above, and ignoring failures due to missing packages
> on my host:
>   - there are several weird build errors,
>   - this doesn't play well with O=,
>   - lots of scary warnings when building for 32-bit,
>   - ...

Really?  Turns out I haven't built for 32-bit on this laptop, so I'm
installing all the packages I need now.  I seem to reemember it being
painful due to liburcu packaging not allowing me to install both a
32-bit and 64-bit package at the same time.

> At least the kunit tests build (and run[1] ;-) most of the time...

That's unworthy.  kunit isn't trying to do anything nearly as hard
as the radix tree tests are.

> [1] test_xarray started failing on m68k recently
>     https://lore.kernel.org/all/CAMuHMdU_bfadUO=0OZ=AoQ9EAmQPA4wsLCBqohXR+QCeCKRn4A@mail.gmail.com/

I really wish Andrew would stop merging xarray patches.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 14:05               ` Liam R. Howlett
@ 2025-01-30 14:24                 ` Geert Uytterhoeven
  2025-01-30 15:16                   ` Liam R. Howlett
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2025-01-30 14:24 UTC (permalink / raw)
  To: Liam R. Howlett
  Cc: Sidhartha Kumar, akpm, christophe.leroy, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

Hi Liam,

On Thu, 30 Jan 2025 at 15:06, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> * Geert Uytterhoeven <geert@linux-m68k.org> [250130 08:26]:
> > On Thu, 30 Jan 2025 at 13:52, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > * Geert Uytterhoeven <geert@linux-m68k.org> [250130 03:21]:
> > > > On Wed, 29 Jan 2025 at 23:26, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > > > I've never used the kunit testing of xarray and have used the userspace
> > > > > testing instead, so I can't speak to the obscure invocation as both
> > > > > commands seem insanely long and obscure to me.
> > > >
> > > > The long and obscure command line is a red herring: a simple
> > > > "modprobe test_xarray" is all it takes...
> > >
> > > That command worked before too...
> >
> > Exactly, great!
> >
> > > > > You should look at the userspace testing (that this broke) as it has
> > > > > been really useful in certain scenarios.
> > > >
> > > > BTW, how do I even build tools/testing/radix-tree?
> > > > "make tools/help" doesn't show the radix-tree test.
> > > > "make tools/all" doesn't seem to try to build it.
> > > > Same for "make kselftest-all".
> > >
> > > make
> >
> > Where?
> > > > BTW, how do I even build tools/testing/radix-tree?
>                                 ^^^^^^^^^^^^^^^^^^^^^^^

Things like "make -C drivers/net/ethernet/" stopped working
ca. 20y ago.

> > > Or look at the make file and stop guessing.  Considering how difficult
> >
> > There is no Makefile referencing tools/testing/radix-tree or the
> > radix-tree subdir. That's why I asked...
> >
> > Oh, I am supposed to run make in tools/testing/radix-tree/?
> > What a surprise!
> >
> > Which is a pain when building in a separate output directory, as you
> > cannot just do "make -C tools/testing/radix-tree" there, but have to
> > type the full "make -C tools/testing/radix-tree O=..." (and optionally
> > ARCH=... and CROSS_COMPILE=...; oh wait, these are ignored :-( in the
> > source directory instead...
>
> I'll await your patch to link all this together.  Please Cc the authors.

I gave it a try for kselftests a few years ago.
https://lore.kernel.org/all/20190114135144.26096-1-geert+renesas@glider.be
Unfortunately only one patch was applied...

> > > it is to get m68k to build, you should probably know how to read a
> > > makefile.
> >
> > Like all other kernel cross-compilation? Usually you don't even have
> > to know where your cross-compiler is living:
> >
> >     make ARCH=m68k
>
> Ignoring that I had to make a config - which asked challenging
> questions...

make ARCH=m68k defconfig

> And ignoring the steps to get m68k compiler...

apt install gcc-m68k-linux-gnu?

> > > > When trying the above, and ignoring failures due to missing packages
> > > > on my host:
> > > >   - there are several weird build errors,
> > > >   - this doesn't play well with O=,
> > > >   - lots of scary warnings when building for 32-bit,
> > > >   - ...
> > > >
>
> In file included from ./include/linux/sched.h:12,
>                  from arch/m68k/kernel/asm-offsets.c:15:
> ./arch/m68k/include/asm/current.h:7:30: error: invalid register name for ‘current’
>     7 | register struct task_struct *current __asm__("%a2");

Which compiler are you using?

> > > > At least the kunit tests build (and run[1] ;-) most of the time...
> > >
> > > Do they?  How about you break something in xarray and then try to boot
> > > the kunit, or try to boot to load that module.
> >
> > If you break the kernel beyond the point of booting, you can indeed
> > not run any test modules...
>
> Which is extremely easy when you are changing code that runs so early in
> the boot.
>
> My code found a compiler issue because it's the first function that
> returns a boolean.  This is stupid.

Sorry. I don't understand this comment.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 14:09               ` Lorenzo Stoakes
@ 2025-01-30 14:38                 ` Geert Uytterhoeven
  2025-01-30 14:49                   ` Lorenzo Stoakes
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2025-01-30 14:38 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin,
	Matthew Wilcox, linux-mm

Hi Lorenzo,

On Thu, 30 Jan 2025 at 15:09, Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
> Having written a ton of test code, I've unfortunately encountered a lot of
> this sort of push-back and it's HUGELY off-putting. Writing test code
> should be ENCOURAGED not litigated against.

I am not discouraging nor pushing back on any testing code (on the
contrary, I test every single new kunit test that appears upstream).
My apologies if I gave the impression.

> The truth is far too little kernel code is tested to any degree, and this
> is part of why.
>
> On kunit collaboration, I attended an in-person talk at LPC on kunit
> userland testing where it was broadly agreed that at this point in time,
> the xarray/radix tree tests weren't really suited to the framework.
>
> Therefore I think the healthy means of pushing forward with integration is
> in sensible discussion and if patches, RFC patches in collaboration with
> authors.

Good.

> The unhealthy approach is to needle one of the biggest contributors to core
> test code in the kernel on a thread because you don't seem to want to cd to
> a directory and run make.

My initial issue was that I could not find out where that is documented.

    $ make help
    ...
    Userspace tools targets:
      use "make tools/help"
      or  "cd tools; make help"

    $ make tools/help
    Possible targets:
    ...
    You can do:
      ...
      $ make tools/all

      builds all tools.

But that command does not build tools/testing/radix-tree, so I was
completely lost.

> Why is this relevant to me? I am the author of the VMA test suite, on which
> I spent countless hours + relied heavily on Liam's work to do so, and
> equally there you have to cd to a directory and run make.

Thanks for your work!  One suggestion for improvement: tools/testing/vma
does not seem to be built by "make tools/all" either.

> But at the same time in both cases, testability of key internal components
> is ENORMOUSLY improved and allows for REALLY exciting possibilities in test
> coverage, really isolating functions for unit testing, enormously fast
> iteration speed, etc. etc.
>
> I ask you to weigh up the desire to enumerate your misgivings about the
> testing approach used here vs. all of the above.

I repeat: I am not against these tests.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 14:38                 ` Geert Uytterhoeven
@ 2025-01-30 14:49                   ` Lorenzo Stoakes
  0 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Stoakes @ 2025-01-30 14:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Liam R. Howlett, Sidhartha Kumar, akpm, christophe.leroy,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin,
	Matthew Wilcox, linux-mm

On Thu, Jan 30, 2025 at 03:38:36PM +0100, Geert Uytterhoeven wrote:
> Hi Lorenzo,
>
> On Thu, 30 Jan 2025 at 15:09, Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> > Having written a ton of test code, I've unfortunately encountered a lot of
> > this sort of push-back and it's HUGELY off-putting. Writing test code
> > should be ENCOURAGED not litigated against.
>
> I am not discouraging nor pushing back on any testing code (on the
> contrary, I test every single new kunit test that appears upstream).
> My apologies if I gave the impression.

My point is what comes across as pedantry often seems to arrive (in my
experience) in response to test code submissions.

A wag would say 'isn't that true of all kernel code?' :) but my experience
has been that there has been poor RoI on the kind of feedback you get
vs. meaningful benefits to the point that it feels like letting yourself in
for yak shaving.

We may be terribly misreading you in this thread, but it's in any case how
it comes across. Obviously this isn't helped by the context of this
discussion!

>
> > The truth is far too little kernel code is tested to any degree, and this
> > is part of why.
> >
> > On kunit collaboration, I attended an in-person talk at LPC on kunit
> > userland testing where it was broadly agreed that at this point in time,
> > the xarray/radix tree tests weren't really suited to the framework.
> >
> > Therefore I think the healthy means of pushing forward with integration is
> > in sensible discussion and if patches, RFC patches in collaboration with
> > authors.
>
> Good.
>
> > The unhealthy approach is to needle one of the biggest contributors to core
> > test code in the kernel on a thread because you don't seem to want to cd to
> > a directory and run make.
>
> My initial issue was that I could not find out where that is documented.
>
>     $ make help
>     ...
>     Userspace tools targets:
>       use "make tools/help"
>       or  "cd tools; make help"
>
>     $ make tools/help
>     Possible targets:
>     ...
>     You can do:
>       ...
>       $ make tools/all
>
>       builds all tools.
>
> But that command does not build tools/testing/radix-tree, so I was
> completely lost.

Right, I mean this should be easy enough to fix.

If what you're saying is - make this more discoverable, make this easily
buildable from the top of the tree - then sure, and I expect this shouldn't
be too challenging.

>
> > Why is this relevant to me? I am the author of the VMA test suite, on which
> > I spent countless hours + relied heavily on Liam's work to do so, and
> > equally there you have to cd to a directory and run make.
>
> Thanks for your work!  One suggestion for improvement: tools/testing/vma
> does not seem to be built by "make tools/all" either.

You're welcome, and right yeah, should be relatively simple to fix.

This kind of straightforward practical thing is fine. 'You must use the
space wombat with extra fireworks' type feedback is not so much...

>
> > But at the same time in both cases, testability of key internal components
> > is ENORMOUSLY improved and allows for REALLY exciting possibilities in test
> > coverage, really isolating functions for unit testing, enormously fast
> > iteration speed, etc. etc.
> >
> > I ask you to weigh up the desire to enumerate your misgivings about the
> > testing approach used here vs. all of the above.
>
> I repeat: I am not against these tests.

Good, but I would also emphasise what I'm trying to get across with the
wall of text here (sorry, that's very me) - let's not get bogged down in
the framework or whether it might be preferable to have it under kunit or
this that or the other thing vs. the enormous benefits this stuff brings.

Sometimes, esp. with new approaches to testing, it has to be at the very
least incremental.

If you're saying 'just get it documented in a make help and make it
buildable from the top of the tree' then fair enough and hopefully not too
difficult.

In other words - we are all after the same thing here, let's work towards
making everything better, and not unnecessarily throw unshaved yaks in the
way :)

>
> Thanks!
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 14:24                 ` Geert Uytterhoeven
@ 2025-01-30 15:16                   ` Liam R. Howlett
  2025-01-31  7:39                     ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: Liam R. Howlett @ 2025-01-30 15:16 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sidhartha Kumar, akpm, christophe.leroy, justinstitt,
	linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy, morbo, mpe,
	nathan, naveen, ndesaulniers, npiggin, Matthew Wilcox, linux-mm

* Geert Uytterhoeven <geert@linux-m68k.org> [250130 09:25]:
> Hi Liam,

Hi Geert,

I'd like to say sorry for getting upset about this.

> 
> On Thu, 30 Jan 2025 at 15:06, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > I'll await your patch to link all this together.  Please Cc the authors.
> 
> I gave it a try for kselftests a few years ago.
> https://lore.kernel.org/all/20190114135144.26096-1-geert+renesas@glider.be
> Unfortunately only one patch was applied...

It is difficult to integrate the test framework due to the nature of it
stubbing out a lot of the kernel code it uses.

This is also a strength as it can be used to test unlikely failure
scenarios that are difficult or impossible to recreate in kunit or a
running kernel - even with injections of failures.

It can also be used to isolate issues for recreating, which is very
valuable in larger, longer running issues.

I also use the userspace testing to test rcu using threads and I think
that would be even more challenging on a booted system.

> 
> > > > it is to get m68k to build, you should probably know how to read a
> > > > makefile.
> > >
> > > Like all other kernel cross-compilation? Usually you don't even have
> > > to know where your cross-compiler is living:
> > >
> > >     make ARCH=m68k
> >
> > Ignoring that I had to make a config - which asked challenging
> > questions...
> 
> make ARCH=m68k defconfig

That also prompts, defoldconfig did not.

> 
> > And ignoring the steps to get m68k compiler...
> 
> apt install gcc-m68k-linux-gnu?

There are a few compilers, multilib or such?  I've had issues with
getting all the archs working for cross compile on the same machine
(arm, arm64, riscv, m68k, ppc, ppc64, parisc).

> 
> > > > > When trying the above, and ignoring failures due to missing packages
> > > > > on my host:
> > > > >   - there are several weird build errors,
> > > > >   - this doesn't play well with O=,
> > > > >   - lots of scary warnings when building for 32-bit,
> > > > >   - ...
> > > > >
> >
> > In file included from ./include/linux/sched.h:12,
> >                  from arch/m68k/kernel/asm-offsets.c:15:
> > ./arch/m68k/include/asm/current.h:7:30: error: invalid register name for ‘current’
> >     7 | register struct task_struct *current __asm__("%a2");
> 
> Which compiler are you using?

I've had a hard time getting m68k to boot in qemu because of the lack of
userspace.  I use m68k for nommu testing, but have a hard time getting
the buildroot to work correctly to build what I need.

This was a grumpy, pre-coffee way of saying that some tasks are not
straight forward and are extremely difficult to make straight forward.

Sorry, I should not have made such rash comments.  I respect you and
your work and appreciate the help you have given me in the past.

I would also like to thank you for your earlier statements.  After
rereading them, I believe I misunderstood what you were saying.  I've
been trying to make these tests a part of automatic testing somehow,
even getting them to run in userspace.

> 
> > > > > At least the kunit tests build (and run[1] ;-) most of the time...
> > > >
> > > > Do they?  How about you break something in xarray and then try to boot
> > > > the kunit, or try to boot to load that module.
> > >
> > > If you break the kernel beyond the point of booting, you can indeed
> > > not run any test modules...
> >
> > Which is extremely easy when you are changing code that runs so early in
> > the boot.
> >
> > My code found a compiler issue because it's the first function that
> > returns a boolean.  This is stupid.
> 
> Sorry. I don't understand this comment.

I had a bug a few years ago that turned out to be a clang compiler issue
with booleans.  It turns out my code was the first function to return
a boolean and that wasn't being properly handled by the compiler (thanks
to mitigation of clearing registers with certain .config/compiler
flags).. it's not important.

More importantly, I think I get your point, you think that the testing
should be integrated and complain if it's broken - at least by bots.  I
don't think this is practical in all cases, unfortunately.

Although I would like to strive for this - and I do by keeping any tests
I can as a kernel module while keeping the harder to recreate issues as
user space.  I think we do a good job keeping testing up to date and
adding testcases as new issues are discovered.

Thanks,
Liam



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-29 21:28     ` Tamir Duberstein
  2025-01-29 22:26       ` Liam R. Howlett
@ 2025-01-31  0:22       ` Andrew Morton
  1 sibling, 0 replies; 20+ messages in thread
From: Andrew Morton @ 2025-01-31  0:22 UTC (permalink / raw)
  To: Tamir Duberstein
  Cc: Liam R. Howlett, Sidhartha Kumar, christophe.leroy, geert,
	justinstitt, linux-kernel, linux-m68k, linuxppc-dev, llvm, maddy,
	morbo, mpe, nathan, naveen, ndesaulniers, npiggin,
	Matthew Wilcox, linux-mm

On Wed, 29 Jan 2025 16:28:32 -0500 Tamir Duberstein <tamird@gmail.com> wrote:

> > How are grammar corrections going to the right person (but not the
> > mailing list) while an entire conversion to kunit is not [1]?
> 
> Very simple: the tests are not properly included in MAINTAINERS. I
> sent https://lore.kernel.org/all/20250129-xarray-test-maintainer-v1-1-482e31f30f47@gmail.com/
> a few minutes ago for this reason.

I failed to notice that this patch didn't cc linux-mm.

MAINTAINERS doesn't ask people to cc linux-mm on xarray changes.

linux-mm averages 130 messages/day - I think it's reasonable to believe
that MM developers spend a few minutes a day scanning the Subject:s.

This:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: MAINTAINERS: include linux-mm for xarray maintenance
Date: Thu Jan 30 04:16:20 PM PST 2025

MM developers have an interest in the xarray code.

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 MAINTAINERS |    1 +
 1 file changed, 1 insertion(+)

--- a/MAINTAINERS~maintainers-include-linux-mm-for-xarray-maintenance
+++ a/MAINTAINERS
@@ -25676,6 +25676,7 @@ F:	arch/x86/entry/vdso/
 XARRAY
 M:	Matthew Wilcox <willy@infradead.org>
 L:	linux-fsdevel@vger.kernel.org
+L:	linux-mm@kvack.org
 S:	Supported
 F:	Documentation/core-api/xarray.rst
 F:	include/linux/idr.h
_



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] xarray: port tests to kunit
  2025-01-30 15:16                   ` Liam R. Howlett
@ 2025-01-31  7:39                     ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2025-01-31  7:39 UTC (permalink / raw)
  To: Liam R. Howlett, Geert Uytterhoeven, Sidhartha Kumar, akpm,
	christophe.leroy, justinstitt, linux-kernel, linux-m68k,
	linuxppc-dev, llvm, maddy, morbo, mpe, nathan, naveen,
	ndesaulniers, npiggin, Matthew Wilcox, linux-mm

Hi Liam,

On Thu, 30 Jan 2025 at 16:17, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> * Geert Uytterhoeven <geert@linux-m68k.org> [250130 09:25]:
> > On Thu, 30 Jan 2025 at 15:06, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > > > it is to get m68k to build, you should probably know how to read a
> > > > > makefile.
> > > >
> > > > Like all other kernel cross-compilation? Usually you don't even have
> > > > to know where your cross-compiler is living:
> > > >
> > > >     make ARCH=m68k
> > >
> > > Ignoring that I had to make a config - which asked challenging
> > > questions...
> >
> > make ARCH=m68k defconfig
>
> That also prompts, defoldconfig did not.

Hmm, using a defconfig should never ask for questions.
Perhaps this might happen if it has an option enabled that depends on
specific compiler support?

> > > And ignoring the steps to get m68k compiler...
> >
> > apt install gcc-m68k-linux-gnu?
>
> There are a few compilers, multilib or such?  I've had issues with
> getting all the archs working for cross compile on the same machine
> (arm, arm64, riscv, m68k, ppc, ppc64, parisc).

I have installed all of the above (and more) from Ubuntu
(except for parisc, as Ubuntu does not have it), and more
from https://www.kernel.org/pub/tools/crosstool/.
All of them should work fine (for building kernels).

> > > > > > When trying the above, and ignoring failures due to missing packages
> > > > > > on my host:
> > > > > >   - there are several weird build errors,
> > > > > >   - this doesn't play well with O=,
> > > > > >   - lots of scary warnings when building for 32-bit,
> > > > > >   - ...
> > > > > >
> > >
> > > In file included from ./include/linux/sched.h:12,
> > >                  from arch/m68k/kernel/asm-offsets.c:15:
> > > ./arch/m68k/include/asm/current.h:7:30: error: invalid register name for ‘current’
> > >     7 | register struct task_struct *current __asm__("%a2");
> >
> > Which compiler are you using?
>
> I've had a hard time getting m68k to boot in qemu because of the lack of
> userspace.  I use m68k for nommu testing, but have a hard time getting
> the buildroot to work correctly to build what I need.

I only do m68k with MMU, and use Debian (ports) userland.
Perhaps https://landley.net/toybox/ might give you a nommu userland.

> More importantly, I think I get your point, you think that the testing
> should be integrated and complain if it's broken - at least by bots.  I
> don't think this is practical in all cases, unfortunately.

Exactly:
  - (cross)building kernel module tests is easy, and included in a
     kernel build, so build failures are detected early.
  - (cross)building the userland testing tools is cumbersome.
So there's an opportunity for improvement...

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-01-31  7:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20241205-xarray-kunit-port-v1-1-ee44bc7aa201@gmail.com>
     [not found] ` <07cf896e-adf8-414f-a629-a808fc26014a@oracle.com>
2025-01-29 21:26   ` [PATCH] xarray: port tests to kunit Liam R. Howlett
2025-01-29 21:28     ` Tamir Duberstein
2025-01-29 22:26       ` Liam R. Howlett
2025-01-29 22:33         ` Tamir Duberstein
2025-01-29 23:02           ` Matthew Wilcox
2025-01-29 23:08             ` Tamir Duberstein
2025-01-29 23:11               ` Matthew Wilcox
2025-01-29 23:17                 ` Tamir Duberstein
2025-01-30  8:21         ` Geert Uytterhoeven
2025-01-30 12:51           ` Liam R. Howlett
2025-01-30 13:25             ` Geert Uytterhoeven
2025-01-30 14:05               ` Liam R. Howlett
2025-01-30 14:24                 ` Geert Uytterhoeven
2025-01-30 15:16                   ` Liam R. Howlett
2025-01-31  7:39                     ` Geert Uytterhoeven
2025-01-30 14:09               ` Lorenzo Stoakes
2025-01-30 14:38                 ` Geert Uytterhoeven
2025-01-30 14:49                   ` Lorenzo Stoakes
2025-01-30 14:22           ` Matthew Wilcox
2025-01-31  0:22       ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox