numa/core patches

Hi all,

Here's a re-post of the NUMA scheduling and migration improvement patches that we are working on. These include techniques from AutoNUMA and the sched/numa tree and form a unified basis - it has got all the bits that look good and mergeable.

With these patches applied, the mbind system calls expand to new modes of lazy-migration binding, and if the CONFIG_SCHED_NUMA=y .config option is enabled the scheduler will automatically sample the working set of tasks via page faults. Based on that information the scheduler then tries to balance smartly, put tasks on a home node and migrate CPU work and memory on the same node.

They are functional in their current state and have had testing on a variety of x86 NUMA hardware.

These patches will continue their life in tip:numa/core and unless there are major showstoppers they are intended for the v3.8 merge window.

We believe that they provide a solid basis for future work.

Please review .. once again and holler if you see anything funny! :-)

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

Its a bit awkward but it was the least painful means of modifying the queue selection. Used in a later patch to conditionally use a random queue.

---
 kernel/sched/fair.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Index: tip/kernel/sched/fair.c
===================================================================
@@ -3063,6 +3063,9 @@ struct lb_env {
 	unsigned int		loop;
 	unsigned int		loop_break;
 	unsigned int		loop_max;
+
+	struct rq *		(*find_busiest_queue)(struct lb_env *,
+						      struct sched_group *);
 };
 
 /*

@@ -4236,13 +4239,14 @@ static int load_balance(int this_cpu, st
 	struct cpumask *cpus = __get_cpu_var(load_balance_tmpmask);
 
 	struct lb_env env = {
-		.sd		= sd,
-		.dst_cpu	= this_cpu,
-		.dst_rq		= this_rq,
-		.dst_grpmask    = sched_group_cpus(sd->groups),
-		.idle		= idle,
-		.loop_break	= sched_nr_migrate_break,
-		.cpus		= cpus,
+		.sd		    = sd,
+		.dst_cpu	    = this_cpu,
+		.dst_rq		    = this_rq,
+		.dst_grpmask        = sched_group_cpus(sd->groups),
+		.idle		    = idle,
+		.loop_break	    = sched_nr_migrate_break,
+		.cpus		    = cpus,
+		.find_busiest_queue = find_busiest_queue,
 	};
 
 	cpumask_copy(cpus, cpu_active_mask);

@@ -4261,7 +4265,7 @@ redo:
 		goto out_balanced;
 	}
 
-	busiest = find_busiest_queue(&env, group);
+	busiest = env.find_busiest_queue(&env, group);
 	if (!busiest) {
 		schedstat_inc(sd, lb_nobusyq[idle]);
 		goto out_balanced;

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

This is probably a first: formal description of a complex high-level computing problem, within the kernel source.

Rik van Riel <[email protected]>

---
 Documentation/scheduler/numa-problem.txt |  230 +++++++++++++++++++++++++++++++
 1 file changed, 230 insertions(+)
 create mode 100644 Documentation/scheduler/numa-problem.txt

Index: tip/Documentation/scheduler/numa-problem.txt
===================================================================
@@ -0,0 +1,230 @@
+
+
+Effective NUMA scheduling problem statement, described formally:
+
+ * minimize interconnect traffic
+
+For each task 't_i' we have memory, this memory can be spread over multiple
+physical nodes, let us denote this as: 'p_i,k', the memory task 't_i' has on
+node 'k' in [pages].  
+
+If a task shares memory with another task let us denote this as:
+'s_i,k', the memory shared between tasks including 't_i' residing on node
+'k'.
+
+Let 'M' be the distribution that governs all 'p' and 's', ie. the page placement.
+
+Similarly, lets define 'fp_i,k' and 'fs_i,k' resp. as the (average) usage
+frequency over those memory regions [1/s] such that the product gives an
+(average) bandwidth 'bp' and 'bs' in [pages/s].
+
+(note: multiple tasks sharing memory naturally avoid duplicat accounting
+       because each task will have its own access frequency 'fs')
+
+(pjt: I think this frequency is more numerically consistent if you explicitly 
+      restrict p/s above to be the working-set. (It also makes explicit the 
+      requirement for <C0,M0> to change about a change in the working set.)
+
+      Doing this does have the nice property that it lets you use your frequency
+      measurement as a weak-ordering for the benefit a task would receive when
+      we can't fit everything.
+
+      e.g. task1 has working set 10mb, f=90%
+           task2 has working set 90mb, f=10%
+
+      Both are using 9mb/s of bandwidth, but we'd expect a much larger benefit
+      from task1 being on the right node than task2. )
+
+Let 'C' map every task 't_i' to a cpu 'c_i' and its corresponding node 'n_i':
+
+  C: t_i -> {c_i, n_i}
+
+This gives us the total interconnect traffic between nodes 'k' and 'l',
+'T_k,l', as:
+
+  T_k,l = \Sum_i bp_i,l + bs_i,l + \Sum bp_j,k + bs_j,k where n_i == k, n_j == l
+
+And our goal is to obtain C0 and M0 such that:
+
+  T_k,l(C0, M0) =< T_k,l(C, M) for all C, M where k != l
+
+(note: we could introduce 'nc(k,l)' as the cost function of accessing memory
+       on node 'l' from node 'k', this would be useful for bigger NUMA systems
+
+ pjt: I agree nice to have, but intuition suggests diminishing returns on more
+      usual systems given factors like things like Haswell's enormous 35mb l3
+      cache and QPI being able to do a direct fetch.)
+
+(note: do we need a limit on the total memory per node?)
+
+
+ * fairness
+
+For each task 't_i' we have a weight 'w_i' (related to nice), and each cpu
+'c_n' has a compute capacity 'P_n', again, using our map 'C' we can formulate a
+load 'L_n':
+
+  L_n = 1/P_n * \Sum_i w_i for all c_i = n
+
+using that we can formulate a load difference between CPUs
+
+  L_n,m = | L_n - L_m |
+
+Which allows us to state the fairness goal like:
+
+  L_n,m(C0) =< L_n,m(C) for all C, n != m
+
+(pjt: It can also be usefully stated that, having converged at C0:
+
+   | L_n(C0) - L_m(C0) | <= 4/3 * | G_n( U(t_i, t_j) ) - G_m( U(t_i, t_j) ) |
+
+      Where G_n,m is the greedy partition of tasks between L_n and L_m. This is
+      the "worst" partition we should accept; but having it gives us a useful 
+      bound on how much we can reasonably adjust L_n/L_m at a Pareto point to 
+      favor T_n,m. )
+
+Together they give us the complete multi-objective optimization problem:
+
+  min_C,M [ L_n,m(C), T_k,l(C,M) ]
+
+
+
+Notes:
+
+ - the memory bandwidth problem is very much an inter-process problem, in
+   particular there is no such concept as a process in the above problem.
+
+ - the naive solution would completely prefer fairness over interconnect
+   traffic, the more complicated solution could pick another Pareto point using
+   an aggregate objective function such that we balance the loss of work
+   efficiency against the gain of running, we'd want to more or less suggest
+   there to be a fixed bound on the error from the Pareto line for any
+   such solution.
+
+References:
+
+  http://en.wikipedia.org/wiki/Mathematical_optimization
+  http://en.wikipedia.org/wiki/Multi-objective_optimization
+
+
+* warning, significant hand-waving ahead, improvements welcome *
+
+
+Partial solutions / approximations:
+
+ 1) have task node placement be a pure preference from the 'fairness' pov.
+
+This means we always prefer fairness over interconnect bandwidth. This reduces
+the problem to:
+
+  min_C,M [ T_k,l(C,M) ]
+
+ 2a) migrate memory towards 'n_i' (the task's node).
+
+This creates memory movement such that 'p_i,k for k != n_i' becomes 0 -- 
+provided 'n_i' stays stable enough and there's sufficient memory (looks like
+we might need memory limits for this).
+
+This does however not provide us with any 's_i' (shared) information. It does
+however remove 'M' since it defines memory placement in terms of task
+placement.
+
+XXX properties of this M vs a potential optimal
+
+ 2b) migrate memory towards 'n_i' using 2 samples.
+
+This separates pages into those that will migrate and those that will not due
+to the two samples not matching. We could consider the first to be of 'p_i'
+(private) and the second to be of 's_i' (shared).
+
+This interpretation can be motivated by the previously observed property that
+'p_i,k for k != n_i' should become 0 under sufficient memory, leaving only
+'s_i' (shared). (here we loose the need for memory limits again, since it
+becomes indistinguishable from shared).
+
+XXX include the statistical babble on double sampling somewhere near
+
+This reduces the problem further; we loose 'M' as per 2a, it further reduces
+the 'T_k,l' (interconnect traffic) term to only include shared (since per the
+above all private will be local):
+
+  T_k,l = \Sum_i bs_i,l for every n_i = k, l != k
+
+[ more or less matches the state of sched/numa and describes its remaining
+  problems and assumptions. It should work well for tasks without significant
+  shared memory usage between tasks. ]
+
+Possible future directions:
+
+Motivated by the form of 'T_k,l', try and obtain each term of the sum, so we
+can evaluate it;
+
+ 3a) add per-task per node counters
+
+At fault time, count the number of pages the task faults on for each node.
+This should give an approximation of 'p_i' for the local node and 's_i,k' for
+all remote nodes.
+
+While these numbers provide pages per scan, and so have the unit [pages/s] they
+don't count repeat access and thus aren't actually representable for our
+bandwidth numberes.
+
+ 3b) additional frequency term
+
+Additionally (or instead if it turns out we don't need the raw 'p' and 's' 
+numbers) we can approximate the repeat accesses by using the time since marking
+the pages as indication of the access frequency.
+
+Let 'I' be the interval of marking pages and 'e' the elapsed time since the
+last marking, then we could estimate the number of accesses 'a' as 'a = I / e'.
+If we then increment the node counters using 'a' instead of 1 we might get
+a better estimate of bandwidth terms.
+
+ 3c) additional averaging; can be applied on top of either a/b.
+
+[ Rik argues that decaying averages on 3a might be sufficient for bandwidth since
+  the decaying avg includes the old accesses and therefore has a measure of repeat
+  accesses.
+
+  Rik also argued that the sample frequency is too low to get accurate access
+  frequency measurements, I'm not entirely convinced, event at low sample 
+  frequencies the avg elapsed time 'e' over multiple samples should still
+  give us a fair approximation of the avg access frequency 'a'.
+
+  So doing both b&c has a fair chance of working and allowing us to distinguish
+  between important and less important memory accesses.
+
+  Experimentation has shown no benefit from the added frequency term so far. ]
+
+This will give us 'bp_i' and 'bs_i,k' so that we can approximately compute
+'T_k,l' Our optimization problem now reads:
+
+  min_C [ \Sum_i bs_i,l for every n_i = k, l != k ]
+
+And includes only shared terms, this makes sense since all task private memory
+will become local as per 2.
+
+This suggests that if there is significant shared memory, we should try and
+move towards it.
+
+ 4) move towards where 'most' memory is
+
+The simplest significance test is comparing the biggest shared 's_i,k' against
+the private 'p_i'. If we have more shared than private, move towards it.
+
+This effectively makes us move towards where most our memory is and forms a
+feed-back loop with 2. We migrate memory towards us and we migrate towards
+where 'most' memory is.
+
+(Note: even if there were two tasks fully trashing the same shared memory, it
+       is very rare for there to be an 50/50 split in memory, lacking a perfect
+       split, the small will move towards the larger. In case of the perfect
+       split, we'll tie-break towards the lower node number.)
+
+ 5) 'throttle' 4's node placement
+
+Since per 2b our 's_i,k' and 'p_i' require at least two scans to 'stabilize'
+and show representative numbers, we should limit node-migration to not be
+faster than this.
+
+ n) poke holes in previous that require more stuff and describe it.

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

Who does not love the smell of formal methods first thing in the morning?

You can use the computing award monies as a pension.

Mel Gorman
SUSE Labs

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

The only issue I have with this document is that it does not have any description of how the source code tries to solve the problem at hand.

A description of how the problem is solved will make the documentation useful to people trying to figure out why the NUMA code does what it does.

Of course, since we still do not know what sched-numa needs to do in order to match autonuma performance, that description would have to be updated later, anyway.

All rights reversed

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

We're going to play games with page-protections, ensure we don't lose them over a THP split.

Collapse seems to always allocate a new (huge) page which should already end up on the new target node so loosing protections there isn't a problem.

---
 arch/x86/include/asm/pgtable.h |    1 
 mm/huge_memory.c               |  105 +++++++++++++++++++----------------------
 2 files changed, 51 insertions(+), 55 deletions(-)

Index: tip/arch/x86/include/asm/pgtable.h
===================================================================
@@ -349,6 +349,7 @@ static inline pgprot_t pgprot_modify(pgp
 }
 
 #define pte_pgprot(x) __pgprot(pte_flags(x) & PTE_FLAGS_MASK)
+#define pmd_pgprot(x) __pgprot(pmd_val(x) & ~_HPAGE_CHG_MASK)
 
 #define canon_pgprot(p) __pgprot(massage_pgprot(p))
 

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

Why?

If PROT_NONE becomes a present pte, we lose samples. If a present pte becomes PROT_NONE, we get spurious faults and some sampling oddities. Both situations only apply when a THP is being split which implies distruption anyway (mprotect, page reclaim etc.) and neither seems that series. It seems premature at this point of the series and looks like it could have been dropped entirely.

                       entry = maybe_mkwrite(pte_mkdirty(entry), vma);
                       if (!pmd_write(*pmd))
                               entry = pte_wrprotect(entry);
                       else
                               BUG_ON(page_mapcount(page) != 1);

Mel Gorman
SUSE Labs

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

From: Rik van Riel <[email protected]>

We need pte_present to return true for _PAGE_PROTNONE pages, to indicate that the pte is associated with a page.

However, for TLB flushing purposes, we would like to know whether the pte points to an actually accessible page. This allows us to skip remote TLB flushes for pages that are not actually accessible.

---
 arch/x86/include/asm/pgtable.h |    6 ++++++
 include/asm-generic/pgtable.h  |    4 ++++
 2 files changed, 10 insertions(+)

Index: tip/arch/x86/include/asm/pgtable.h
===================================================================
@@ -408,6 +408,12 @@ static inline int pte_present(pte_t a)
 	return pte_flags(a) & (_PAGE_PRESENT | _PAGE_PROTNONE);
 }
 
+#define __HAVE_ARCH_PTE_ACCESSIBLE
+static inline int pte_accessible(pte_t a)
+{
+	return pte_flags(a) & _PAGE_PRESENT;
+}
+
 static inline int pte_hidden(pte_t pte)
 {
 	return pte_flags(pte) & _PAGE_HIDDEN;

Index: tip/include/asm-generic/pgtable.h
===================================================================
@@ -219,6 +219,10 @@ static inline int pmd_same(pmd_t pmd_a,
 #define move_pte(pte, prot, old_addr, new_addr)	(pte)
 #endif
 
+#ifndef __HAVE_ARCH_PTE_ACCESSIBLE
+#define pte_accessible(pte)		pte_present(pte)
+#endif
+
 #ifndef flush_tlb_fix_spurious_fault
 #define flush_tlb_fix_spurious_fault(vma, address) flush_tlb_page(vma, address)
 #endif

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

NAK NAK NAK.

   #define pte_accessible pte_accessible

   #ifndef pte_accessible

  #ifndef pte_accessible
    #define pte_accessible(pte) ((void)(pte),1)
  #endif

Yeah...

Indeed that's much safer and each arch can opt-in consciously instead of us offering a potentially unsafe optimization.

I have fixed it as per the updated patch below. Only very lightly tested.

Thanks,

Ingo

----------------------------->
Subject: x86/mm: Introduce pte_accessible()
From: Rik van Riel <[email protected]>
Date: Tue, 9 Oct 2012 15:31:12 +0200

We need pte_present to return true for _PAGE_PROTNONE pages, to indicate that the pte is associated with a page.

However, for TLB flushing purposes, we would like to know whether the pte points to an actually accessible page. This allows us to skip remote TLB flushes for pages that are not actually accessible.

Fill in this method for x86 and provide a safe (but slower) method on other architectures.

---
 arch/x86/include/asm/pgtable.h |    6 ++++++
 include/asm-generic/pgtable.h  |    4 ++++
 2 files changed, 10 insertions(+)

Index: tip/arch/x86/include/asm/pgtable.h
===================================================================
@@ -408,6 +408,12 @@ static inline int pte_present(pte_t a)
 	return pte_flags(a) & (_PAGE_PRESENT | _PAGE_PROTNONE);
 }
 
+#define pte_accessible pte_accessible
+static inline int pte_accessible(pte_t a)
+{
+	return pte_flags(a) & _PAGE_PRESENT;
+}
+
 static inline int pte_hidden(pte_t pte)
 {
 	return pte_flags(pte) & _PAGE_HIDDEN;

Index: tip/include/asm-generic/pgtable.h
===================================================================
@@ -219,6 +219,10 @@ static inline int pmd_same(pmd_t pmd_a,
 #define move_pte(pte, prot, old_addr, new_addr)	(pte)
 #endif
 
+#ifndef pte_accessible
+# define pte_accessible(pte)		((void)(pte),1)
+#endif
+
 #ifndef flush_tlb_fix_spurious_fault
 #define flush_tlb_fix_spurious_fault(vma, address) flush_tlb_page(vma, address)
 #endif

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

It feels like we are putting the cart before the horse to be taking TLB flushing optimisations into account this early in the series. That aside, what was wrong with the following patches?

autonuma: define _PAGE_NUMA
arch-dependant definition of a flag that happens to be PROT_NONE on x86 but could be anything at all really which would help portability

autonuma: pte_numa() and pmd_numa()
makes pte_present do what you want
adds pte_numa and pmd_numa which potentially could have been used instead of pte_accessible

autonuma: teach gup_fast about pmd_numa
sortof self-explanatory

and building on those? The arch-dependant nature of _PAGE_NUMA might have avoided Linus sending the childrens college fund to the swear jar and avoided this complaint;

===
because you have no idea if other architectures do

(a) the same trick as x86 does for PROT_NONE (I can already tell you from a quick grep that ia64, m32r, m68k and sh do it)
(b) might not perhaps be caching non-present pte's anyway
====

The "autonuma: define _PAGE_NUMA" happens to use PROT_NONE but as an implementation detail rather than by design and as a bonus point describes what it is doing. The "autonuma" part in the title is misleading, it's not autonuma-specific at all and could have been dropped or just renamed "numa:"

Mel Gorman
SUSE Labs

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

From: Rik van Riel <[email protected]>

If ptep_set_access_flags() is invoked to upgrade access permissions on a PTE, there is no security or data integrity reason to do a remote TLB flush.

Lazily letting another CPU incur a spurious page fault occasionally is (much!) cheaper than aggressively flushing everybody else's TLB.

---
 arch/x86/mm/pgtable.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Index: tip/arch/x86/mm/pgtable.c
===================================================================
@@ -306,11 +306,26 @@ int ptep_set_access_flags(struct vm_area
 			  pte_t entry, int dirty)
 {
 	int changed = !pte_same(*ptep, entry);
+	/*
+	 * If the page used to be inaccessible (_PAGE_PROTNONE), or
+	 * this call upgrades the access permissions on the same page,
+	 * it is safe to skip the remote TLB flush.
+	 */
+	bool flush_remote = false;
+	if (!pte_accessible(*ptep))
+		flush_remote = false;
+	else if (pte_pfn(*ptep) != pte_pfn(entry) ||
+			(pte_write(*ptep) && !pte_write(entry)) ||
+			(pte_exec(*ptep) && !pte_exec(entry)))
+		flush_remote = true;
 
 	if (changed && dirty) {
 		*ptep = entry;
 		pte_update_defer(vma->vm_mm, address, ptep);
-		flush_tlb_page(vma, address);
+		if (flush_remote)
+			flush_tlb_page(vma, address);
+		else
+			__flush_tlb_one(address);
 	}
 
 	return changed;

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

