[PATCH] Reduce TLB flushing during process migration

[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.

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:

Re: [PATCH] Reduce TLB flushing during process migration

[Jack Steiner]

On Wed, May 12, 2004 at 03:08:36PM -0700, Andrew Morton wrote:
> David Mosberger <davidm@napali.hpl.hp.com> wrote:
> >
> > Please remove it until a proper solution exists.
> 
> I'll submit this on Friday, when bugalugs returns.
> 

Here is an updated patch for flushing the TLB on SN2 after a process
migration. The patch is based on the latest "//linux.bkbits.net/linux-2.5"
bitkeeper tree.

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

--- linuxbase/kernel/sched.c	2004-05-21 20:47:33.000000000 -0500
+++ linux/kernel/sched.c	2004-05-21 22:42:39.000000000 -0500
@@ -40,6 +40,7 @@
 #include <linux/cpu.h>
 #include <linux/percpu.h>
 #include <linux/kthread.h>
+#include <asm/tlb.h>
 
 #ifdef CONFIG_NUMA
 #define cpu_to_node_mask(cpu) node_to_cpumask(cpu_to_node(cpu))
@@ -1311,6 +1312,7 @@
 		wake_up_process(mt);
 		put_task_struct(mt);
 		wait_for_completion(&req.done);
+		tlb_migrate_finish(p->mm);
 		return;
 	}
 out:
@@ -3317,6 +3319,7 @@
 		task_rq_unlock(rq, &flags);
 		wake_up_process(rq->migration_thread);
 		wait_for_completion(&req.done);
+		tlb_migrate_finish(p->mm);
 		return 0;
 	}
 out:
--- linuxbase/arch/ia64/sn/kernel/sn2/sn2_smp.c	2004-05-21 20:47:20.000000000 -0500
+++ linux/arch/ia64/sn/kernel/sn2/sn2_smp.c	2004-05-21 20:48:25.000000000 -0500
@@ -27,6 +27,7 @@
 #include <asm/delay.h>
 #include <asm/io.h>
 #include <asm/smp.h>
+#include <asm/tlb.h>
 #include <asm/numa.h>
 #include <asm/bitops.h>
 #include <asm/hw_irq.h>
@@ -60,6 +61,13 @@
 }
 
 
+void
+sn_tlb_migrate_finish(struct mm_struct *mm)
+{
+	if (mm = current->mm)
+		flush_tlb_mm(mm);
+}
+
 
 /**
  * sn2_global_tlb_purge - globally purge translation cache of virtual address range
@@ -114,6 +122,13 @@
 		return;
 	}
 
+	if (atomic_read(&mm->mm_users) = 1) {
+		flush_tlb_mm(mm);
+		preempt_enable();
+		return;
+	}
+
+
 	nix = 0;
 	for (cnode=find_first_bit(&nodes_flushed, NR_NODES); cnode < NR_NODES; 
 			cnode=find_next_bit(&nodes_flushed, NR_NODES, ++cnode))
--- linuxbase/arch/ia64/kernel/machvec.c	2004-05-21 20:47:20.000000000 -0500
+++ linux/arch/ia64/kernel/machvec.c	2004-05-21 20:48:25.000000000 -0500
@@ -62,6 +62,12 @@
 EXPORT_SYMBOL(machvec_timer_interrupt);
 
 void
+machvec_tlb_migrate_finish (struct mm_struct *mm)
+{
+}
+EXPORT_SYMBOL(machvec_tlb_migrate_finish);
+
+void
 machvec_dma_sync_single (struct device *hwdev, dma_addr_t dma_handle, size_t size, int dir)
 {
 	mb();
--- linuxbase/include/asm-generic/tlb.h	2004-05-21 20:47:28.000000000 -0500
+++ linux/include/asm-generic/tlb.h	2004-05-21 20:48:25.000000000 -0500
@@ -146,4 +146,6 @@
 		__pmd_free_tlb(tlb, pmdp);			\
 	} while (0)
 
+#define tlb_migrate_finish(mm) flush_tlb_mm(mm)
+
 #endif /* _ASM_GENERIC__TLB_H */
--- linuxbase/include/asm-ia64/tlb.h	2004-05-21 20:47:29.000000000 -0500
+++ linux/include/asm-ia64/tlb.h	2004-05-21 20:48:25.000000000 -0500
@@ -44,6 +44,7 @@
 #include <asm/pgalloc.h>
 #include <asm/processor.h>
 #include <asm/tlbflush.h>
+#include <asm/machvec.h>
 
 #ifdef CONFIG_SMP
 # define FREE_PTE_NR		2048
@@ -211,6 +212,8 @@
 	tlb->end_addr = address + PAGE_SIZE;
 }
 
+#define tlb_migrate_finish(mm)	platform_tlb_migrate_finish(mm)
+
 #define tlb_start_vma(tlb, vma)			do { } while (0)
 #define tlb_end_vma(tlb, vma)			do { } while (0)
 
