LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] lib/test_free_pages: Add basic progress indicators
@ 2020-10-18 14:04 Geert Uytterhoeven
  2020-10-18 14:25 ` Matthew Wilcox
  0 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2020-10-18 14:04 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andrew Morton, Mike Rapoport, linux-kernel, Geert Uytterhoeven

The test module to check that free_pages() does not leak memory does not
provide any feedback whatsoever its state or progress, but may take some
time on slow machines.  Add the printing of messages upon starting each
phase of the test, and upon completion.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 lib/test_free_pages.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/test_free_pages.c b/lib/test_free_pages.c
index 074e76bd76b2b3c9..25ae1ac2624ae481 100644
--- a/lib/test_free_pages.c
+++ b/lib/test_free_pages.c
@@ -5,6 +5,8 @@
  * Author: Matthew Wilcox <willy@infradead.org>
  */
 
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
 #include <linux/gfp.h>
 #include <linux/mm.h>
 #include <linux/module.h>
@@ -26,8 +28,11 @@ static void test_free_pages(gfp_t gfp)
 
 static int m_in(void)
 {
+	pr_info("Testing with GFP_KERNEL\n");
 	test_free_pages(GFP_KERNEL);
+	pr_info("Testing with GFP_KERNEL | __GFP_COMP\n");
 	test_free_pages(GFP_KERNEL | __GFP_COMP);
+	pr_info("Test completed\n");
 
 	return 0;
 }
-- 
2.17.1


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

* Re: [PATCH] lib/test_free_pages: Add basic progress indicators
  2020-10-18 14:04 [PATCH] lib/test_free_pages: Add basic progress indicators Geert Uytterhoeven
@ 2020-10-18 14:25 ` Matthew Wilcox
  2020-10-18 14:39   ` Geert Uytterhoeven
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2020-10-18 14:25 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Andrew Morton, Mike Rapoport, linux-kernel

On Sun, Oct 18, 2020 at 04:04:45PM +0200, Geert Uytterhoeven wrote:
> The test module to check that free_pages() does not leak memory does not
> provide any feedback whatsoever its state or progress, but may take some
> time on slow machines.  Add the printing of messages upon starting each
> phase of the test, and upon completion.

It's not supposed to take a long time.  Can you crank down that 1000 *
1000 to something more appropriate?


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

* Re: [PATCH] lib/test_free_pages: Add basic progress indicators
  2020-10-18 14:25 ` Matthew Wilcox
@ 2020-10-18 14:39   ` Geert Uytterhoeven
  2020-10-18 15:01     ` Matthew Wilcox
  0 siblings, 1 reply; 7+ messages in thread
From: Geert Uytterhoeven @ 2020-10-18 14:39 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Andrew Morton, Mike Rapoport, Linux Kernel Mailing List

Hi Matthew,

On Sun, Oct 18, 2020 at 4:25 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Sun, Oct 18, 2020 at 04:04:45PM +0200, Geert Uytterhoeven wrote:
> > The test module to check that free_pages() does not leak memory does not
> > provide any feedback whatsoever its state or progress, but may take some
> > time on slow machines.  Add the printing of messages upon starting each
> > phase of the test, and upon completion.
>
> It's not supposed to take a long time.  Can you crank down that 1000 *

It took 1m11s on ARAnyM, running on an i7-8700K.
Real hardware may even take longer.

> 1000 to something more appropriate?

What would be a suitable value? You do want to see it "leak gigabytes
of memory and probably OOM your system" if something's wrong,
so decreasing the value a lot may not be a good idea?

Regardless, if it OOMs, I think you do want to see this happens
while running this test.

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] 7+ messages in thread