I agree it is pretty ugly. However, the above patch did get rid of a gigantic performance regression with Peter's code.

Doing unnecessary remote TLB flushes was costing about 90% performance with specjbb on a 4 node system.

However, if we can guarantee that ptep_set_access_flags is only ever called for pte permission _upgrades_, we can simply get rid of the remote TLB flush on x86, and skip the paranoia tests we are doing above.

Do we have that kind of guarantee?

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

Rik, *LOOK* at the code like I asked you to, instead of making excuses for it.

I'm not necessarily arguing with what the code tries to do. I'm arguing with the fact that the code is pure and utter *garbage*.

It has two major (and I mean *MAJOR*) problems, both of which individually should make you ashamed for ever posting that piece of shit:

The obvious-without-even-understanding-semantics problem:

- it's humongously stupidly written. It calculates that
'flush_remote' flag WHETHER IT GETS USED OR NOT.

Christ. I can kind of expect stuff like that in driver code etc,
but in VM routines?

Yes, the compiler may be smart enough to actually fix up the
idiocy. That doesn't make it less stupid.

The more-subtle-but-fundamental-problem:

- regardless of how stupidly written it is on a very superficial
level, it's even more stupid in a much more fundamental way.

That whole routine is explicitly written to be opportunistic. It is
*documented* to only set the access flags, so comparing anything else is stupid, wouldn't you say?

