Re: [PATCH] Reduce TLB flushing during process migration

Re: [PATCH] Reduce TLB flushing during process migration

[Manfred Spraul]

Martin wrote:

>diff -Nru a/kernel/sched.c b/kernel/sched.c
>--- a/kernel/sched.c	Tue Feb 17 07:33:59 2004
>+++ b/kernel/sched.c	Tue Feb 17 07:33:59 2004
>@@ -25,6 +25,7 @@
> #include <linux/highmem.h>
> #include <linux/smp_lock.h>
> #include <asm/mmu_context.h>
>+#include <asm/tlbflush.h>
> #include <linux/interrupt.h>
> #include <linux/completion.h>
> #include <linux/kernel_stat.h>
>@@ -1135,6 +1136,14 @@
> 		task_rq_unlock(rq, &flags);
> 		wake_up_process(rq->migration_thread);
> 		wait_for_completion(&req.done);
>+
>+		/*
>+		 * we want a new context here. This eliminates TLB
>+		 * flushes on the cpus where the process executed prior to
>+		 * the migration.
>+		 */
>+		flush_tlb_mm(current->mm);
>+
>  
>
I think flush_tlb_mm() is the wrong function - e.g. for i386, it's a 
wasted flush, because i386 disconnects previous cpus from the tlb flush 
automatically.
And it's always the wrong thing if you've migrated one thread of a task 
that runs on multiple cpus. I think you need a new hook.

--
    Manfred

Re: [PATCH] Reduce TLB flushing during process migration

[David S. Miller]

On Tue, 17 Feb 2004 17:22:14 +0100
Manfred Spraul <manfred@colorfullife.com> wrote:

> >+		 * we want a new context here. This eliminates TLB
> >+		 * flushes on the cpus where the process executed prior to
> >+		 * the migration.
> >+		 */
> >+		flush_tlb_mm(current->mm);
 ...
> I think flush_tlb_mm() is the wrong function - e.g. for i386, it's a 
> wasted flush, because i386 disconnects previous cpus from the tlb flush 
> automatically.
> And it's always the wrong thing if you've migrated one thread of a task 
> that runs on multiple cpus. I think you need a new hook.

Yes, you're probably right.  Just name it tlb_migrate_prepare(mm) or
something like that.

I think most if not all non-x86 platforms will define this straight
to flush_tlb_mm().

Re: [PATCH] Reduce TLB flushing during process migration

[Martin Hicks]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]



On Tue, Feb 17, 2004 at 10:08:52AM -0800, David S. Miller wrote:
> On Tue, 17 Feb 2004 17:22:14 +0100
> Manfred Spraul <manfred@colorfullife.com> wrote:
> 
> > >+		 * we want a new context here. This eliminates TLB
> > >+		 * flushes on the cpus where the process executed prior to
> > >+		 * the migration.
> > >+		 */
> > >+		flush_tlb_mm(current->mm);
>  ...
> > I think flush_tlb_mm() is the wrong function - e.g. for i386, it's a 
> > wasted flush, because i386 disconnects previous cpus from the tlb flush 
> > automatically.
> > And it's always the wrong thing if you've migrated one thread of a task 
> > that runs on multiple cpus. I think you need a new hook.
> 
> Yes, you're probably right.  Just name it tlb_migrate_prepare(mm) or
> something like that.
> 
> I think most if not all non-x86 platforms will define this straight
> to flush_tlb_mm().

Here's an updated patch that creates the new wrapper.  I only changed
ia64 to call flush_tlb_mm() because I'm a little wary about assuming
that all non-x86 arches want this.

Boot tested on ia64, compile tested on x86.

mh

-- 
Martin Hicks                Wild Open Source Inc.
mort@wildopensource.com     613-266-2296

[-- Attachment #2: process-migration-2.patch --]
[-- Type: text/plain, Size: 2192 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1680  -> 1.1681 
#	include/asm-generic/tlb.h	1.20    -> 1.21   
#	include/asm-ia64/tlb.h	1.18    -> 1.19   
#	      kernel/sched.c	1.240   -> 1.241  
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/02/17	mort@tomahawk.engr.sgi.com	1.1681
# Add tlb_migrate_prepare().  Most arches will define this to tlb_flush_mm().
# x86 is an exception.  For now make the default to do nothing.  Let each
# arch define tlb_migrate_prepare appropriately.
# --------------------------------------------
#
diff -Nru a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
--- a/include/asm-generic/tlb.h	Tue Feb 17 11:31:56 2004
+++ b/include/asm-generic/tlb.h	Tue Feb 17 11:31:56 2004
@@ -146,4 +146,6 @@
 		__pmd_free_tlb(tlb, pmdp);			\
 	} while (0)
 
+#define tlb_migrate_prepare(mm)
+
 #endif /* _ASM_GENERIC__TLB_H */
diff -Nru a/include/asm-ia64/tlb.h b/include/asm-ia64/tlb.h
--- a/include/asm-ia64/tlb.h	Tue Feb 17 11:31:56 2004
+++ b/include/asm-ia64/tlb.h	Tue Feb 17 11:31:56 2004
@@ -210,6 +210,8 @@
 	tlb->end_addr = address + PAGE_SIZE;
 }
 