* Re: [PATCH] lib/test_free_pages: Add basic progress indicators
  2020-10-18 14:39   ` Geert Uytterhoeven
@ 2020-10-18 15:01     ` Matthew Wilcox
  2020-10-18 17:12       ` Mike Rapoport
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2020-10-18 15:01 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Andrew Morton, Mike Rapoport, Linux Kernel Mailing List

On Sun, Oct 18, 2020 at 04:39:27PM +0200, Geert Uytterhoeven wrote:
> Hi Matthew,
> 
> On Sun, Oct 18, 2020 at 4:25 PM Matthew Wilcox <willy@infradead.org> wrote:
> > On Sun, Oct 18, 2020 at 04:04:45PM +0200, Geert Uytterhoeven wrote:
> > > The test module to check that free_pages() does not leak memory does not
> > > provide any feedback whatsoever its state or progress, but may take some
> > > time on slow machines.  Add the printing of messages upon starting each
> > > phase of the test, and upon completion.
> >
> > It's not supposed to take a long time.  Can you crank down that 1000 *
> 
> It took 1m11s on ARAnyM, running on an i7-8700K.
> Real hardware may even take longer.

71 seconds is clearly too long.  0.7 seconds would be fine, so 10 * 1000
would be appropriate, but then that's only 320MB which might not be
enough to notice on a modern machine.

> > 1000 to something more appropriate?
> 
> What would be a suitable value? You do want to see it "leak gigabytes
> of memory and probably OOM your system" if something's wrong,
> so decreasing the value a lot may not be a good idea?
> 
> Regardless, if it OOMs, I think you do want to see this happens
> while running this test.

How about scaling with the amount of memory on the machine?

This might cause problems on machines with terabytes of memory.
Maybe we should cap it at a terabyte?

diff --git a/lib/test_free_pages.c b/lib/test_free_pages.c
index 074e76bd76b2..aa18fa52290a 100644
--- a/lib/test_free_pages.c
+++ b/lib/test_free_pages.c
@@ -9,11 +9,11 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 
-static void test_free_pages(gfp_t gfp)
+static void test_free_pages(gfp_t gfp, unsigned long totalram)
 {
-	unsigned int i;
+	unsigned long i, max = totalram / 8;
 
-	for (i = 0; i < 1000 * 1000; i++) {
+	for (i = 0; i < max; i++) {
 		unsigned long addr = __get_free_pages(gfp, 3);
 		struct page *page = virt_to_page(addr);
 
@@ -26,8 +26,11 @@ static void test_free_pages(gfp_t gfp)
 
 static int m_in(void)
 {
-	test_free_pages(GFP_KERNEL);
-	test_free_pages(GFP_KERNEL | __GFP_COMP);
+	struct sysinfo si;
+
+	si_meminfo(&si);
+	test_free_pages(GFP_KERNEL, si.totalram);
+	test_free_pages(GFP_KERNEL | __GFP_COMP, si.totalram);
 
 	return 0;
 }

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

* Re: [PATCH] lib/test_free_pages: Add basic progress indicators
  2020-10-18 15:01     ` Matthew Wilcox
@ 2020-10-18 17:12       ` Mike Rapoport
  2020-10-19 14:05         ` Matthew Wilcox
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Rapoport @ 2020-10-18 17:12 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Geert Uytterhoeven, Andrew Morton, Linux Kernel Mailing List

On Sun, Oct 18, 2020 at 04:01:46PM +0100, Matthew Wilcox wrote:
> On Sun, Oct 18, 2020 at 04:39:27PM +0200, Geert Uytterhoeven wrote:
> > Hi Matthew,
> > 
> > On Sun, Oct 18, 2020 at 4:25 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > On Sun, Oct 18, 2020 at 04:04:45PM +0200, Geert Uytterhoeven wrote:
> > > > The test module to check that free_pages() does not leak memory does not
> > > > provide any feedback whatsoever its state or progress, but may take some
> > > > time on slow machines.  Add the printing of messages upon starting each
> > > > phase of the test, and upon completion.
> > >
> > > It's not supposed to take a long time.  Can you crank down that 1000 *
> > 
> > It took 1m11s on ARAnyM, running on an i7-8700K.
> > Real hardware may even take longer.
> 
> 71 seconds is clearly too long.  0.7 seconds would be fine, so 10 * 1000
> would be appropriate, but then that's only 320MB which might not be
> enough to notice on a modern machine.
> 
> > > 1000 to something more appropriate?
> > 
> > What would be a suitable value? You do want to see it "leak gigabytes
> > of memory and probably OOM your system" if something's wrong,
> > so decreasing the value a lot may not be a good idea?
> > 
> > Regardless, if it OOMs, I think you do want to see this happens
> > while running this test.
> 
> How about scaling with the amount of memory on the machine?
> 
> This might cause problems on machines with terabytes of memory.
> Maybe we should cap it at a terabyte?