Documented where? It's actually explicitly documented in the pgtable-generic.c file which has the generic implementation of that thing. But it's implicitly documented both in the name of the function (do take another look) *and* in the actual implementation of the function.

Look at the code: it doesn't even always update the page tables AT ALL (and no, the return value does *not* reflect whether it updated it or not!)

Also, notice how we update the pte entry with a simple

    *ptep = entry;

statement, not with the usual expensive page table updates? The only thing that makes this safe is that we *only* do it with the exact same page frame number (anything else would be disastrously buggy on 32-bit PAE, for example). And we only ever do it with the dirty bit always set, because otherwise we might be silently dropping a concurrent hardware update of the dirty bit of the previous pte value on another CPU.

The latter requirement is why the x86 code does

    if (changed && dirty) {

while the generic code checks just "If (changed)" (and then uses the much more expensive set_pte_at() that has the proper dirty-bit guarantees, and generates atomic accesses, not to mention various virtualization crap).

In other words, everything that was added by that patch is PURE AND UTTER SHIT. And THAT is what I'm objecting to.

Guess what? If you want to optimize the function to not do remote TLB flushes, then just do that! None of the garbage. Just change the

    flush_tlb_page(vma, address);

line to

    __flush_tlb_one(address);

and it should damn well work. Because everything I see about "flush_remote" looks just wrong, wrong, wrong.

And if there really is some reason for that whole flush_remote braindamage, then we have much bigger problems, namely the fact that we've broken the documented semantics of that function, and we're doing various other things that are completely and utterly invalid unless the above semantics hold.

So that patch should be burned, and possibly used as an example of horribly crappy code for later generations. At no point should it be applied.

Linus

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

That may not even be needed. Apparently Intel chips automatically flush an entry from the TLB when it causes a page fault. I assume AMD chips do the same, because flush_tlb_fix_spurious_fault evaluates to nothing on x86.

Are there architectures where we do need to flush remote TLBs on upgrading the permissions on a PTE?

Because that is what the implementation in
pgtable-generic.c seems to be doing as well...

Want to just remove the TLB flush entirely and see if anything breaks in 3.8-rc1?

From reading the code again, it looks like things should indeed work ok.

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

Yes. It's not architected as far as I know, though. But I agree, it's possible - even likely - we could avoid TLB flushing entirely on x86.

If you want to try it, I would seriously suggest you do it as a separate commit though, just in case.

I *suspect* that whole TLB flush just magically became an SMP one without anybody ever really thinking about it.

So it's quite possible we could do this to the pgtable-generic.c code too. However, we don't actually have any generic way to do a local single-address flush (the __flush_tlb_one() thing is architecture-specific, although it exists on a few architectures). We'd need to add a local_flush_tlb_page(vma, address) function.

Alternatively, we could decide to use the "tlb_fix_spurious_fault()" thing in there. Possibly just do it unconditionally in the caller - or even just specify that the fault handler has to do it. And stop returning a value at all from ptep_set_access_flags() (I *think* that's the only thing the return value gets used for - flushing the TLB on the local cpu for the cpu's that want it).

I would be open to it, but just in case it causes bisectable problems I'd really want to see it in two patches ("make it always do the local flush" followed by "remove even the local flush"), and then it would pinpoint any need.

Linus

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

Ok, will do it like that. INVLPG overhead is small effect, nevertheless it's worth trying.

What *has* shown up in my profiles though, and which drove some of these changes is that for heavily threaded VM-intense workloads such as a single SPECjbb JVM instance running on all CPUs and all nodes, TLB flushes with any sort of serialization aspect are absolutely deadly.

So just to be *able* to verify the performance benefit and impact of some of the later NUMA-directed changes, we had to eliminate a number of scalability bottlenecks and put these optimization patches in front of the main changes.

That is why you have to go 20+ patches into the queue to see the real point :-/

Yeah, and I think part of the problem is that it's also a not particularly straightforward to analyze performance bottleneck:
SMP TLB flushing does not show up as visible high overhead in profiles mainly, it mostly shows up as extra idle time.

If the nature of the workload is that it has extra available paralellism that can fill in the idle time, it will mask much of the effect and there's only a slight shift in the profile.

It needs a borderline loaded system and sleep profiling to pinpoint these sources of overhead.

[...]

Yeah, 100% agreed.

Thanks,

Ingo

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

Actually, it is architected on x86. This was first described in the intel appnote 317080 "TLBs, Paging-Structure Caches, and Their Invalidation", last paragraph of section 5.1. Nowadays, the same contents are buried somewhere in Volume 3 of the architecture manual (in my copy: 4.10.4.1 Operations that Invalidate TLBs and Paging-Structure Caches)

I would be very worried about assuming every non-x86 arch has similar TLB semantics. However, if their fault handlers always invalidate TLB for pages that get spurious faults, then skipping the remote invalidation would be fine. (I believe this is what tlb_fix_spurious_fault() is for ?)

Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

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

This unfortunately would only work for processes with no threads because it only works on the current logical CPU.

-Andi

[email protected] -- Speaking for myself only

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

That is fine.

Potentially triggering a spurious page fault on
another CPU is bound to be better than always
doing a synchronous remote TLB flush, waiting
for who knows how many CPUs to acknowledge the
IPI...

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

The other killer is the fundamental IPI delay - which makes it 'invisible' to regular profiling and makes it hard to analyze.

So yes, even the local flush is a win, a major one - and the flush-less one is likely a win too, because INVLPG has some TLB-cache-walking costs.

Rik, mind sending an updated patch that addresses Linus's concerns, or should I code it up if you are busy?

We can also certainly try the second patch, but I'd do it at the end of the series, to put some tree distance between the two patches, to not concentrate regression risks too tightly in the Git space, to help out with hard to bisect problems...

Thanks,

Ingo

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

I'd also like to have the second patch separately because I'd like to measure spurious fault frequency before and after the change, with a reference workload.

Just a single page fault, even it's a minor one, might make a micro-optimization a net loss. INVLPG might be the cheaper option on average - it needs to be measured. (I'll do that, just please keep it separate from the main TLB-flush optimization.)