--- linuxbase/include/asm-ia64/machvec.h	2004-05-21 20:47:29.000000000 -0500
+++ linux/include/asm-ia64/machvec.h	2004-05-21 20:48:25.000000000 -0500
@@ -19,6 +19,7 @@
 struct scatterlist;
 struct irq_desc;
 struct page;
+struct mm_struct;
 
 typedef void ia64_mv_setup_t (char **);
 typedef void ia64_mv_cpu_init_t (void);
@@ -26,6 +27,7 @@
 typedef void ia64_mv_send_ipi_t (int, int, int, int);
 typedef void ia64_mv_timer_interrupt_t (int, void *, struct pt_regs *);
 typedef void ia64_mv_global_tlb_purge_t (unsigned long, unsigned long, unsigned long);
+typedef void ia64_mv_tlb_migrate_finish_t (struct mm_struct *);
 typedef struct irq_desc *ia64_mv_irq_desc (unsigned int);
 typedef u8 ia64_mv_irq_to_vector (u8);
 typedef unsigned int ia64_mv_local_vector_to_irq (u8 vector);
@@ -72,6 +74,7 @@
 extern void machvec_noop (void);
 extern void machvec_setup (char **);
 extern void machvec_timer_interrupt (int, void *, struct pt_regs *);
+extern void machvec_tlb_migrate_finish (struct mm_struct *);
 extern void machvec_dma_sync_single (struct device *, dma_addr_t, size_t, int);
 extern void machvec_dma_sync_sg (struct device *, struct scatterlist *, int, int);
 
@@ -95,6 +98,7 @@
 #  define platform_send_ipi	ia64_mv.send_ipi
 #  define platform_timer_interrupt	ia64_mv.timer_interrupt
 #  define platform_global_tlb_purge	ia64_mv.global_tlb_purge
+#  define platform_tlb_migrate_finish	ia64_mv.tlb_migrate_finish
 #  define platform_dma_init		ia64_mv.dma_init
 #  define platform_dma_alloc_coherent	ia64_mv.dma_alloc_coherent
 #  define platform_dma_free_coherent	ia64_mv.dma_free_coherent
@@ -140,6 +144,7 @@
 	ia64_mv_send_ipi_t *send_ipi;
 	ia64_mv_timer_interrupt_t *timer_interrupt;
 	ia64_mv_global_tlb_purge_t *global_tlb_purge;
+	ia64_mv_tlb_migrate_finish_t *tlb_migrate_finish;
 	ia64_mv_dma_init *dma_init;
 	ia64_mv_dma_alloc_coherent *dma_alloc_coherent;
 	ia64_mv_dma_free_coherent *dma_free_coherent;
@@ -181,6 +186,7 @@
 	platform_send_ipi,			\
 	platform_timer_interrupt,		\
 	platform_global_tlb_purge,		\
+	platform_tlb_migrate_finish,		\
 	platform_dma_init,			\
 	platform_dma_alloc_coherent,		\
 	platform_dma_free_coherent,		\
@@ -260,6 +266,9 @@
 #ifndef platform_global_tlb_purge
 # define platform_global_tlb_purge	ia64_global_tlb_purge /* default to architected version */
 #endif
+#ifndef platform_tlb_migrate_finish
+# define platform_tlb_migrate_finish machvec_tlb_migrate_finish
+#endif
 #ifndef platform_dma_init
 # define platform_dma_init		swiotlb_init
 #endif
--- linuxbase/include/asm-ia64/machvec_sn2.h	2004-05-21 20:47:29.000000000 -0500
+++ linux/include/asm-ia64/machvec_sn2.h	2004-05-21 20:48:25.000000000 -0500
@@ -39,6 +39,7 @@
 extern ia64_mv_send_ipi_t sn2_send_IPI;
 extern ia64_mv_timer_interrupt_t sn_timer_interrupt;
 extern ia64_mv_global_tlb_purge_t sn2_global_tlb_purge;
+extern ia64_mv_tlb_migrate_finish_t	sn_tlb_migrate_finish;
 extern ia64_mv_irq_desc sn_irq_desc;
 extern ia64_mv_irq_to_vector sn_irq_to_vector;
 extern ia64_mv_local_vector_to_irq sn_local_vector_to_irq;
@@ -83,6 +84,7 @@
 #define platform_send_ipi		sn2_send_IPI
 #define platform_timer_interrupt	sn_timer_interrupt
 #define platform_global_tlb_purge       sn2_global_tlb_purge
+#define platform_tlb_migrate_finish	sn_tlb_migrate_finish
 #define platform_pci_fixup		sn_pci_fixup
 #define platform_inb			__sn_inb
 #define platform_inw			__sn_inw
-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

Re: [PATCH] Reduce TLB flushing during process migration

[Andrew Morton]

Jack Steiner <steiner@sgi.com> wrote:
>
> Here is an updated patch for flushing the TLB on SN2 after a process
>  migration. The patch is based on the latest "//linux.bkbits.net/linux-2.5"
>  bitkeeper tree.

Instant oops on ia32 because p->mm is NULL for kernel threads.  You might
want to check that on ia64 too - it's not clear to me why it works there.