+#define tlb_migrate_prepare(mm) flush_tlb_mm(mm)
+
 #define tlb_start_vma(tlb, vma)			do { } while (0)
 #define tlb_end_vma(tlb, vma)			do { } while (0)
 
diff -Nru a/kernel/sched.c b/kernel/sched.c
--- a/kernel/sched.c	Tue Feb 17 11:31:56 2004
+++ b/kernel/sched.c	Tue Feb 17 11:31:56 2004
@@ -25,6 +25,7 @@
 #include <linux/highmem.h>
 #include <linux/smp_lock.h>
 #include <asm/mmu_context.h>
+#include <asm/tlb.h>
 #include <linux/interrupt.h>
 #include <linux/completion.h>
 #include <linux/kernel_stat.h>
@@ -1135,6 +1136,14 @@
 		task_rq_unlock(rq, &flags);
 		wake_up_process(rq->migration_thread);
 		wait_for_completion(&req.done);
+
+		/*
+		 * we want a new context here. This eliminates TLB
+		 * flushes on the cpus where the process executed prior to
+		 * the migration.
+		 */
+		tlb_migrate_prepare(current->mm);
+
 		return;
 	}
 out:

Re: [PATCH] Reduce TLB flushing during process migration

[David S. Miller]

On Tue, 17 Feb 2004 15:05:58 -0500
Martin Hicks <mort@wildopensource.com> wrote:

> Here's an updated patch that creates the new wrapper.  I only changed
> ia64 to call flush_tlb_mm() because I'm a little wary about assuming
> that all non-x86 arches want this.

Please use "do { } while (0)" for the NOP define of the macro else
we'll end up with empty-statement warnings from the compiler.

Re: [PATCH] Reduce TLB flushing during process migration

[Martin Hicks]

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]



On Tue, Feb 17, 2004 at 12:08:28PM -0800, David S. Miller wrote:
> On Tue, 17 Feb 2004 15:05:58 -0500
> Martin Hicks <mort@wildopensource.com> wrote:
> 
> > Here's an updated patch that creates the new wrapper.  I only changed
> > ia64 to call flush_tlb_mm() because I'm a little wary about assuming
> > that all non-x86 arches want this.
> 
> Please use "do { } while (0)" for the NOP define of the macro else
> we'll end up with empty-statement warnings from the compiler.

Certainly.

Andrew:  Are you willing to pickup this change?

mh

-- 
Martin Hicks                Wild Open Source Inc.
mort@wildopensource.com     613-266-2296

[-- Attachment #2: process-migration-3.patch --]
[-- Type: text/plain, Size: 2208 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1680  -> 1.1681 
#	include/asm-generic/tlb.h	1.20    -> 1.22   
#	include/asm-ia64/tlb.h	1.18    -> 1.19   
#	      kernel/sched.c	1.240   -> 1.241  
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/02/17	mort@tomahawk.engr.sgi.com	1.1681
# Add tlb_migrate_prepare().  Most arches will define this to tlb_flush_mm().
# x86 is an exception.  For now make the default to do nothing.  Let each
# arch define tlb_migrate_prepare appropriately.
# --------------------------------------------
#
diff -Nru a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
--- a/include/asm-generic/tlb.h	Tue Feb 17 12:32:18 2004
+++ b/include/asm-generic/tlb.h	Tue Feb 17 12:32:18 2004
@@ -146,4 +146,6 @@
 		__pmd_free_tlb(tlb, pmdp);			\
 	} while (0)
 
+#define tlb_migrate_prepare(mm) do { } while(0)
+
 #endif /* _ASM_GENERIC__TLB_H */
diff -Nru a/include/asm-ia64/tlb.h b/include/asm-ia64/tlb.h
--- a/include/asm-ia64/tlb.h	Tue Feb 17 12:32:18 2004
+++ b/include/asm-ia64/tlb.h	Tue Feb 17 12:32:18 2004
@@ -210,6 +210,8 @@
 	tlb->end_addr = address + PAGE_SIZE;
 }
 
+#define tlb_migrate_prepare(mm) flush_tlb_mm(mm)
+
 #define tlb_start_vma(tlb, vma)			do { } while (0)
 #define tlb_end_vma(tlb, vma)			do { } while (0)
 
diff -Nru a/kernel/sched.c b/kernel/sched.c
--- a/kernel/sched.c	Tue Feb 17 12:32:18 2004
+++ b/kernel/sched.c	Tue Feb 17 12:32:18 2004
@@ -25,6 +25,7 @@
 #include <linux/highmem.h>
 #include <linux/smp_lock.h>
 #include <asm/mmu_context.h>