Thanks,

Ingo

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

Here are the TLB patches as requested:
---8<---

The function ptep_set_access_flags() is only ever invoked to upgrade access permissions on a PTE. That makes it safe to skip flushing the TLBs on remote TLBs. The worst that can happen is a spurious page fault on other CPUs, which would flush that TLB entry.

Lazily letting another CPU incur a spurious page fault occasionally is (much!) cheaper than aggressively flushing everybody else's TLB.

---
 arch/x86/mm/pgtable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 8573b83..b3b852c 100644
@@ -310,7 +310,7 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	if (changed && dirty) {
 		*ptep = entry;
 		pte_update_defer(vma->vm_mm, address, ptep);
-		flush_tlb_page(vma, address);
+		__flush_tlb_one(address);
 	}
 
 	return changed;

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

Same deal. Please don't call these "access permissions". That confuses the issue.

Linus

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

I can change the text of the changelog, however it looks like do_wp_page does actually use ptep_set_access_flags to set the write bit in the pte...

I guess both need to be reflected in the changelog text somehow?

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

Yeah, and by now, after all this discussion, I suspect it should be committed with a comment too. Commit messages are good and all, but unless chasing a particular bug they introduced, we shouldn't expect people to read them for background information.

Linus

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

Will do :)

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

Now that we have the TLB things taken care of, and comments to patches 10/31 and 26/31 have been addressed, is there anything else that needs to be done before these NUMA patches can be merged?

Anyone, this is a good time to speak up. We have some time to address whatever concern you may have.

(besides waiting for the next merge window)

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

Hi,

I know I'm very late in speaking up - but still I'll hazard a try. This is not exactly the same topic but closely related.

There is a a different call to flush_tlb_fix_spurious( ), towards the end of handle_pte_fault( ) which commit 61c77326d "x86, mm: Avoid unnecessary TLB flush" made no-op for X86. However is this really needed for any arch at all - even if we don't know all the arch specific quirks.

Given the code flow below

handle_pte_fault( )
....
....
if ptep_set_access_flags()-> if PTE chg remote TLB shot (pgtable-generic.c ver)
update_mmu_cache -> if PTE chg local TLB possibly shot too
else
flush_tlb_fix_spurious_fault -> PTE didn't change - still remote TLB shotdown

So for PTE unchanged case, we default to doing remote TLB IPIs (barring X86) - unless arch makes this macro NULL.

Thing is, in case of SMP races - due to PTE being different - any fixups to local/remote will be handled within ptep_set_access_flags( ) - arch-specific or generic versions. What I fail to understand is need to do anything - specially a remote shootdown, for PTE not changed case.

I could shut up and just make it NO-OP for ARC, but ....

Please note that for the record, the addition of this special case was done via following change. It might help answer what I feel to comprehend.

2005-10-29 1a44e14 [PATCH] .text page fault SMP scalability optimization

I might be totally off track so please feel free to bash me - but atleast I would end up knowing more !

Thx,
-Vineet

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

Intel has an architectural guarantee that the TLB entry causing a page fault gets invalidated automatically. This means we should be able to drop the local TLB invalidation.

Because of the way other areas of the page fault code work, chances are good that all x86 CPUs do this. However, if someone somewhere has an x86 CPU that does not invalidate the TLB entry causing a page fault, this one-liner should be easy to revert.

---
 arch/x86/mm/pgtable.c | 1 -
 1 file changed, 1 deletion(-)

index b3b852c..15e5953 100644
@@ -310,7 +310,6 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	if (changed && dirty) {
 		*ptep = entry;
 		pte_update_defer(vma->vm_mm, address, ptep);
-		__flush_tlb_one(address);
 	}
 
 	return changed;

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

This does not strike me as a good standard of validation for such a change

At the very least we should have an ACK from AMD and from VIA, and preferably ping RDC and some of the other embedded folks. Given an AMD and VIA ACK I'd be fine. I doubt anyone knows any more what Cyrix CPUs did or cared about and I imagine H Peter or Linus can answer for Transmeta ;-)

Alan

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

Fair enough.

If it turns out any of those CPUs need an explicit flush, then we can also adjust flush_tlb_fix_spurious_fault to actually do a local flush on x86 (or at least on those CPUs).

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

Yes. And even if we have 'confirmation' from documentation and elsewhere, testing has to be done to see actual real behavior of CPUs, so this is going to be a separate, bisectable commit put under surveillance ;-)

Thanks,

Ingo

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

Florian, would you happen to know who at RDC could be contacted to verify whether a TLB entry causing a page fault gets invalidated automatically, upon entering the page fault path?

Borislav, would you happen to know whether AMD (and VIA) CPUs automatically invalidate TLB entries that cause page faults? If you do not know, would you happen who to ask? :)

If these CPUs do not invalidate a TLB entry causing a page fault (a write fault on a read-only PTE), then we may have to change the kernel so flush_tlb_fix_spurious_fault does something on the CPU models in question...

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

Short answer: yes.

Long answer (from APM v2, section 5.5.2):

Use of Cached Entries When Reporting a Page Fault Exception.

On current AMD64 processors, when any type of page fault exception is encountered by the MMU, any cached upper-level entries that lead to the faulting entry are flushed (along with the TLB entry, if already cached) and the table walk is repeated to confirm the page fault using the table entries in memory. This is done because a table entry is allowed to be upgraded (by marking it as present, or by removing its write, execute or supervisor restrictions) without explicitly maintaining TLB coherency. Such an upgrade will be found when the table is re-walked, which resolves the fault. If the fault is confirmed on the re-walk however, a page fault exception is reported, and upper level entries that may have been cached on the re-walk are flushed.

HTH.

Regards/Gruss,
Boris.

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

Can you verify that this is true for older cpu's too (ie the old pre-64-bit ones, say K6 and original Athlon)?

Well, that was traditionally solved by not caching not-present entries at all. Which can be a problem for some things (prefetch of NULL etc), so caching and then re-checking on faults is potentially the correct thing, but I'm just mentioning it because it might not be much of an argument for older microarchitectures..

.. but this is obviously what we're interested in. And since AMD has documented it (as well as Intel), I have this strong suspicion that operating systems have traditionally relied on this behavior.

I don't remember the test coverage details from my Transmeta days, and while I certainly saw the page table walker, it wasn't my code.