--- 25/include/asm-generic/tlb.h~reduce-tlb-flushing-during-process-migration-oops-fix	2004-05-24 22:20:30.018626600 -0700
+++ 25-akpm/include/asm-generic/tlb.h	2004-05-24 22:21:12.621150032 -0700
@@ -147,6 +147,10 @@ static inline void tlb_remove_page(struc
 		__pmd_free_tlb(tlb, pmdp);			\
 	} while (0)
 
-#define tlb_migrate_finish(mm) flush_tlb_mm(mm)
+#define tlb_migrate_finish(mm)					\
+	do {							\
+		if (likely(mm))					\
+			flush_tlb_mm(mm);			\
+	} while (0)
 
 #endif /* _ASM_GENERIC__TLB_H */


But I don't understand why your patch adds two flush_tlb_mm() calls into
sched.c on ia32 where there previously was no such call.

Re: [PATCH] Reduce TLB flushing during process migration

[Jack Steiner]

On Mon, May 24, 2004 at 10:26:12PM -0700, Andrew Morton wrote:
> Jack Steiner <steiner@sgi.com> wrote:
> >
> > Here is an updated patch for flushing the TLB on SN2 after a process
> >  migration. The patch is based on the latest "//linux.bkbits.net/linux-2.5"
> >  bitkeeper tree.
> 
> Instant oops on ia32 because p->mm is NULL for kernel threads.  You might
> want to check that on ia64 too - it's not clear to me why it works there.

                                                                                                                                     
On IA64, it is ok to call flush_tlb_mm() with a NULL mm. I not sure if
this is *suppose* to be part of the API for flush_tlb_mm() but it is
allowed on IA64.  flush_tlb_mm() has an explicit check for NULL mm & simply 
returns.
                                                                                                                                     
                                                                                                                                     
                                                                                                                                     
> 
> --- 25/include/asm-generic/tlb.h~reduce-tlb-flushing-during-process-migration-oops-fix	2004-05-24 22:20:30.018626600 -0700
> +++ 25-akpm/include/asm-generic/tlb.h	2004-05-24 22:21:12.621150032 -0700
> @@ -147,6 +147,10 @@ static inline void tlb_remove_page(struc
>  		__pmd_free_tlb(tlb, pmdp);			\
>  	} while (0)
>  
> -#define tlb_migrate_finish(mm) flush_tlb_mm(mm)
> +#define tlb_migrate_finish(mm)					\
> +	do {							\
> +		if (likely(mm))					\
> +			flush_tlb_mm(mm);			\
> +	} while (0)
>  
>  #endif /* _ASM_GENERIC__TLB_H */
> 
> 
> But I don't understand why your patch adds two flush_tlb_mm() calls into
> sched.c on ia32 where there previously was no such call.

I thought I had a good reason. Let me check & make sure I didnt overlook
something. I'll get back to you.....

-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

Re: [PATCH] Reduce TLB flushing during process migration

[Jack Steiner]

> > 
> > But I don't understand why your patch adds two flush_tlb_mm() calls into
> > sched.c on ia32 where there previously was no such call.
> 
> I thought I had a good reason. Let me check & make sure I didnt overlook
> something. I'll get back to you.....
> 

You are right - the second call is not needed. I wanted to clear
cpu_vm_mask after an exec migration. That is not required because
exec calls init_new_context() shortly after the migrate anyway.



However, I found an unrelated issue.
It looks like the call to sched_balance_exec() from do_execve()
is in the wrong spot.


	int do_execve(char * filename,
        	char __user *__user *argv,
        	char __user *__user *envp,
        	struct pt_regs * regs)
	{
        	struct linux_binprm bprm;
        	struct file *file;
        	int retval;
        	int i;
	
        	sched_balance_exec();
	
        	file = open_exec(filename);
	
        	retval = PTR_ERR(file);
        	if (IS_ERR(file))
                	return retval;
	
        	bprm.p = PAGE_SIZE*MAX_ARG_PAGES-sizeof(void *);
		...



The code calls sched_balance_exec() before determining whether "filename"
actually exists. 

In many cases, users have several entries in $PATH.
If a full path name is not specified on the 'exec" call, the library 
code iterates thru the files in the PATH list until it finds the program. This
can result is numerous migrations of the parent process before
the program is actually found.


Shouldn't the call to sched_balance_exec() be moved to just before "bprm...."
or am I overlooking something?




--- linuxbase/fs/exec.c	2004-05-21 20:47:27.000000000 -0500
+++ linux/fs/exec.c	2004-06-01 10:46:35.000000000 -0500
@@ -1083,14 +1083,14 @@
 	int retval;
 	int i;
 
-	sched_balance_exec();
-
 	file = open_exec(filename);
 
 	retval = PTR_ERR(file);
 	if (IS_ERR(file))
 		return retval;
 
+	sched_balance_exec();
+
 	bprm.p = PAGE_SIZE*MAX_ARG_PAGES-sizeof(void *);
 	memset(bprm.page, 0, MAX_ARG_PAGES*sizeof(bprm.page[0]));
 



-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.