On ARAnyM wih 782 MBytes of RAM running on i7-8650U it takes ~1.75
seconds.
Still, I think adding some verbosity to the test wouldn't hurt ;-)

> diff --git a/lib/test_free_pages.c b/lib/test_free_pages.c
> index 074e76bd76b2..aa18fa52290a 100644
> --- a/lib/test_free_pages.c
> +++ b/lib/test_free_pages.c
> @@ -9,11 +9,11 @@
>  #include <linux/mm.h>
>  #include <linux/module.h>
>  
> -static void test_free_pages(gfp_t gfp)
> +static void test_free_pages(gfp_t gfp, unsigned long totalram)
>  {
> -	unsigned int i;
> +	unsigned long i, max = totalram / 8;
>  
> -	for (i = 0; i < 1000 * 1000; i++) {
> +	for (i = 0; i < max; i++) {
>  		unsigned long addr = __get_free_pages(gfp, 3);
>  		struct page *page = virt_to_page(addr);
>  
> @@ -26,8 +26,11 @@ static void test_free_pages(gfp_t gfp)
>  
>  static int m_in(void)
>  {
> -	test_free_pages(GFP_KERNEL);
> -	test_free_pages(GFP_KERNEL | __GFP_COMP);
> +	struct sysinfo si;
> +
> +	si_meminfo(&si);
> +	test_free_pages(GFP_KERNEL, si.totalram);
> +	test_free_pages(GFP_KERNEL | __GFP_COMP, si.totalram);
>  
>  	return 0;
>  }

-- 
Sincerely yours,
Mike.

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

* Re: [PATCH] lib/test_free_pages: Add basic progress indicators
  2020-10-18 17:12       ` Mike Rapoport
@ 2020-10-19 14:05         ` Matthew Wilcox
  2020-10-19 14:20           ` Geert Uytterhoeven
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2020-10-19 14:05 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Geert Uytterhoeven, Andrew Morton, Linux Kernel Mailing List

On Sun, Oct 18, 2020 at 08:12:52PM +0300, Mike Rapoport wrote:
> On Sun, Oct 18, 2020 at 04:01:46PM +0100, Matthew Wilcox wrote:
> > On Sun, Oct 18, 2020 at 04:39:27PM +0200, Geert Uytterhoeven wrote:
> > > Hi Matthew,
> > > 
> > > On Sun, Oct 18, 2020 at 4:25 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > > On Sun, Oct 18, 2020 at 04:04:45PM +0200, Geert Uytterhoeven wrote:
> > > > > The test module to check that free_pages() does not leak memory does not
> > > > > provide any feedback whatsoever its state or progress, but may take some
> > > > > time on slow machines.  Add the printing of messages upon starting each
> > > > > phase of the test, and upon completion.
> > > >
> > > > It's not supposed to take a long time.  Can you crank down that 1000 *
> > > 
> > > It took 1m11s on ARAnyM, running on an i7-8700K.
> > > Real hardware may even take longer.
> > 
> > 71 seconds is clearly too long.  0.7 seconds would be fine, so 10 * 1000
> > would be appropriate, but then that's only 320MB which might not be
> > enough to notice on a modern machine.
> > 
> > > > 1000 to something more appropriate?
> > > 
> > > What would be a suitable value? You do want to see it "leak gigabytes
> > > of memory and probably OOM your system" if something's wrong,
> > > so decreasing the value a lot may not be a good idea?
> > > 
> > > Regardless, if it OOMs, I think you do want to see this happens
> > > while running this test.
> > 
> > How about scaling with the amount of memory on the machine?
> > 
> > This might cause problems on machines with terabytes of memory.
> > Maybe we should cap it at a terabyte?
> 
> On ARAnyM wih 782 MBytes of RAM running on i7-8650U it takes ~1.75
> seconds.

That seems like a somewhat unusual configuration.  I think it's pretty
strange to find an actual m68k with more than 128MB of memory.  I mean,
I can set up my laptop to believe it has 64TB of memory, and this will
run slowly, but I don't think it's any real problem.

> Still, I think adding some verbosity to the test wouldn't hurt ;-)

I prefer the unix philosophy of only emitting messages if something's
wrong.


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

* Re: [PATCH] lib/test_free_pages: Add basic progress indicators
  2020-10-19 14:05         ` Matthew Wilcox
@ 2020-10-19 14:20           ` Geert Uytterhoeven
  0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2020-10-19 14:20 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Mike Rapoport, Andrew Morton, Linux Kernel Mailing List

Hi Matthew,

On Mon, Oct 19, 2020 at 4:05 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Sun, Oct 18, 2020 at 08:12:52PM +0300, Mike Rapoport wrote:
> > On Sun, Oct 18, 2020 at 04:01:46PM +0100, Matthew Wilcox wrote:
> > > On Sun, Oct 18, 2020 at 04:39:27PM +0200, Geert Uytterhoeven wrote:
> > > > On Sun, Oct 18, 2020 at 4:25 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > > > On Sun, Oct 18, 2020 at 04:04:45PM +0200, Geert Uytterhoeven wrote:
> > > > > > The test module to check that free_pages() does not leak memory does not
> > > > > > provide any feedback whatsoever its state or progress, but may take some
> > > > > > time on slow machines.  Add the printing of messages upon starting each
> > > > > > phase of the test, and upon completion.
> > > > >
> > > > > It's not supposed to take a long time.  Can you crank down that 1000 *
> > > >
> > > > It took 1m11s on ARAnyM, running on an i7-8700K.
> > > > Real hardware may even take longer.
> > >
> > > 71 seconds is clearly too long.  0.7 seconds would be fine, so 10 * 1000
> > > would be appropriate, but then that's only 320MB which might not be
> > > enough to notice on a modern machine.
> > >
> > > > > 1000 to something more appropriate?
> > > >
> > > > What would be a suitable value? You do want to see it "leak gigabytes
> > > > of memory and probably OOM your system" if something's wrong,
> > > > so decreasing the value a lot may not be a good idea?
> > > >
> > > > Regardless, if it OOMs, I think you do want to see this happens
> > > > while running this test.
> > >
> > > How about scaling with the amount of memory on the machine?
> > >
> > > This might cause problems on machines with terabytes of memory.
> > > Maybe we should cap it at a terabyte?
> >
> > On ARAnyM wih 782 MBytes of RAM running on i7-8650U it takes ~1.75
> > seconds.
>
> That seems like a somewhat unusual configuration.  I think it's pretty
> strange to find an actual m68k with more than 128MB of memory.  I mean,
> I can set up my laptop to believe it has 64TB of memory, and this will
> run slowly, but I don't think it's any real problem.

Have you tried it on one of your parisc boxes? They tend to have quite
some RAM, compared to CPU speed.

I gave the unmodified test a try on a 1.5 GHz Cortex-A15, which is still
a decent arm32 system, and it took 4.25s.

> > Still, I think adding some verbosity to the test wouldn't hurt ;-)
>
> I prefer the unix philosophy of only emitting messages if something's
> wrong.

This is not a normal program, but a test.
Alternatively, convert it to kunit, so we'll at least be aware when
it starts running, and when it has completed ;-)

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] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-18 14:04 [PATCH] lib/test_free_pages: Add basic progress indicators Geert Uytterhoeven
2020-10-18 14:25 ` Matthew Wilcox
2020-10-18 14:39   ` Geert Uytterhoeven
2020-10-18 15:01     ` Matthew Wilcox
2020-10-18 17:12       ` Mike Rapoport
2020-10-19 14:05         ` Matthew Wilcox
2020-10-19 14:20           ` Geert Uytterhoeven

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git