My gut feel is that this is likely something x86 just always does (because it's the right thing to do to keep things simple for software), but getting explicit confirmation about older AMD cpu's would definitely be good.

Linus

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

Albeit with a slight delay, the answer is yes: all AMD cpus automatically invalidate cached TLB entries (and intermediate walk results, for that matter) on a #PF.

I don't know, however, whether it would be prudent to have some sort of a cheap assertion in the code (cheaper than INVLPG %ADDR, although on older cpus we do MOV CR3) just in case. This should be enabled only with DEBUG_VM on, of course...

HTH.

Regards/Gruss,
Boris.

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

Thanks. I suspect it ends up being basically architectural, and that Windows (and quite possibly Linux versions too) have depended on the behavior.

I wonder how we could actually test for it. We'd have to have some per-cpu page-fault address check (along with a generation count on the mm or similar). I doubt we'd figure out anything that works reliably and efficiently and would actually show any problems (plus we would have no way to ever know we even got the code right, since presumably we'd never find hardware that actually shows the behavior we'd be looking for..)

Linus

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

Hmm, touching some wrong page through the stale TLB entry could be a pretty nasty issue to debug. But you're probably right: how does one test cheaply whether a PTE just got kicked out of the TLB? Maybe mark it not-present but this would force a rewalk in the case when it is shared, which is penalty we don't want to pay.

Oh well...

Regards/Gruss,
Boris.

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

Would it be enough to simply print out a warning if we fault on the same address twice (or three times) in a row, and then flush the local TLB?

I realize this would not just trigger on CPUs that fail to invalidate TLB entries that cause faults, but also on kernel paths that cause a page fault to be re-taken...

... but then again, don't we want to find those paths and fix them, anyway? :)

All rights reversed

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

I'm actually curious if the architecture docs/software developer manuals for IA-32 mandate any TLB invalidations on a #PF

Is there any official vendor documentation on the subject?

And perhaps equally valid, should we trust it if it exists?

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

Yes. Quoting a prior email:

Actually, it is architected on x86. This was first described in the intel appnote 317080 "TLBs, Paging-Structure Caches, and Their Invalidation", last paragraph of section 5.1. Nowadays, the same contents are buried somewhere in Volume 3 of the architecture manual (in my copy: 4.10.4.1 Operations that Invalidate TLBs and Paging-Structure Caches)

I know that Intel has been very careful in documenting the architected TLB behaviors and did it with the understanding that people should be able to depend on what's being written up there.

Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

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

The function ptep_set_access_flags is only ever used to upgrade access permissions to a page. That means the only negative side effect of not flushing remote TLBs is that other CPUs may incur spurious page faults, if they happen to access the same address, and still have a PTE with the old permissions cached in their TLB.

Having another CPU maybe incur a spurious page fault is faster than always incurring the cost of a remote TLB flush, so replace the remote TLB flush with a purely local one.

This should be safe on every architecture that correctly implements flush_tlb_fix_spurious_fault() to actually invalidate the local TLB entry that caused a page fault, as well as on architectures where the hardware invalidates TLB entries that cause page faults.

In the unlikely event that you are hitting what appears to be an infinite loop of page faults, and 'git bisect' took you to this changeset, your architecture needs to implement flush_tlb_fix_spurious_fault to actually flush the TLB entry.

---
 mm/pgtable-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e642627..0361369 100644
@@ -27,7 +27,7 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	int changed = !pte_same(*ptep, entry);
 	if (changed) {
 		set_pte_at(vma->vm_mm, address, ptep, entry);
-		flush_tlb_page(vma, address);
+		flush_tlb_fix_spurious_fault(vma, address);
 	}
 	return changed;
 }

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

NOTE: It's *not* "access permissions". It's "access flags".

Big difference. This is not about permissions at all.

The access flags are the Accessed and Dirty bits. And the dirty bit is never *cleared* by this function, it's only ever potentially set. That, together with the fact that the accessed flag is "best effort" rather than exact, is what makes this function so special to begin with.

Linus

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

Anyway, modulo the misleading commit messages, ACK from on the series.

Linus

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

It looks like do_wp_page also sets the write bit in the pte "entry" before passing it to ptep_set_access_flags, making that the place where the write bit is set in the pte.

Is this a bug in do_wp_page?

Am I reading things wrong?

reuse:

                flush_cache_page(vma, address, pte_pfn(orig_pte));
                entry = pte_mkyoung(orig_pte);
                entry = maybe_mkwrite(pte_mkdirty(entry), vma);
                if (ptep_set_access_flags(vma, address, page_table, 
entry,1))
                        update_mmu_cache(vma, address, page_table);

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

Hmm. Right you are. That's indeed worth noting that it can indeed change access permissions in that particular way (ie enabling writes).

So yeah, good catch. And it's ok to add the writeable bits like this (and it can't race with hardware like the dirty bit can, since hardware never sets writability).

In fact, it should probably be documented in the source code somewhere. In particular, there's a very subtle requirement that you can only set the writable bit if the dirty bit is also set at the same time, for example.

Linus

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

The function ptep_set_access_flags is only ever used to upgrade access permissions to a page. That means the only negative side effect of not flushing remote TLBs is that other CPUs may incur spurious page faults, if they happen to access the same address, and still have a PTE with the old permissions cached in their TLB.

Having another CPU maybe incur a spurious page fault is faster than always incurring the cost of a remote TLB flush, so replace the remote TLB flush with a purely local one.

This should be safe on every architecture that correctly implements flush_tlb_fix_spurious_fault() to actually invalidate the local TLB entry that caused a page fault, as well as on architectures where the hardware invalidates TLB entries that cause page faults.

In the unlikely event that you are hitting what appears to be an infinite loop of page faults, and 'git bisect' took you to this changeset, your architecture needs to implement flush_tlb_fix_spurious_fault to actually flush the TLB entry.

---
 mm/pgtable-generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index e642627..d8397da 100644
