Home » Mailing list archives » linux-kernel » 2008 » August » 21

Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.

Previous thread: [x86] apic timer tick interval is not so accurate in periodic mode by Kevin Hao on Wednesday, August 20, 2008 - 5:36 am. (2 messages)

Next thread: Major stalls due to commit 7c5026aa9b81dd45df8d3f4e0be73e485976a8b6 by Mel Gorman on Wednesday, August 20, 2008 - 8:05 am. (3 messages)
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, Christoph Lameter <cl@...>
Cc: <kosaki.motohiro@...>, <tokunaga.keiich@...>
Subject: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 7:05 am

Hi Cristoph,


Thank you for explain your quicklist plan at OLS.


So, I made summary to issue of quicklist.

if you have a bit time, Could you please read this mail and patches?

And, if possible, Could you please tell me your feeling?


--------------------------------------------------------------------


Now, Quicklist store some page in each CPU as cache.

(Each CPU has node_free_pages/16 pages)


and it is used for page table cache.

Then, exit() increase cache, the other hand fork() spent it.


So, if apache type (one parent and many child model) middleware run,

One CPU process fork(), Other CPU process the middleware work and exit().


At that time, One CPU don't have page table cache at all,

Others have maximum caches.


	QList_max = (#ofCPUs - 1) x Free / 16

	=> QList_max / (Free + QList_max) = (#ofCPUs - 1) / (16 + #ofCPUs - 1)


So, How much quicklist spent memory at maximum case?

That is #CPUs proposional because it is per CPU cache but cache amount calculation doesn't use #ofCPUs.


	Above calculation mean


	 Number of CPUs per node            2    4    8   16

	 ==============================  ====================

	 QList_max / (Free + QList_max)   5.8%  16%  30%  48%


Wow! Quicklist can spent about 50% memory at worst case.

More unfortunately, it doesn't have any cache shrinking mechanism.

So it cause some wrong thing.


1. End user misunderstand to memory leak happend.

	=> /proc/meminfo should display amount quicklist


2. It can cause OOM killer

	=> Amount of quicklists shouldn't be proposional to #ofCPUs.


--
[view original message]
From: Andrew Morton <akpm@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <linux-kernel@...>, <linux-mm@...>, <cl@...>, <kosaki.motohiro@...>, <tokunaga.keiich@...>, <stable@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 2:31 pm

On Wed, 20 Aug 2008 20:05:51 +0900


OK, that's a fatal bug and it's present in 2.6.25.x and 2.6.26.x.  A

serious issue.


The patches do apply to both stable kernels and I have tagged them for

backporting into them.  They're nice and small, but I didn't get a

really solid yes-this-is-what-we-should-do from Christoph?


This (from [patch 2/2]): "(Although its patch applied, quicklist can

waste 64GB on 1TB server (= 1TB / 16), it is still too much??)" is a

bit of a worry.  Yes, 64GB is too much!  But at least this is now only

a performance issue rather than a stability issue, yes?

--
[view original message]
From: Robin Holt <holt@...>
To: Andrew Morton <akpm@...>, KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, <stable@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 10:42 pm

That 64GB is not quite correct.  That assumes all 1TB is free.  The

quicklists are trimmed down as the nodes undergo allocations.  The

problem I see right now is that page tables allocated on one node and

freed on a cpu on a different node could be placed early enough on the

quicklist that it will not be freed until the other node gets under

memory pressure.


Could you give the following a try?  It hasn't even been compiled.  I

think this in addition to your cpus per node change are the right thing

to do.


Thanks,

Robin


Index: ia64-cleanups/include/linux/quicklist.h

===================================================================

--- ia64-cleanups.orig/include/linux/quicklist.h	2008-08-20 21:35:10.000000000 -0500

+++ ia64-cleanups/include/linux/quicklist.h	2008-08-20 21:38:00.891943270 -0500

@@ -66,6 +66,15 @@ static inline void __quicklist_free(int 


 static inline void quicklist_free(int nr, void (*dtor)(void *), void *pp)

 {

+#ifdef CONFIG_NUMA

+	unsigned long nid = page_to_nid(virt_to_page(pp));

+

+	if (unlikely(nid != numa_node_id())) {

+		free_page((unsigned long)pp);

+		return;

+	}

+#endif

+

 	__quicklist_free(nr, dtor, pp, virt_to_page(pp));

 }


--
[view original message]
From: Christoph Lameter <cl@...>
To: Robin Holt <holt@...>
Cc: Andrew Morton <akpm@...>, KOSAKI Motohiro <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <tokunaga.keiich@...>, <stable@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Thursday, August 21, 2008 - 9:07 am

We removed this code because it frees a page before the TLB flush has been

performed. This code segment was the reason that quicklists were not accepted

for x86.

--
[view original message]
From: Robin Holt <holt@...>
To: Christoph Lameter <cl@...>
Cc: Robin Holt <holt@...>, Andrew Morton <akpm@...>, KOSAKI Motohiro <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <tokunaga.keiich@...>, <stable@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Thursday, August 21, 2008 - 9:14 am

How could we do this.  It was a _HUGE_ problem on altix boxes.  When you

started a jobs with a large number of MPI ranks, they would all start

from the shepherd process on a single node and the children would

migrate to a different cpu.  Unless subsequent jobs used enough memory

to flush those remote quicklists, we would end up with a depleted node

that never reclaimed.


Thanks,

Robin

--
[view original message]
From: Christoph Lameter <cl@...>
To: Robin Holt <holt@...>
Cc: Andrew Morton <akpm@...>, KOSAKI Motohiro <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <tokunaga.keiich@...>, <stable@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Thursday, August 21, 2008 - 9:18 am

Well I tried to get the quicklist stuff resolved at SGI multiple times last

year when the early free before flush was discovered but there did not seem to

be much interest at that point, so we dropped it.


In order to make this work correctly we would need to create a list of remote

pages. These remote pages would then be freed after the TLB flush.

--
[view original message]
From: Robin Holt <holt@...>
To: Christoph Lameter <cl@...>
Cc: Robin Holt <holt@...>, Andrew Morton <akpm@...>, KOSAKI Motohiro <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <tokunaga.keiich@...>, <stable@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Thursday, August 21, 2008 - 9:45 am

Well, now that you dope slap me, I vaguely remember this.  I also seem

to recall being very busy with other stuff and convincing myself that a

proper resolution would magically appear.  Argh.


Sorry,

Robin

--
[view original message]
From: Christoph Lameter <cl@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 10:10 am

I believe what I said at the OLS was that quicklists are fundamentally crappy

and should be replaced by something that works (Guess that is what you meant

by "plan"?). Quicklists were generalized from the IA64 arch code.


Good fixup but I would think that some more radical rework is needed.


Maybe some of this needs to vanish into the TLB handling logic?


Then I have thought for awhile that the main reason that quicklists exist are

the performance problems in the page allocator. If you can make the single

page alloc / free pass competitive in performance with quicklists then we

could get rid of all uses.


--
[view original message]
From: Robin Holt <holt@...>
To: Christoph Lameter <cl@...>
Cc: KOSAKI Motohiro <kosaki.motohiro@...>, LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 10:13 pm

It is more than the free/alloc cycle, the quicklist saves us from

having to zero the page.  In a sparsely filled page table, it saves time

and cache footprint.  In a heavily used page table, you end up with a

near wash.


One problem I see is somebody got rid of the node awareness.  We used

to not put pages onto a quicklist when they were being released from a

different node than the cpu is on.  Not sure where that went.  It was

done because of the trap page problem described here.


Thanks,

Robin

--
[view original message]
From: David Miller <davem@...>
To: <holt@...>
Cc: <cl@...>, <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 11:08 pm

From: Robin Holt <holt@sgi.com>


NUMA awareness is one of the reasons I keep thinking about dropping

quicklist usage on sparc64.


Using SLAB/SLUB for the page table bits with appropriate constructor

and destructor bits ought to be able to approximate the gains

from avoiding the initialization for cached objects.

--
[view original message]
From: Christoph Lameter <cl@...>
To: David Miller <davem@...>
Cc: <holt@...>, <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Thursday, August 21, 2008 - 9:10 am

Its a bit strange to use the small object allocator for page sized

allocations. Plus there is this tie in with the tlb flushing logic. So I think

this would be more clean if it would be moved into the asm-generic/tlb.h or so.

--
[view original message]
From: Robin Holt <holt@...>
To: Christoph Lameter <cl@...>
Cc: KOSAKI Motohiro <kosaki.motohiro@...>, LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 10:16 pm

Poorly worded.  Here is the code I am referring to:


#ifdef CONFIG_NUMA

        unsigned long nid = page_to_nid(virt_to_page(pgtable_entry));


        if (unlikely(nid != numa_node_id())) {

                free_page((unsigned long)pgtable_entry);

                return;

        }

#endif


Thanks,

Robin

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Christoph Lameter <cl@...>
Cc: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 10:49 am

Hi


Unfortunately, Multiple ia64 customer of my campany are suffered by

Quicklist, now.

because Quicklist works well for HPC likes application, but business

server's application has very different behavior.

IOW, Quicklist works well on best case, but it doesn't concern to worst case.


So, if possible, I'd like to make short term solution.


What do you think wrong TLB handing?


Agreed.

Do you have any page allocator enhancement plan?

Can I help it?

--
[view original message]
From: Christoph Lameter <cl@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 0/2] Quicklist is slighly problematic.
Date: Wednesday, August 20, 2008 - 11:26 am

The generic TLB code could be made to do allow the allocation, the batching


A simple approach would be to use the queueing method used in quicklists in

the page allocator hotpath. But the devil is in the details .... There are

numerous checks for the type of page that are done by the page allocator and

not for the quicklists. Somehow we need to work around these.


--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, Christoph Lameter <cl@...>, <tokunaga.keiich@...>
Cc: <kosaki.motohiro@...>
Subject: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Wednesday, August 20, 2008 - 7:08 am

When a test program which does task migration runs, my 8GB box spends 800MB of memory

for quicklist. This is not memory leak but doesn't seem good.


% cat /proc/meminfo


MemTotal:        7701568 kB

MemFree:         4724672 kB

(snip)

Quicklists:       844800 kB


because


- My machine spec is

	number of numa node: 2

	number of cpus:      8 (4CPU x2 node)

        total mem:           8GB (4GB x2 node)

        free mem:            about 5GB


- Maximum quicklist usage is here


	 Number of CPUs per node            2    4    8   16

	 ==============================  ====================

	 QList_max / (Free + QList_max)   5.8%  16%  30%  48%


- Then, 4.7GB x 16% ~= 880MB.

  So, Quicklist can use 800MB.


So, if following spec machine run that program


   CPUs: 64 (8cpu x 8node)

   Mem:  1TB (128GB x8node)


Then, quicklist can waste 300GB (= 1TB x 30%).

it is fairly too large.


So, I don't like cache policies which is proportional to # of cpus.


My patch changes the number of caches

from:

   per-cpu-cache-amount = memory_on_node / 16

to

   per-cpu-cache-amount = memory_on_node / 16 / numder_of_cpus_on_node.


I think this is reasonable. but even if this patch is applied, quicklist

can cache tons of memory on big machine.


(Although its patch applied, quicklist can waste 64GB on 1TB server (= 1TB / 16),

 it is still too much??)


test program is below.

--------------------------------------------------------------------------------

#define _GNU_SOURCE


#include <stdio.h>

#include <errno.h>

#include <stdlib.h>

#include <string.h>

#include <sched.h>

#include <unistd.h>

#include <sys/mman.h>

#include <sys/wait.h>


#define BUFFSIZE 512


int max_cpu(void)	/* get max number of logical cpus from /proc/cpuinfo */

{

  FILE *fd;

  char *ret, buffer[BUFFSIZE];

  int cpu = 1;


  fd = fopen("/proc/cpuinfo", "r");

  if (fd == NULL) {

    perror("fopen(/proc/cpuinfo)");

    exit(EXIT_FAILURE);

  }

  while (1)...
[ message continues ]
[view original message]
From: Andrew Morton <akpm@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Christoph Lameter <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 2:46 am

sparc64 allmodconfig:


mm/quicklist.c: In function `max_pages':

mm/quicklist.c:44: error: invalid lvalue in unary `&'


we seem to have a made a spectacular mess of cpumasks lately.

--
[view original message]
From: David Miller <davem@...>
To: <akpm@...>
Cc: <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 3:13 am

From: Andrew Morton <akpm@linux-foundation.org>


It should explode similarly on x86, since it also defines node_to_cpumask()

as an inline function.


IA64 seems to be one of the few platforms to define this as a macro

evaluating to the node-to-cpumask array entry, so it's clear what

platform Motohiro-san did build testing on :-)

--
[view original message]
From: Mike Travis <travis@...>
To: David Miller <davem@...>
Cc: <akpm@...>, <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Monday, August 25, 2008 - 2:40 pm

I think the more correct usage would be:


	{

		node_to_cpumask_ptr(v, node);

		num_cpus_per_node = cpus_weight_nr(*v);

		max /= num_cpus_per_node;


		return max(max, min_pages);

	}


which should load 'v' with a pointer to the node_to_cpumask_map[node] entry

[and avoid using stack space for the cpumask_t variable for those arch's

that define a node_to_cpumask_map (or similar).]  Otherwise a local cpumask_t

variable '_v' is created to which 'v' is pointing to and thus can be used

directly as an arg to the cpu_xxx ops.


Thanks,


--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Mike Travis <travis@...>
Cc: <kosaki.motohiro@...>, David Miller <davem@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Monday, August 25, 2008 - 7:31 pm

Thank you for your attension.

please see my latest patch (http://marc.info/?l=linux-mm&m=121966459713193&w=2)

it do that.


--
[view original message]
From: Andrew Morton <akpm@...>
To: David Miller <davem@...>
Cc: <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 3:27 am

Seems to compile OK on x86_32, x86_64, ia64 and powerpc for some reason.


This seems to fix things on sparc64:


--- a/mm/quicklist.c~mm-quicklist-shouldnt-be-proportional-to-number-of-cpus-fix

+++ a/mm/quicklist.c

@@ -28,7 +28,7 @@ static unsigned long max_pages(unsigned

 	unsigned long node_free_pages, max;

 	int node = numa_node_id();

 	struct zone *zones = NODE_DATA(node)->node_zones;

-	int num_cpus_per_node;

+	cpumask_t node_cpumask;


 	node_free_pages =

 #ifdef CONFIG_ZONE_DMA

@@ -41,8 +41,8 @@ static unsigned long max_pages(unsigned 


 	max = node_free_pages / FRACTION_OF_NODE_MEM;


-	num_cpus_per_node = cpus_weight_nr(node_to_cpumask(node));

-	max /= num_cpus_per_node;

+	node_cpumask = node_to_cpumask(node);

+	max /= cpus_weight_nr(node_cpumask);


 	return max(max, min_pages);

 }

_


--
[view original message]
From: Peter Zijlstra <peterz@...>
To: Andrew Morton <akpm@...>
Cc: David Miller <davem@...>, <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, travis <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 5:32 am

humm, I thought we wanted to keep cpumask_t stuff away from our stack -

since on insanely large SGI boxen (/me looks at mike) the thing becomes

512 bytes.


--
[view original message]
From: Mike Travis <travis@...>
To: Peter Zijlstra <peterz@...>
Cc: Andrew Morton <akpm@...>, David Miller <davem@...>, <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Monday, August 25, 2008 - 2:44 pm

Yes, thanks for pointing that out!  I did send out an alternate coding

that should keep the cpumask_t off the stack for those arch's that need

to worry about it (using the node_to_cpumask_ptr function).  I should

probably devote some time to documenting some of these gotcha's in one

of the Doc.../ files.


Mike

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Peter Zijlstra <peterz@...>
Cc: <kosaki.motohiro@...>, Andrew Morton <akpm@...>, David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, travis <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 6:04 am

Hi Peter,


Hm, interesting.

I think following patch fill your point, right?


but I worry about it works on sparc64...


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


---

 mm/quicklist.c |    9 ++++++++-

 1 file changed, 8 insertions(+), 1 deletion(-)


Index: b/mm/quicklist.c

===================================================================

--- a/mm/quicklist.c

+++ b/mm/quicklist.c

@@ -26,7 +26,10 @@ DEFINE_PER_CPU(struct quicklist, quickli

 static unsigned long max_pages(unsigned long min_pages)

 {

 	unsigned long node_free_pages, max;

-	struct zone *zones = NODE_DATA(numa_node_id())->node_zones;

+	int node = numa_node_id();

+	struct zone *zones = NODE_DATA(node)->node_zones;

+	int num_cpus_on_node;

+	node_to_cpumask_ptr(cpumask_on_node, node);


 	node_free_pages =

 #ifdef CONFIG_ZONE_DMA

@@ -38,6 +41,10 @@ static unsigned long max_pages(unsigned

 		zone_page_state(&zones[ZONE_NORMAL], NR_FREE_PAGES);


 	max = node_free_pages / FRACTION_OF_NODE_MEM;

+

+	num_cpus_on_node = cpus_weight_nr(*cpumask_on_node);

+	max /= num_cpus_on_node;

+

 	return max(max, min_pages);

 }


--
[view original message]
From: Mike Travis <travis@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: Peter Zijlstra <peterz@...>, Andrew Morton <akpm@...>, David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Monday, August 25, 2008 - 2:48 pm

Exactly!  And (many thanks to them!) the sparc maintainers have

implemented a similar internal function definition for node_to_cpumask_ptr().


Mike

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Mike Travis <travis@...>
Cc: <kosaki.motohiro@...>, Peter Zijlstra <peterz@...>, Andrew Morton <akpm@...>, David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Monday, August 25, 2008 - 7:33 pm

Can I think get your Ack?


--
[view original message]
From: Mike Travis <travis@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: Peter Zijlstra <peterz@...>, Andrew Morton <akpm@...>, David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Tuesday, August 26, 2008 - 4:35 pm

Based on code review, sure.  I'll also give it a try on one of my

test machines as soon as I can.


Mike

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <kosaki.motohiro@...>, Peter Zijlstra <peterz@...>, Andrew Morton <akpm@...>, David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, travis <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 6:22 am

Sorry, following patch is crap.

please forget it.


--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <kosaki.motohiro@...>, Peter Zijlstra <peterz@...>, Andrew Morton <akpm@...>, David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, travis <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 8:02 am

Ah, it's a ok.

it is not crap.


node_to_cpumask_ptr() of generic arch makes local cpumask_t variable.


#define node_to_cpumask_ptr(v, node)                                    \

                cpumask_t _##v = node_to_cpumask(node);                 \

                const cpumask_t *v = &_##v


but gcc optimazer can erase it.

So, it doesn't consume any stack.

checkstack.pl doesn't outpu quicklist related function.


% objdump -d vmlinux | ./scripts/checkstack.pl

0xa000000100647a86 sn2_global_tlb_purge [vmlinux]:      2176

0xa000000100264e86 read_kcore [vmlinux]:                1360

0xa0000001001042a6 crash_save_cpu [vmlinux]:            1152

0xa0000001007869e6 e1000_check_options [vmlinux]:       1152

0xa00000010021b9c6 __mpage_writepage [vmlinux]:         1136

0xa00000010034e9c6 fat_alloc_clusters [vmlinux]:        1136

0xa0000001009c29c6 efi_uart_console_only [vmlinux]:     1136

0xa00000010034afa6 fat_add_entries [vmlinux]:           1088

0xa00000010034d186 fat_free_clusters [vmlinux]:         1088

0xa00000010051f396 tg3_get_estats [vmlinux]:            1072

0xa000000100348f26 fat_alloc_new_dir [vmlinux]:         1040

0xa00000010079df26 cpu_init [vmlinux]:                  1040

0xa00000010020fa46 block_read_full_page [vmlinux]:      1024

0xa00000010021c906 do_mpage_readpage [vmlinux]:         1024

0xa000000100016106 kernel_thread [vmlinux]:             976

0xa000000100031486 convert_to_non_syscall [vmlinux]:    928

0xa0000001001d9486 do_sys_poll [vmlinux]:               848

0xa0000001007a6406 sn_cpu_init [vmlinux]:               768

0xa00000010004bc66 find_save_locs [vmlinux]:            752

0xa0000001009faa26 sn_setup [vmlinux]:                  656

0xa000000100034326 arch_ptrace [vmlinux]:               624

0xa000000100197be6 shmem_getpage [vmlinux]:             624

0xa000000100119046 cpuset_write_resmask [vmlinux]:      608

0xa0000001001da4c6 do_select [vmlinux]:                 592

0xa00000010064dfd0 sn_topology_show [vmlinux]:          592

0xa00000010005b7e6 vm_...
[ message continues ]
[view original message]
From: David Miller <davem@...>
To: <kosaki.motohiro@...>
Cc: <peterz@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 6:09 am

From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


It should.

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: David Miller <davem@...>
Cc: <kosaki.motohiro@...>, <peterz@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 6:13 am

Could you please confirm it?


--
[view original message]
From: David Miller <davem@...>
To: <kosaki.motohiro@...>
Cc: <peterz@...>, <akpm@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, <travis@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 6:26 am

From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


davem@sunset:~/src/GIT/net-2.6$ patch -p1 <diff

patching file mm/quicklist.c

davem@sunset:~/src/GIT/net-2.6$ make mm/quicklist.o

  CHK     include/linux/version.h

  CHK     include/linux/utsrelease.h

  CALL    scripts/checksyscalls.sh

  CC      mm/quicklist.o

davem@sunset:~/src/GIT/net-2.6$

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Andrew Morton <akpm@...>
Cc: David Miller <davem@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 3:31 am

Thank you!!

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: David Miller <davem@...>
Cc: <akpm@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Thursday, August 21, 2008 - 3:18 am

Thank you good advice.

I don't have sparc64 machine but I can get borrowing x86 machine.

So, I'll test on x86 today.

--
[view original message]
From: Christoph Lameter <cl@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 2/2] quicklist shouldn't be proportional to # of CPUs
Date: Wednesday, August 20, 2008 - 11:27 am

Looks good.


Acked-by: Christoph Lameter <cl@linux-foundation.org>

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: LKML <linux-kernel@...>, linux-mm <linux-mm@...>, Andrew Morton <akpm@...>, Christoph Lameter <cl@...>, <tokunaga.keiich@...>
Cc: <kosaki.motohiro@...>
Subject: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Wednesday, August 20, 2008 - 7:07 am

Now, Quicklist can spent several GB memory.

So, if end user can't hou much spent memory, he misunderstand to memory leak happend.


after this patch applied, /proc/meminfo output following.


% cat /proc/meminfo


MemTotal:        7701504 kB

MemFree:         5159040 kB

Buffers:          112960 kB

Cached:           337536 kB

SwapCached:            0 kB

Active:           218944 kB

Inactive:         350848 kB

Active(anon):     120832 kB

Inactive(anon):        0 kB

Active(file):      98112 kB

Inactive(file):   350848 kB

Unevictable:           0 kB

Mlocked:               0 kB

SwapTotal:       2031488 kB

SwapFree:        2031488 kB

Dirty:               320 kB

Writeback:             0 kB

AnonPages:        119488 kB

Mapped:            38528 kB

Slab:            1595712 kB

SReclaimable:      23744 kB

SUnreclaim:      1571968 kB

PageTables:        14336 kB

NFS_Unstable:          0 kB

Bounce:                0 kB

WritebackTmp:          0 kB

CommitLimit:     5882240 kB

Committed_AS:     356672 kB

VmallocTotal:   17592177655808 kB

VmallocUsed:       29056 kB

VmallocChunk:   17592177626304 kB

Quicklists:       283776 kB

HugePages_Total:     0

HugePages_Free:      0

HugePages_Rsvd:      0

HugePages_Surp:      0

Hugepagesize:    262144 kB


Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


---

 fs/proc/proc_misc.c       |    6 ++++--

 include/linux/quicklist.h |    7 +++++++

 2 files changed, 11 insertions(+), 2 deletions(-)


Index: b/fs/proc/proc_misc.c

===================================================================

--- a/fs/proc/proc_misc.c

+++ b/fs/proc/proc_misc.c

@@ -202,7 +202,8 @@ static int meminfo_read_proc(char *page,

 		"Committed_AS:   %8lu kB\n"

 		"VmallocTotal:   %8lu kB\n"

 		"VmallocUsed:    %8lu kB\n"

-		"VmallocChunk:   %8lu kB\n",

+		"VmallocChunk:   %8lu kB\n"

+		"Quicklists:     %8lu kB\n",

 		K(i.totalram),

 		K(i.freeram),

 		K(i.bufferram),

@@ -242,7 +243,8 @@ static int meminfo_read_proc(char *page,

 		K(committed)...
[ message continues ]
[view original message]
From: Andrew Morton <akpm@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>, <kosaki.motohiro@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Wednesday, August 20, 2008 - 2:35 pm

On Wed, 20 Aug 2008 20:07:06 +0900


quicklist_total_size() is racy against cpu hotplug.  That's OK for

/proc/meminfo purposes (occasional transient inaccuracy?), but will it

crash?  Not in the current implementation of per_cpu() afaict, but it

might crash if we ever teach cpu hotunplug to free up the percpu

resources.


I see no cpu hotplug handling in the quicklist code.  Do we leak all

the hot-unplugged CPU's pages?


--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Andrew Morton <akpm@...>
Cc: <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Thursday, August 21, 2008 - 3:36 am

First, Quicklist doesn't concern to cpu hotplug at all.

it is another quicklist problem.


Next, I think it doesn't cause crash. but I haven't any test.

So, I'll test cpu hotplug/unplug testing today.


Yes.


Thanks!

--
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <kosaki.motohiro@...>, Andrew Morton <akpm@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Thursday, August 21, 2008 - 9:05 pm

OK.

I ran cpu hotplug/unplug coutinuous workload over 12H.

then, system crash doesn't happend.


So, I believe my patch is cpu unplug safe.


test method

--------------------------------------------------------------

1. open 7 terminal and following script run on each console.


CPU=cpuXXX; while true; do echo 0 > /sys/devices/system/cpu/$CPU/online; echo 1 > /sys/devi

ces/system/cpu/$CPU/online;done


2. open another console, following command run.


watch -n 1 cat /proc/meminfo


--
[view original message]
From: Andrew Morton <akpm@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Friday, August 22, 2008 - 12:28 am

err, which patch?


I presently have:


mm-show-quicklist-memory-usage-in-proc-meminfo.patch

mm-show-quicklist-memory-usage-in-proc-meminfo-fix.patch

mm-quicklist-shouldnt-be-proportional-to-number-of-cpus.patch

mm-quicklist-shouldnt-be-proportional-to-number-of-cpus-fix.patch


Is that what you have?


I'll consolidate them into two patches and will append them here.  Please check.


From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


At present the quicklists store some page for each CPU as a cache.  (Each

CPU has node_free_pages/16 pages)


It is used for page table cache.  Then, exit() increase cache, the other

hand fork() spent it.


So, if apache type (one parent and many child model) middleware run, One

CPU process fork(), Other CPU process the middleware work and exit().


At that time, One CPU don't have page table cache at all, Others have

maximum caches.


	QList_max = (#ofCPUs - 1) x Free / 16

	=> QList_max / (Free + QList_max) = (#ofCPUs - 1) / (16 + #ofCPUs - 1)


So, How much quicklist spent memory at maximum case?  That is #CPUs

proposional because it is per CPU cache but cache amount calculation

doesn't use #ofCPUs.


	Above calculation mean


	 Number of CPUs per node            2    4    8   16

	 ==============================  ====================

	 QList_max / (Free + QList_max)   5.8%  16%  30%  48%


Wow!  Quicklist can spent about 50% memory at worst case.  More

unfortunately, it doesn't have any cache shrinking mechanism.  So it cause

some wrong thing.


1. End user misunderstand to memory leak happend.

	=> /proc/meminfo should display amount quicklist


2. It can cause OOM killer

	=> Amount of quicklists shouldn't be proportional to number of CPUs.


This patch:


Quicklists can consume several GB memory.  So, if end user can't see how

much memory is used, he can fail to understand why a memory leak happend.


after this patch applied, /proc/meminfo output following.


% cat /proc/meminfo


MemTotal:        7701504 kB

M...
[ message continues ]
[view original message]
From: KOSAKI Motohiro <kosaki.motohiro@...>
To: Andrew Morton <akpm@...>
Cc: <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Saturday, August 23, 2008 - 4:24 am

Andrew, Thank you for your attention.


I test on


mm-show-quicklist-memory-usage-in-proc-meminfo.patch

mm-show-quicklist-memory-usage-in-proc-meminfo-fix.patch


and 


http://marc.info/?l=linux-mm&m=121931317407295&w=2 


the above url's patch already checked sparc64 compilable by David.

and I tested it.


So, if possible, Could you replace current quicklist-shouldnt-be-proportional

patch to that?

(of cource, current -mm patch also works well)


the same patch attached below because web mail interface is a bit ugly.


From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


When a test program which does task migration runs, my 8GB box spends

800MB of memory for quicklist.  This is not memory leak but doesn't seem

good.


% cat /proc/meminfo


MemTotal:        7701568 kB

MemFree:         4724672 kB

(snip)

Quicklists:       844800 kB


because


- My machine spec is

	number of numa node: 2

	number of cpus:      8 (4CPU x2 node)

        total mem:           8GB (4GB x2 node)

        free mem:            about 5GB


- Maximum quicklist usage is here


	 Number of CPUs per node            2    4    8   16

	 ==============================  ====================

	 QList_max / (Free + QList_max)   5.8%  16%  30%  48%


- Then, 4.7GB x 16% ~= 880MB.

  So, Quicklist can use 800MB.


So, if following spec machine run that program


   CPUs: 64 (8cpu x 8node)

   Mem:  1TB (128GB x8node)


Then, quicklist can waste 300GB (= 1TB x 30%).  It is too large.


So, I don't like cache policies which is proportional to # of cpus.


My patch changes the number of caches

from:

   per-cpu-cache-amount = memory_on_node / 16

to

   per-cpu-cache-amount = memory_on_node / 16 / number_of_cpus_on_node.


I think this is reasonable.  but even if this patch is applied, quicklist

can cache tons of memory on big machine.


(Although its patch applied, quicklist can waste 64GB on 1TB server (= 1TB

/ 16), it is still too much??)


test program is below.

---------------------...
[ message continues ]
[view original message]
From: Andrew Morton <akpm@...>
To: KOSAKI Motohiro <kosaki.motohiro@...>
Cc: <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Sunday, August 24, 2008 - 1:29 am

OK, there's just too much potential for miscommunication and error here.


Please resend everything as a sequence-numbered, fully-changlelogged

signed-off patch series against current mainline.


--
[view original message]
From: Robin Holt <holt@...>
To: Andrew Morton <akpm@...>
Cc: KOSAKI Motohiro <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <cl@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Friday, August 22, 2008 - 9:23 am

Christoph,


Could we maybe add a per_cpu off-node quicklist and just always free

that in check_pgt_cache?  That would get us back the freeing of off-node

page tables.


Thanks,

Robin


--
[view original message]
From: Christoph Lameter <cl@...>
To: Robin Holt <holt@...>
Cc: Andrew Morton <akpm@...>, KOSAKI Motohiro <kosaki.motohiro@...>, <linux-kernel@...>, <linux-mm@...>, <tokunaga.keiich@...>
Subject: Re: [RFC][PATCH 1/2] Show quicklist at meminfo
Date: Friday, August 22, 2008 - 9:56 am

Yes that is what I suggested and if you check your email from last year then

you will find an internal discussion and patches for such an approach.

--
Previous thread: [x86] apic timer tick interval is not so accurate in periodic mode by Kevin Hao on Wednesday, August 20, 2008 - 5:36 am. (2 messages)

Next thread: Major stalls due to commit 7c5026aa9b81dd45df8d3f4e0be73e485976a8b6 by Mel Gorman on Wednesday, August 20, 2008 - 8:05 am. (3 messages)