+#include <asm/tlb.h>
 #include <linux/interrupt.h>
 #include <linux/completion.h>
 #include <linux/kernel_stat.h>
@@ -1135,6 +1136,14 @@
 		task_rq_unlock(rq, &flags);
 		wake_up_process(rq->migration_thread);
 		wait_for_completion(&req.done);
+
+		/*
+		 * we want a new context here. This eliminates TLB
+		 * flushes on the cpus where the process executed prior to
+		 * the migration.
+		 */
+		tlb_migrate_prepare(current->mm);
+
 		return;
 	}
 out:

[PATCH] Reduce TLB flushing during process migration

[Martin Hicks]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]


Hi Andrew,

Another optimization patch from Jack Steiner, intended to reduce TLB
flushes during process migration.

CC'ed Rusty because this patch is applied on top of his cpuhotplug code.

This patch was generated against -rc4 with the rc3-mm1 patch merged on
top of it.

mh

-- 
Martin Hicks                Wild Open Source Inc.
mort@wildopensource.com     613-266-2296

[-- Attachment #2: process-migration.patch --]
[-- Type: text/plain, Size: 1225 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1680  -> 1.1681 
#	      kernel/sched.c	1.240   -> 1.241  
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/02/17	mort@tomahawk.engr.sgi.com	1.1681
# Process migration optimization.
# --------------------------------------------
#
diff -Nru a/kernel/sched.c b/kernel/sched.c
--- a/kernel/sched.c	Tue Feb 17 07:33:59 2004
+++ b/kernel/sched.c	Tue Feb 17 07:33:59 2004
@@ -25,6 +25,7 @@
 #include <linux/highmem.h>
 #include <linux/smp_lock.h>
 #include <asm/mmu_context.h>
+#include <asm/tlbflush.h>
 #include <linux/interrupt.h>
 #include <linux/completion.h>
 #include <linux/kernel_stat.h>
@@ -1135,6 +1136,14 @@
 		task_rq_unlock(rq, &flags);
 		wake_up_process(rq->migration_thread);
 		wait_for_completion(&req.done);
+
+		/*
+		 * we want a new context here. This eliminates TLB
+		 * flushes on the cpus where the process executed prior to
+		 * the migration.
+		 */
+		flush_tlb_mm(current->mm);
+
 		return;
 	}
 out:

Re: [PATCH] Reduce TLB flushing during process migration

[David S. Miller]

On Tue, 17 Feb 2004 10:49:26 -0500
Martin Hicks <mort@wildopensource.com> wrote:

> Another optimization patch from Jack Steiner, intended to reduce TLB
> flushes during process migration.

I like this change a lot.

Re: [PATCH] Reduce TLB flushing during process migration

[Andrew Morton]

Martin Hicks <mort@wildopensource.com> wrote:
>
> Another optimization patch from Jack Steiner, intended to reduce TLB
> flushes during process migration.

This patch is only applicable to CONFIG_NUMA.  Wouldn't SMP systems benefit
from the same treatment?

And does this optimisation come with any benchmark results?

Thanks.

Re: [PATCH] Reduce TLB flushing during process migration

[Martin Hicks]

On Tue, Feb 17, 2004 at 12:50:38PM -0800, Andrew Morton wrote:
> Martin Hicks <mort@wildopensource.com> wrote:
> >
> > Another optimization patch from Jack Steiner, intended to reduce TLB
> > flushes during process migration.
> 
> This patch is only applicable to CONFIG_NUMA.  Wouldn't SMP systems benefit
> from the same treatment?
> 
> And does this optimisation come with any benchmark results?

Here's some figures from Jack:

---

I dont have a benchmark for JUST the scheduler change, but the entire
node history + sched change is:

        nwchem on a 128p showed:

          Before:
            siosi7.sale.56-shm1cs: Time after atomic energies summed:   140.2
            siosi7.sale.120-shm1cs: Time after atomic energies summed:   306.9

          After:
            56p  = Time after atomic energies summed:    99.3
            120p = Time after atomic energies summed:   110.4

          Almost 3X improvement on 120p.

I dont recall how much was due to the sched fix, but I remember that it was
significant.

Note that the amount of improvement is highly platform specific.


---

mh

-- 
Martin Hicks                Wild Open Source Inc.
mort@wildopensource.com     613-266-2296

Re: [PATCH] Reduce TLB flushing during process migration

[Rusty Russell]

In message <20040217154926.GI12142@localhost> you write:
> CC'ed Rusty because this patch is applied on top of his cpuhotplug code.

Actually, the #ifdef around that function is wrong:

#if defined(CONFIG_NUMA) || defined(CONFIG_HOTPLUG_CPU)
/* If dest_cpu is allowed for this process, migrate us to it. */
void migrate_to_cpu(int dest_cpu)

We don't use this for HOTPLUG_CPU, but we call __migrate_task()
directly.

ie. #ifdef CONFIG_NUMA is sufficient.

I do think this is an obviously good idea though.
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.