@@ -12,8 +12,8 @@
 
 #ifndef __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
 /*
- * Only sets the access flags (dirty, accessed, and
- * writable). Furthermore, we know it always gets set to a "more
+ * Only sets the access flags (dirty, accessed), as well as write 
+ * permission. Furthermore, we know it always gets set to a "more
  * permissive" setting, which allows most architectures to optimize
  * this. We return whether the PTE actually changed, which in turn
  * instructs the caller to do things like update__mmu_cache.  This

@@ -27,7 +27,7 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 	int changed = !pte_same(*ptep, entry);
 	if (changed) {
 		set_pte_at(vma->vm_mm, address, ptep, entry);
-		flush_tlb_page(vma, address);
+		flush_tlb_fix_spurious_fault(vma, address);
 	}
 	return changed;
 }

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

No, the point is, if we are only *increasing* permissions on a page, we can skip the remote TLB invalidations. Later on each remote CPU might possibly get a spurious fault on that page, but that spurious fault will resynchronize their TLBs for that page, so that the instruction retry after the fault won't fault again.

It is often cheaper to let remote CPUs get an occasional spurious fault than to synchronize with them on every permission change.

Of course, none of the above applies if we are *reducing* permissions on a page (we really can't skip TLB invalidations there)

Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

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

Good. I should have known it must be architected, because we've gone back-and-forth on this in the kernel historically. We used to have some TLB invalidates in the faulting path because I wasn't sure whether they were needed or not, but we clearly don't have them any more (and I suspect coverage was always spotty).

And Intel (and AMD) have been very good at documenting as architected these kinds of details that people end up relying on even if they weren't necessarily originally explicitly documented.

Yes. Of course, there may be some case where we unintentionally don't necessarily flush a faulting address (on some architecture that needs it), and then removing the cross-cpu invalidate could expose that pre-existing bug-let, and cause an infinite loop of page faults due to a TLB entry that never gets invalidated even if the page tables are actually up-to-date.

So changing the mm/pgtable-generic.c function sounds like the right thing to do, but would be a bit more scary.

Changing the x86 version sounds safe, *especially* since you point out that the "fault-causes-tlb-invalidate" is architected behavior.

So I'd almost be willing to drop the invalidate in just one single commit, because it really should be safe. The only thing it does is guarantee that the accessed bit gets updated, and the accessed bit just isn't that important. If we never flush the TLB on another CPU that continues to use a TLB entry where the accessed bit is set (even if it's cleared in the in-memory page tables), the worst that can happen is that the accessed bit doesn't ever get set even if that CPU constantly uses the page.

And nobody will *ever* care. The A bit is purely a heuristic for the page LRU thing, we don't care about irrelevant special cases that won't even affect correctness (much less performance - if that thing is really hot and stays in the TLB, if we evict it, it will immediately get reloaded anyway).

And doing a TLB invalidate even locally is worthless: sure, setting the dirty bit and not invalidating the TLB can cause a local micro-tlb fault (not a software-visible one, just microarchitectural pipeline restart with TLB reload) on the next write access (because the TLB would still contain D=0), so *eve*if* the CPU didn't invalidate-on-fault, there's no reason we should invalidate in software on x86.

Again, this can be different on non-x86 architectures with software dirty bits, where a stale TLB entry that never gets flushed could cause infinite TLB faults that never make progress, but that's really a TLB _walker_ issue, not a generic VM issue.

Linus

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

I suspect it would be safe to simply call tlb_fix_spurious_fault() both on x86 and in the generic version.

If tlb_fix_spurious_fault is broken on some architecture, they would already be running into issues like "write page fault loops until the next context switch" :)

Would tlb_fix_spurious_fault take care of that on those architectures?

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

.. assuming that they implement it as a real TLB flush, yes.

But maybe the architecture never noticed that it happened to depend on the fact that we do a cross-CPU invalidate? So a missing tlb_fix_spurious_fault() implementation could cause a short loop of repeated page faults, until the IPI happens. And it would be so incredibly rare that nobody would ever have noticed.

And if that could have happened, then with the cross-cpu invalidate removed, the "incredibly rare short-lived constant page fault retry" could turn into "incredibly rare lockup due to infinite page fault retry due to TLB entry that never turns dirty despite it being marked dirty by SW in the in-memory page tables".

Very unlikely, I agree. And this is only relevant for the non-x86 case, so changing the x86-specific optimized version is an independent issue.

Linus

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

I suspect the next context switch would flush out the TLB, making it a slowdown, not a lockup.

Still a good reason to make such a change in its own commit, so it can be bisected and tracked down.

The commit message could tell architecture maintainers what to do if this particular commit got them into trouble:
implement a proper local TLB flush in tlb_fix_spurious_fault.

I'll send this in as a separate patch.

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

Common case, yes. But the page fault might happen in kernel space (due to a "put_user()" call, say), and with CONFIG_PREEMPT=n.

Sure, put_user() is always done in a context where blocking (and scheduling) is legal, but that doesn't necessarily equate scheduling actually happening. If we're returning to kernel space and don't have any IO, it might never happen.

Anyway, I suspect such behavior it's almost impossible to trigger. Which would just make it rather hard to find.

Linus

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

From: Rik van Riel <[email protected]>

If ptep_clear_flush() is called to clear a page table entry that is accessible anyway by the CPU, eg. a _PAGE_PROTNONE page table entry, there is no need to flush the TLB on remote CPUs.

---
 mm/pgtable-generic.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: tip/mm/pgtable-generic.c
===================================================================
@@ -88,7 +88,8 @@ pte_t ptep_clear_flush(struct vm_area_st
 {
 	pte_t pte;
 	pte = ptep_get_and_clear((vma)->vm_mm, address, ptep);
-	flush_tlb_page(vma, address);
+	if (pte_accessible(pte))
+		flush_tlb_page(vma, address);
 	return pte;
 }
 #endif

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

From: Gerald Schaefer <[email protected]>

This patch adds an implementation of pmd_pgprot() for s390, in preparation to future THP changes.

---
 arch/s390/include/asm/pgtable.h |   13 +++++++++++++
 1 file changed, 13 insertions(+)

Index: tip/arch/s390/include/asm/pgtable.h
===================================================================
@@ -1240,6 +1240,19 @@ static inline void set_pmd_at(struct mm_
 	*pmdp = entry;
 }
 
+static inline pgprot_t pmd_pgprot(pmd_t pmd)
+{
+	pgprot_t prot = PAGE_RW;
+
+	if (pmd_val(pmd) & _SEGMENT_ENTRY_RO) {
+		if (pmd_val(pmd) & _SEGMENT_ENTRY_INV)
+			prot = PAGE_NONE;
+		else
+			prot = PAGE_RO;
+	}
+	return prot;
+}
+
 static inline unsigned long massage_pgprot_pmd(pgprot_t pgprot)
 {
 	unsigned long pgprot_pmd = 0;

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

The additional pmd_pgprot implementations only are necessary if we want to preserve the PROT_NONE protections across a split but that somewhat forces that PROT_NONE be used as the protection bit across all architectures. Is that possible? I think I would prefer that prot-protection-across-splits just went away until it was proven necessary and potentially recoded in terms of _PAGE_NUMA and friends instead.

Mel Gorman
SUSE Labs

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

From: Ralf Baechle <[email protected]>

Add the pmd_pgprot() method that will be needed
by the new NUMA code.

---
 arch/mips/include/asm/pgtable.h |    2 ++
 1 file changed, 2 insertions(+)

Index: tip/arch/mips/include/asm/pgtable.h
===================================================================
@@ -89,6 +89,8 @@ static inline int is_zero_pfn(unsigned l
 
 extern void paging_init(void);
 
+#define pmd_pgprot(x)		__pgprot(pmd_val(x) & ~_PAGE_CHG_MASK)
+
 /*
  * Conversion functions: convert a page and protection to a page entry,
  * and a page entry and page directory to the page they refer to.

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

From: Ingo Molnar <[email protected]>

pgprot_modify() is available on x86, but on other architectures it only gets defined in mm/mprotect.c - breaking the build if anything outside of mprotect.c tries to make use of this function.

Move it to the generic pgprot area in mm.h, so that an upcoming patch can make use of it.

---
 include/linux/mm.h |   13 +++++++++++++
 mm/mprotect.c      |    7 -------
 2 files changed, 13 insertions(+), 7 deletions(-)

Index: tip/include/linux/mm.h
===================================================================
@@ -164,6 +164,19 @@ extern pgprot_t protection_map[16];
 #define FAULT_FLAG_TRIED	0x40	/* second try */
 
 /*
+ * Some architectures (such as x86) may need to preserve certain pgprot
+ * bits, without complicating generic pgprot code.
+ *
+ * Most architectures don't care:
+ */
+#ifndef pgprot_modify
+static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
+{
+	return newprot;
+}
+#endif
+
+/*
  * vm_fault is filled by the the pagefault handler and passed to the vma's
  * ->fault function. The vma's ->fault is responsible for returning a bitmask
  * of VM_FAULT_xxx flags that give details about how the fault was handled.

Index: tip/mm/mprotect.c
===================================================================
@@ -28,13 +28,6 @@
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 
-#ifndef pgprot_modify
-static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
-{
-	return newprot;
-}
-#endif
-
 static void change_pte_range(struct mm_struct *mm, pmd_t *pmd,
 		unsigned long addr, unsigned long end, pgprot_t newprot,
 		int dirty_accountable)

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

Since the NUMA_INTERLEAVE_HIT statistic is useless on its own; it wants to be compared to either a total of interleave allocations or to a miss count, remove it.

Fixing it would be possible, but since we've gone years without these statistics I figure we can continue that way.

Also NUMA_HIT fully includes NUMA_INTERLEAVE_HIT so users might switch to using that.

This cleans up some of the weird MPOL_INTERLEAVE allocation exceptions.

---
 drivers/base/node.c    |    2 -
 include/linux/mmzone.h |    1 
 mm/mempolicy.c         |   68 +++++++++++++++----------------------------------
 mm/vmstat.c            |    1 
 4 files changed, 22 insertions(+), 50 deletions(-)

Index: tip/drivers/base/node.c
===================================================================
@@ -169,7 +169,7 @@ static ssize_t node_read_numastat(struct
 		       node_page_state(dev->id, NUMA_HIT),
 		       node_page_state(dev->id, NUMA_MISS),
 		       node_page_state(dev->id, NUMA_FOREIGN),
-		       node_page_state(dev->id, NUMA_INTERLEAVE_HIT),
+		       0UL,
 		       node_page_state(dev->id, NUMA_LOCAL),
 		       node_page_state(dev->id, NUMA_OTHER));
 }

Index: tip/include/linux/mmzone.h
===================================================================
@@ -137,7 +137,6 @@ enum zone_stat_item {
 	NUMA_HIT,		/* allocated in intended node */
 	NUMA_MISS,		/* allocated in non intended node */
 	NUMA_FOREIGN,		/* was intended here, hit elsewhere */
-	NUMA_INTERLEAVE_HIT,	/* interleaver preferred this zone */
 	NUMA_LOCAL,		/* allocation from local node */
 	NUMA_OTHER,		/* allocation from other node */
 #endif

Index: tip/mm/vmstat.c
===================================================================
@@ -729,7 +729,6 @@ const char * const vmstat_text[] = {
 	"numa_hit",
 	"numa_miss",
 	"numa_foreign",
-	"numa_interleave",
 	"numa_local",
 	"numa_other",
 #endif

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

NACK, as already posted several times.

This breaks the numactl test suite, which is the only way currently to test interleaving.

Please don't ignore review feedback.

-Andi

[email protected] -- Speaking for myself only

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

This patch is not essential to the NUMA series so I've zapped it from the patch queue and fixed up the roll-on effects.

Thanks,

Ingo

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

Make MPOL_LOCAL a real and exposed policy such that applications that relied on the previous default behaviour can explicitly request it.

---
 include/uapi/linux/mempolicy.h |    1 +
 mm/mempolicy.c                 |    9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

Index: tip/include/uapi/linux/mempolicy.h
===================================================================
@@ -20,6 +20,7 @@ enum {
 	MPOL_PREFERRED,
 	MPOL_BIND,
 	MPOL_INTERLEAVE,
+	MPOL_LOCAL,
 	MPOL_MAX,	/* always last member of enum */
 };
 

Index: tip/mm/mempolicy.c
===================================================================
@@ -269,6 +269,10 @@ static struct mempolicy *mpol_new(unsign
 			     (flags & MPOL_F_RELATIVE_NODES)))
 				return ERR_PTR(-EINVAL);
 		}
