drain_node_pages: interrupt latency reduction / optimization

drain_node_pages: interrupt latency reduction / optimization

[Christoph Lameter]

1. Only disable interrupts if there is actually something to free

2. Only dirty the pcp cacheline if we actually freed something.

3. Disable interrupts for each single pcp and not for cleaning
  all the pcps in all zones of a node.

drain_node_pages is called every 2 seconds from cache_reap. This
fix should avoid most disabling of interrupts.

Signed-off-by: Christoph Lameter <clameter@sgi.com>

Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c	2006-03-10 10:34:56.000000000 -0800
+++ linux-2.6/mm/page_alloc.c	2006-03-10 12:55:02.000000000 -0800
@@ -599,7 +599,6 @@ void drain_node_pages(int nodeid)
 	int i, z;
 	unsigned long flags;
 
-	local_irq_save(flags);
 	for (z = 0; z < MAX_NR_ZONES; z++) {
 		struct zone *zone = NODE_DATA(nodeid)->node_zones + z;
 		struct per_cpu_pageset *pset;
@@ -609,11 +608,14 @@ void drain_node_pages(int nodeid)
 			struct per_cpu_pages *pcp;
 
 			pcp = &pset->pcp[i];
-			free_pages_bulk(zone, pcp->count, &pcp->list, 0);
-			pcp->count = 0;
+			if (pcp->count) {
+				local_irq_save(flags);
+				free_pages_bulk(zone, pcp->count, &pcp->list, 0);
+				pcp->count = 0;
+				local_irq_restore(flags);
+			}
 		}
 	}
-	local_irq_restore(flags);
 }
 #endif
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: drain_node_pages: interrupt latency reduction / optimization

[Andrew Morton]

Christoph Lameter <clameter@sgi.com> wrote:
>
> 1. Only disable interrupts if there is actually something to free
> 
> 2. Only dirty the pcp cacheline if we actually freed something.
> 
> 3. Disable interrupts for each single pcp and not for cleaning
>   all the pcps in all zones of a node.
> 
> drain_node_pages is called every 2 seconds from cache_reap. This
> fix should avoid most disabling of interrupts.
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c	2006-03-10 10:34:56.000000000 -0800
> +++ linux-2.6/mm/page_alloc.c	2006-03-10 12:55:02.000000000 -0800
> @@ -599,7 +599,6 @@ void drain_node_pages(int nodeid)
>  	int i, z;
>  	unsigned long flags;
>  
> -	local_irq_save(flags);
>  	for (z = 0; z < MAX_NR_ZONES; z++) {
>  		struct zone *zone = NODE_DATA(nodeid)->node_zones + z;
>  		struct per_cpu_pageset *pset;
> @@ -609,11 +608,14 @@ void drain_node_pages(int nodeid)
>  			struct per_cpu_pages *pcp;
>  
>  			pcp = &pset->pcp[i];
> -			free_pages_bulk(zone, pcp->count, &pcp->list, 0);
> -			pcp->count = 0;
> +			if (pcp->count) {
> +				local_irq_save(flags);
> +				free_pages_bulk(zone, pcp->count, &pcp->list, 0);
> +				pcp->count = 0;
> +				local_irq_restore(flags);
> +			}
>  		}
>  	}

This can cause us to run smp_processor_id() with preempt_count==0 and local
irqs enabled.  This will a) cause nasty runtime warnings and b) possibly go
bad if preemption causes this thread to hop CPUs.

But we've had that problem for a little while, because next_reap_node() is
calling __get_cpu_var() from preemptible code too.

But I _think_ we're OK for now because these functions are only ever called
from pinned-to-cpu kernel threads.

Please test all this with CONFIG_PREEMPT_DEBUG, confirm that it's OK.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: drain_node_pages: interrupt latency reduction / optimization

[Christoph Lameter]

On Fri, 10 Mar 2006, Andrew Morton wrote:

> But I _think_ we're OK for now because these functions are only ever called
> from pinned-to-cpu kernel threads.

Right. This function must be called that way otherwise it is useless. We 
should use raw_smp_processor_id().

> Please test all this with CONFIG_PREEMPT_DEBUG, confirm that it's OK.

Will do that on Monday but if this would not stay on a processor then 
other things in the slab allocator would also break.

Here is a fixup adding some comments and raw_smp_processor_id()

Signed-off-by: Christoph Lameter <clameter@sgi.com>

Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c	2006-03-10 16:09:10.000000000 -0800
+++ linux-2.6/mm/page_alloc.c	2006-03-10 16:10:07.000000000 -0800
@@ -593,6 +593,8 @@ static int rmqueue_bulk(struct zone *zon
 /*
  * Called from the slab reaper to drain pagesets on a particular node that
  * belong to the currently executing processor.
+ * Note that this function must be called with the thread pinned to
+ * a processor.
  */
 void drain_node_pages(int nodeid)
 {
@@ -603,7 +605,7 @@ void drain_node_pages(int nodeid)
 		struct zone *zone = NODE_DATA(nodeid)->node_zones + z;
 		struct per_cpu_pageset *pset;
 
-		pset = zone_pcp(zone, smp_processor_id());
+		pset = zone_pcp(zone, raw_smp_processor_id());
 		for (i = 0; i < ARRAY_SIZE(pset->pcp); i++) {
 			struct per_cpu_pages *pcp;
 
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: drain_node_pages: interrupt latency reduction / optimization

[Andrew Morton]

Christoph Lameter <clameter@sgi.com> wrote:
>
> Here is a fixup adding some comments and raw_smp_processor_id()
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c	2006-03-10 16:09:10.000000000 -0800
> +++ linux-2.6/mm/page_alloc.c	2006-03-10 16:10:07.000000000 -0800
> @@ -593,6 +593,8 @@ static int rmqueue_bulk(struct zone *zon
>  /*
>   * Called from the slab reaper to drain pagesets on a particular node that
>   * belong to the currently executing processor.
> + * Note that this function must be called with the thread pinned to
> + * a processor.
>   */
>  void drain_node_pages(int nodeid)
>  {
> @@ -603,7 +605,7 @@ void drain_node_pages(int nodeid)
>  		struct zone *zone = NODE_DATA(nodeid)->node_zones + z;
>  		struct per_cpu_pageset *pset;
>  
> -		pset = zone_pcp(zone, smp_processor_id());
> +		pset = zone_pcp(zone, raw_smp_processor_id());
>  		for (i = 0; i < ARRAY_SIZE(pset->pcp); i++) {
>  			struct per_cpu_pages *pcp;

hm.  That replaces a runtime check with a comment.  If someone comes along
and reuses this function wrongly they'll have a nasty subtle bug.

IOW: it might be best to keep the smp_processor_id() check in there.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: drain_node_pages: interrupt latency reduction / optimization

[Christoph Lameter]

> > -		pset = zone_pcp(zone, smp_processor_id());
> > +		pset = zone_pcp(zone, raw_smp_processor_id());
> hm.  That replaces a runtime check with a comment.  If someone comes along
> and reuses this function wrongly they'll have a nasty subtle bug.
>
> IOW: it might be best to keep the smp_processor_id() check in there.

Ahh. The DEBUG_PREEMPT is much more intelligent now and checks for this in 
the right way.  I agree leave as is.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>