+	} else if (mode == MPOL_LOCAL) {
+		if (!nodes_empty(*nodes))
+			return ERR_PTR(-EINVAL);
+		mode = MPOL_PREFERRED;
 	} else if (nodes_empty(*nodes))
 		return ERR_PTR(-EINVAL);
 	policy = kmem_cache_alloc(policy_cache, GFP_KERNEL);

@@ -2371,7 +2375,6 @@ void numa_default_policy(void)
  * "local" is pseudo-policy:  MPOL_PREFERRED with MPOL_F_LOCAL flag
  * Used only for mpol_parse_str() and mpol_to_str()
  */
-#define MPOL_LOCAL MPOL_MAX
 static const char * const policy_modes[] =
 {
 	[MPOL_DEFAULT]    = "default",

@@ -2424,12 +2427,12 @@ int mpol_parse_str(char *str, struct mem
 	if (flags)
 		*flags++ = '\0';	/* terminate mode string */
 
-	for (mode = 0; mode <= MPOL_LOCAL; mode++) {
+	for (mode = 0; mode < MPOL_MAX; mode++) {
 		if (!strcmp(str, policy_modes[mode])) {
 			break;
 		}
 	}
-	if (mode > MPOL_LOCAL)
+	if (mode >= MPOL_MAX)
 		goto out;
 
 	switch (mode) {

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

Seems reasonable but Michael Kerrisk should be cc'd because when the dust settles on this there may be a manual page update required.

Mel Gorman
SUSE Labs

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

From: Lee Schermerhorn <[email protected]>

This patch augments the MPOL_MF_LAZY feature by adding a "NOOP" policy to mbind(). When the NOOP policy is used with the 'MOVE and 'LAZY flags, mbind() will map the pages PROT_NONE so that they will be migrated on the next touch.

This allows an application to prepare for a new phase of operation where different regions of shared storage will be assigned to worker threads, w/o changing policy. Note that we could just use "default" policy in this case. However, this also allows an application to request that pages be migrated, only if necessary, to follow any arbitrary policy that might currently apply to a range of pages, without knowing the policy, or without specifying multiple mbind()s for ranges with different policies.

[ Bug in early version of mpol_parse_str() reported by Fengguang Wu. ]

---
 include/uapi/linux/mempolicy.h |    1 +
 mm/mempolicy.c                 |   11 ++++++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

Index: tip/include/uapi/linux/mempolicy.h
===================================================================
@@ -21,6 +21,7 @@ enum {
 	MPOL_BIND,
 	MPOL_INTERLEAVE,
 	MPOL_LOCAL,
+	MPOL_NOOP,		/* retain existing policy for range */
 	MPOL_MAX,	/* always last member of enum */
 };
 

Index: tip/mm/mempolicy.c
===================================================================
@@ -251,10 +251,10 @@ static struct mempolicy *mpol_new(unsign
 	pr_debug("setting mode %d flags %d nodes[0] %lx\n",
 		 mode, flags, nodes ? nodes_addr(*nodes)[0] : -1);
 
-	if (mode == MPOL_DEFAULT) {
+	if (mode == MPOL_DEFAULT || mode == MPOL_NOOP) {
 		if (nodes && !nodes_empty(*nodes))
 			return ERR_PTR(-EINVAL);
-		return NULL;	/* simply delete any existing policy */
+		return NULL;
 	}
 	VM_BUG_ON(!nodes);
 

@@ -1146,7 +1146,7 @@ static long do_mbind(unsigned long start
 	if (start & ~PAGE_MASK)
 		return -EINVAL;
 
-	if (mode == MPOL_DEFAULT)
+	if (mode == MPOL_DEFAULT || mode == MPOL_NOOP)
 		flags &= ~MPOL_MF_STRICT;
 
 	len = (len + PAGE_SIZE - 1) & PAGE_MASK;

@@ -2381,7 +2381,8 @@ static const char * const policy_modes[]
 	[MPOL_PREFERRED]  = "prefer",
 	[MPOL_BIND]       = "bind",
 	[MPOL_INTERLEAVE] = "interleave",
-	[MPOL_LOCAL]      = "local"
+	[MPOL_LOCAL]      = "local",
+	[MPOL_NOOP]	  = "noop",	/* should not actually be used */
 };
 
 

@@ -2432,7 +2433,7 @@ int mpol_parse_str(char *str, struct mem
 			break;
 		}
 	}
-	if (mode >= MPOL_MAX)
+	if (mode >= MPOL_MAX || mode == MPOL_NOOP)
 		goto out;
 
 	switch (mode) {

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

The MPOL_MF_LAZY feature doesn't exist yet so it's hard to augment at this point :)

This implies that a user-space application has a two-stage process. Stage 1, it marks a range NOOP and the stage 2 marks the range lazy. That feels like it might violate Rusty's API design rule of "The obvious use is wrong." What is the motivation for exposing NOOP to userspace? Instead why does mbind(addr, len, MPOL_MF_LAZY, nodemask, maxnode, flags) not imply that the range gets marked PROT_NONE (or PROT_NUMA or some other variant that is not arch-specific)?

It also seems that MPOL_MF_LAZY must imply MPOL_MF_MOVE or it's a bit pointless without the application having to specify the exact flag combination

I very much like the idea because potentially a motivated developer could use this mechanism to avoid any ping-pong problems with an automatic migration scheme. It could even be argued that any application using MPOL_MF_LAZY get unsubscribed from any automatic mechanism to avoid interference.

Mel Gorman
SUSE Labs

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

From: Lee Schermerhorn <[email protected]>

This patch provides a new function to test whether a page resides on a node that is appropriate for the mempolicy for the vma and address where the page is supposed to be mapped. This involves looking up the node where the page belongs. So, the function returns that node so that it may be used to allocated the page without consulting the policy again.

A subsequent patch will call this function from the fault path. Because of this, I don't want to go ahead and allocate the page, e.g., via alloc_page_vma() only to have to free it if it has the correct policy. So, I just mimic the alloc_page_vma() node computation logic--sort of.

Note: we could use this function to implement a MPOL_MF_STRICT behavior when migrating pages to match mbind() mempolicy--e.g., to ensure that pages in an interleaved range are reinterleaved rather than left where they are when they reside on any page in the interleave nodemask.

[ Added MPOL_F_LAZY to trigger migrate-on-fault;
simplified code now that we don't have to bother with special crap for interleaved ]

---
 include/linux/mempolicy.h      |    8 ++++
 include/uapi/linux/mempolicy.h |    1 
 mm/mempolicy.c                 |   76 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+)

Index: tip/include/linux/mempolicy.h
===================================================================
@@ -198,6 +198,8 @@ static inline int vma_migratable(struct
 	return 1;
 }
 
+extern int mpol_misplaced(struct page *, struct vm_area_struct *, unsigned long);
+
 #else
 
 struct mempolicy {};

@@ -323,5 +325,11 @@ static inline int mpol_to_str(char *buff
 	return 0;
 }
 
+static inline int mpol_misplaced(struct page *page, struct vm_area_struct *vma,
+				 unsigned long address)
+{
+	return -1; /* no node preference */
+}
+
 #endif /* CONFIG_NUMA */
 #endif

Index: tip/include/uapi/linux/mempolicy.h
===================================================================
@@ -61,6 +61,7 @@ enum mpol_rebind_step {
 #define MPOL_F_SHARED  (1 << 0)	/* identify shared policies */
 #define MPOL_F_LOCAL   (1 << 1)	/* preferred local allocation */
 #define MPOL_F_REBINDING (1 << 2)	/* identify policies in rebinding */
+#define MPOL_F_MOF	(1 << 3) /* this policy wants migrate on fault */
 
 
 #endif /* _UAPI_LINUX_MEMPOLICY_H */

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

In order to facilitate a lazy -- fault driven -- migration of pages, create a special transient PROT_NONE variant, we can then use the 'spurious' protection faults to drive our migrations from.

Pages that already had an effective PROT_NONE mapping will not be detected to generate these 'spuriuos' faults for the simple reason that we cannot distinguish them on their protection bits, see pte_numa().

This isn't a problem since PROT_NONE (and possible PROT_WRITE with dirty tracking) aren't used or are rare enough for us to not care about their placement.

---
 include/linux/huge_mm.h |   19 ++++++++++++
 include/linux/mm.h      |   18 +++++++++++
 mm/huge_memory.c        |   32 ++++++++++++++++++++
 mm/memory.c             |   75 +++++++++++++++++++++++++++++++++++++++++++-----
 mm/mprotect.c           |   24 ++++++++++-----
 5 files changed, 154 insertions(+), 14 deletions(-)

Index: tip/include/linux/huge_mm.h
===================================================================
@@ -159,6 +159,13 @@ static inline struct page *compound_tran
 	}
 	return page;
 }
+
+extern bool pmd_numa(struct vm_area_struct *vma, pmd_t pmd);
+
+extern void do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
+				  unsigned long address, pmd_t *pmd,
+				  unsigned int flags, pmd_t orig_pmd);
+
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
 #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
 #define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })

@@ -195,6 +202,18 @@ static inline int pmd_trans_huge_lock(pm
 {
 	return 0;
 }
+
+static inline bool pmd_numa(struct vm_area_struct *vma, pmd_t pmd)
+{
+	return false;
+}
+
+static inline void do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
+				  unsigned long address, pmd_t *pmd,
+				  unsigned int flags, pmd_t orig_pmd)
+{
+}
+
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
 #endif /* _LINUX_HUGE_MM_H */

Index: tip/include/linux/mm.h
===================================================================
@@ -1091,6 +1091,9 @@ extern unsigned long move_page_tables(st
 extern unsigned long do_mremap(unsigned long addr,
 			       unsigned long old_len, unsigned long new_len,
 			       unsigned long flags, unsigned long new_addr);
+extern void change_protection(struct vm_area_struct *vma, unsigned long start,
+			      unsigned long end, pgprot_t newprot,
+			      int dirty_accountable);
 extern int mprotect_fixup(struct vm_area_struct *vma,
 			  struct vm_area_struct **pprev, unsigned long start,
 			  unsigned long end, unsigned long newflags);

@@ -1561,6 +1564,21 @@ static inline pgprot_t vm_get_page_prot(
 }
 #endif
 
+static inline pgprot_t vma_prot_none(struct vm_area_struct *vma)
+{
+	/*
+	 * obtain PROT_NONE by removing READ|WRITE|EXEC privs
+	 */
+	vm_flags_t vmflags = vma->vm_flags & ~(VM_READ|VM_WRITE|VM_EXEC);
+	return pgprot_modify(vma->vm_page_prot, vm_get_page_prot(vmflags));
+}
+
+static inline void
+change_prot_none(struct vm_area_struct *vma, unsigned long start, unsigned long end)
+{
+	change_protection(vma, start, end, vma_prot_none(vma), 0);
+}
+
 struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
 int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
 			unsigned long pfn, unsigned long size, pgprot_t);

Index: tip/mm/huge_memory.c
===================================================================
@@ -725,6 +725,38 @@ out:
 	return handle_pte_fault(mm, vma, address, pte, pmd, flags);
 }
 
+bool pmd_numa(struct vm_area_struct *vma, pmd_t pmd)
+{
+	/*
+	 * See pte_numa().
+	 */
+	if (pmd_same(pmd, pmd_modify(pmd, vma->vm_page_prot)))
+		return false;
+
+	return pmd_same(pmd, pmd_modify(pmd, vma_prot_none(vma)));
+}
+
+void do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
+			   unsigned long address, pmd_t *pmd,
+			   unsigned int flags, pmd_t entry)
+{
+	unsigned long haddr = address & HPAGE_PMD_MASK;
+
+	spin_lock(&mm->page_table_lock);
+	if (unlikely(!pmd_same(*pmd, entry)))
+		goto out_unlock;
+
+	/* do fancy stuff */
+
+	/* change back to regular protection */
+	entry = pmd_modify(entry, vma->vm_page_prot);
+	if (pmdp_set_access_flags(vma, haddr, pmd, entry, 1))
+		update_mmu_cache_pmd(vma, address, entry);
+
+out_unlock:
+	spin_unlock(&mm->page_table_lock);
+}
+
 int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 		  pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
 		  struct vm_area_struct *vma)

Index: tip/mm/mprotect.c
===================================================================
@@ -112,7 +112,7 @@ static inline void change_pud_range(stru
 	} while (pud++, addr = next, addr != end);
 }
 
-static void change_protection(struct vm_area_struct *vma,
+static void change_protection_range(struct vm_area_struct *vma,
 		unsigned long addr, unsigned long end, pgprot_t newprot,
 		int dirty_accountable)
 {

@@ -134,6 +134,20 @@ static void change_protection(struct vm_
 	flush_tlb_range(vma, start, end);
 }
 
+void change_protection(struct vm_area_struct *vma, unsigned long start,
+		       unsigned long end, pgprot_t newprot,
+		       int dirty_accountable)
+{
+	struct mm_struct *mm = vma->vm_mm;
+
+	mmu_notifier_invalidate_range_start(mm, start, end);
+	if (is_vm_hugetlb_page(vma))
+		hugetlb_change_protection(vma, start, end, newprot);
+	else
+		change_protection_range(vma, start, end, newprot, dirty_accountable);
+	mmu_notifier_invalidate_range_end(mm, start, end);
+}
+
 int
 mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	unsigned long start, unsigned long end, unsigned long newflags)

@@ -206,12 +220,8 @@ success:
 		dirty_accountable = 1;
 	}
 
-	mmu_notifier_invalidate_range_start(mm, start, end);
-	if (is_vm_hugetlb_page(vma))
-		hugetlb_change_protection(vma, start, end, vma->vm_page_prot);
-	else
-		change_protection(vma, start, end, vma->vm_page_prot, dirty_accountable);
-	mmu_notifier_invalidate_range_end(mm, start, end);
+	change_protection(vma, start, end, vma->vm_page_prot, dirty_accountable);
+
 	vm_stat_account(mm, oldflags, vma->vm_file, -nrpages);
 	vm_stat_account(mm, newflags, vma->vm_file, nrpages);
 	perf_event_mmap(vma);

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

The changelog should mention that fault-driven migration also means that the full cost of migration is incurred by the process. If someone in the future tries to do the migration in a kernel thread they should be reminded that the fault-driven choice was deliberate.

Mel Gorman
SUSE Labs

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

From: Lee Schermerhorn <[email protected]>

This patch adds another mbind() flag to request "lazy migration". The flag, MPOL_MF_LAZY, modifies MPOL_MF_MOVE* such that the selected pages are marked PROT_NONE. The pages will be migrated in the fault path on "first touch", if the policy dictates at that time.

"Lazy Migration" will allow testing of migrate-on-fault via mbind(). Also allows applications to specify that only subsequently touched pages be migrated to obey new policy, instead of all pages in range. This can be useful for multi-threaded applications working on a large shared data area that is initialized by an initial thread resulting in all pages on one [or a few, if overflowed] nodes. After PROT_NONE, the pages in regions assigned to the worker threads will be automatically migrated local to the threads on 1st touch.

---
 include/uapi/linux/mempolicy.h |   13 ++++++++--
 mm/mempolicy.c                 |   49 ++++++++++++++++++++++++++---------------
 2 files changed, 42 insertions(+), 20 deletions(-)

Index: tip/include/uapi/linux/mempolicy.h
===================================================================
@@ -49,9 +49,16 @@ enum mpol_rebind_step {
 
 /* Flags for mbind */
 #define MPOL_MF_STRICT	(1<<0)	/* Verify existing pages in the mapping */
-#define MPOL_MF_MOVE	(1<<1)	/* Move pages owned by this process to conform to mapping */
-#define MPOL_MF_MOVE_ALL (1<<2)	/* Move every page to conform to mapping */
-#define MPOL_MF_INTERNAL (1<<3)	/* Internal flags start here */
+#define MPOL_MF_MOVE	 (1<<1)	/* Move pages owned by this process to conform
+				   to policy */
+#define MPOL_MF_MOVE_ALL (1<<2)	/* Move every page to conform to policy */
+#define MPOL_MF_LAZY	 (1<<3)	/* Modifies '_MOVE:  lazy migrate on fault */
+#define MPOL_MF_INTERNAL (1<<4)	/* Internal flags start here */
+
+#define MPOL_MF_VALID	(MPOL_MF_STRICT   | 	\
+			 MPOL_MF_MOVE     | 	\
+			 MPOL_MF_MOVE_ALL |	\
+			 MPOL_MF_LAZY)
 
 /*
  * Internal flags that share the struct mempolicy flags word with

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

I like the idea. Applications will need some way of progamatically detecting if MPOL_MF_LAZY is available. I guess they could always try the flag and if it fails, do nothing. When/if the manual page for it happens it probably will be a lot more obvious. The changelog does not actually describe what this patch does though

This patch is a preparation step for "lazy migration". When the flag is used, the range is marked prot_none. In a later patch it will be detected at fault time that the page is misplaced and migrate it at that point.