x86,tlb,mm: make lazy TLB mode even lazier

Hi Rik,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.17]
[also build test ERROR on next-20180620]
[cannot apply to tip/x86/core linus/master mmotm/master v4.18-rc1] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Rik-van-Riel/x86-tlb
-mm-make-lazy-TLB-mode-even-lazier/20180621-045620
config: x86_64-randconfig-x016-201824 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Rik-van-Riel/x86-tlb-mm-make-lazy-TLB-mode- even-lazier/20180621-045620 HEAD 7f2e7d915758c367dd0515efc17af977592fa141 builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

Quoted 2 lines

Quoted 2 lines

mm/init-mm.c:38:1: error: Only string constants are supported as initializers for randomized structures with flexible arrays

    };

kernel/fork.c +0 -3

 	/*
-	 * FIXME! The "sizeof(struct mm_struct)" currently includes the
-	 * whole struct cpumask for the OFFSTACK case. We could change
-	 * this to *only* allocate as much of it as required by the

Quoted 20 lines · Rik van Riel

(quoted diff) +0 -1

 	/*
-	 * FIXME! The "sizeof(struct mm_struct)" currently

includes the

(quoted diff) +0 -1

-	 * whole struct cpumask for the OFFSTACK case. We could

change

(quoted diff) +0 -1

-	 * this to *only* allocate as much of it as required by

the

(quoted diff) +0 -1

-	 * maximum number of CPU's we can ever have.  The

cpumask_allocation

(quoted diff) +0 -1

-	 * is at the end of the structure, exactly for that

reason.

(quoted diff) +1 -0

+	 * The mm_cpumask is located at the end of mm_struct, and

is

kernel/fork.c +3 -1

+	 * dynamically sized based on nr_cpu_ids.
 	 */
+	mm_size = sizeof(struct mm_struct) + cpumask_size();
+
 	mm_cachep = kmem_cache_create_usercopy("mm_struct",
-			sizeof(struct mm_struct),

ARCH_MIN_MMSTRUCT_ALIGN,

kernel/fork.c void __init proc_caches_init(void) · line 2271

Quoted 1 line

kernel/fork.c +1 -0

+			mm_size, ARCH_MIN_MMSTRUCT_ALIGN,

Could you add a bit to that comment, like "dynamically sized based on nr_cpu_ids" ... which is sized based on the number of possible CPUs.

I found myself wondering how that interacts with hotplug.

arch/x86/mm/tlb.c +5 -0

+void tlb_flush_remove_tables(struct mm_struct *mm)
+{
+	int cpu = get_cpu();
+	/*
+	 * XXX: this really only needs to be called for CPUs in lazy

TLB mode.

arch/x86/mm/tlb.c +3 -0

+	 */
+	if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids)
+		smp_call_function_many(mm_cpumask(mm),

tlb_flush_remove_tables_local, (void *)mm, 1);
+}

Andy discovered that speculative memory accesses while in lazy TLB mode can crash a system, when a CPU tries to dereference a speculative access using memory contents that used to be valid page table memory, but have since been reused for something else and point into la-la land.

The latter problem can be prevented in two ways. The first is to always send a TLB shootdown IPI to CPUs in lazy TLB mode, while the second one is to only send the TLB shootdown at page table freeing time.

The second should result in fewer IPIs, since operationgs like mprotect and madvise are very common with some workloads, but do not involve page table freeing. Also, on munmap, batching of page table freeing covers much larger ranges of virtual memory than the batching of unmapped user pages.

Signed-off-by Rik van Riel <[email protected]>

Tested-by Song Liu <[email protected]>

---
 arch/x86/include/asm/tlbflush.h |  5 +++++
 arch/x86/mm/tlb.c               | 24 ++++++++++++++++++++++++
 include/asm-generic/tlb.h       | 10 ++++++++++
 mm/memory.c                     | 22 ++++++++++++++--------
 4 files changed, 53 insertions(+), 8 deletions(-)

arch/x86/include/asm/tlbflush.h +5 -0

index 6690cd3fc8b1..3aa3204b5dc0 100644
@@ -554,4 +554,9 @@ extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
        native_flush_tlb_others(mask, info)
 #endif
 
+extern void tlb_flush_remove_tables(struct mm_struct *mm);
+extern void tlb_flush_remove_tables_local(void *arg);
+
+#define HAVE_TLB_FLUSH_REMOVE_TABLES
+
 #endif /* _ASM_X86_TLBFLUSH_H */

arch/x86/mm/tlb.c +3 -0

index e055d1a06699..61773b07ed54 100644
@@ -646,6 +646,30 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
        put_cpu();
 }
 
+void tlb_flush_remove_tables_local(void *arg)
+{
+       struct mm_struct *mm = arg;

Quoted 18 lines · Rik van Riel

+++ b/arch/x86/mm/tlb.c

arch/x86/mm/tlb.c +8 -0

@@ -646,6 +646,30 @@ void flush_tlb_mm_range(struct mm_struct *mm,
 unsigned long start,
        put_cpu();
 }
 
+void tlb_flush_remove_tables_local(void *arg)
+{
+       struct mm_struct *mm = arg;
+
+       if (this_cpu_read(cpu_tlbstate.loaded_mm) == mm &&
+                       this_cpu_read(cpu_tlbstate.is_lazy))
+               /*
+                * We're in lazy mode.  We need to at least flush

our
+ * paging-structure cache to avoid speculatively
reading

arch/x86/mm/tlb.c void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start, · line 661

Quoted 6 lines

+ * garbage into our TLB. Since switching to
init_mm is barely
+ * slower than a minimal flush, just switch to
init_mm.

arch/x86/mm/tlb.c +2 -0

+                */
+               switch_mm_irqs_off(NULL, &init_mm, NULL);

Can you add braces?

Quoted 7 lines

arch/x86/mm/tlb.c +7 -0

+}
+
+void tlb_flush_remove_tables(struct mm_struct *mm)
+{
+       int cpu = get_cpu();
+       /*
+        * XXX: this really only needs to be called for CPUs in

Quoted 5 lines

lazy TLB mode.

arch/x86/mm/tlb.c +3 -0

+        */
+       if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids)
+               smp_call_function_many(mm_cpumask(mm),

tlb_flush_remove_tables_local, (void *)mm, 1);

I suspect that most if the gain will come from fixing this limitation :)

arch/x86/include/asm/tlbflush.h struct tlb_context { · line 173

Quoted 2 lines · Rik van Riel

arch/x86/include/asm/tlbflush.h +2 -0

+#define TLBSTATE_OK	0
+#define TLBSTATE_LAZY	1

Could we spell out a bit more about what "OK" means in comments? It obviously means "not lazy", but anything else?

Lazy TLB mode can result in an idle CPU being woken up by a TLB flush, when all it really needs to do is reload %CR3 at the next context switch, assuming no page table pages got freed.

This patch deals with that issue by introducing a third TLB state, TLBSTATE_FLUSH, which causes %CR3 to be reloaded at the next context switch.

Atomic compare and exchange is used to close races between the TLB shootdown code and the context switch code. Keying off just the tlb_gen is likely to not be enough, since that would not give lazy_clb_can_skip_flush() information on when it is facing a race and has to send the IPI to a CPU in the middle of a LAZY -> OK switch.

Unlike the 2016 version of this patch, CPUs in TLBSTATE_LAZY are not removed from the mm_cpumask(mm), since that would prevent the TLB flush IPIs at page table free time from being sent to all the CPUs that need them.

Signed-off-by Rik van Riel <[email protected]>

Tested-by Song Liu <[email protected]>

---
 arch/x86/include/asm/tlbflush.h |   5 ++
 arch/x86/include/asm/uv/uv.h    |   6 +--
 arch/x86/mm/tlb.c               | 108 ++++++++++++++++++++++++++++++++++++----
 arch/x86/platform/uv/tlb_uv.c   |   2 +-
 4 files changed, 107 insertions(+), 14 deletions(-)

arch/x86/include/asm/tlbflush.h +5 -0

index 88a4d6b87ff7..0a8bdd6084de 100644
@@ -171,6 +171,7 @@ struct tlb_context {
 
 #define TLBSTATE_OK    0
 #define TLBSTATE_LAZY  1
+#define TLBSTATE_FLUSH 2
 
 struct tlb_state {
        /*

@@ -199,6 +200,10 @@ struct tlb_state {
         *    We're heuristically guessing that the CR3 load we
         *    skipped more than makes up for the overhead added by
         *    lazy mode.
+        *
+        *  - Lazily using a real mm, which has seen a TLB invalidation on
+        *    other CPUs. TLB needs to be flushed at context switch time,
+        *    state == TLBSTATE_FLUSH.
         */
        int state;
 

arch/x86/include/asm/uv/uv.h +3 -3

index a80c0673798f..d801afb5fe90 100644
@@ -17,7 +17,7 @@ extern int is_uv_hubless(void);
 extern void uv_cpu_init(void);
 extern void uv_nmi_init(void);
 extern void uv_system_init(void);
-extern const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
+extern struct cpumask *uv_flush_tlb_others(struct cpumask *cpumask,
                                                 const struct flush_tlb_info *info);
 
 #else  /* X86_UV */

@@ -27,8 +27,8 @@ static inline int is_uv_system(void)  { return 0; }
 static inline int is_uv_hubless(void)  { return 0; }
 static inline void uv_cpu_init(void)   { }
 static inline void uv_system_init(void)        { }
-static inline const struct cpumask *
-uv_flush_tlb_others(const struct cpumask *cpumask,
+static inline struct cpumask *
+uv_flush_tlb_others(struct cpumask *cpumask,
                    const struct flush_tlb_info *info)
 { return cpumask; }
 

arch/x86/mm/tlb.c +98 -10

index e063e623e52c..4b27d8469848 100644
@@ -7,6 +7,7 @@
 #include <linux/export.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
+#include <linux/gfp.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>

@@ -227,8 +228,6 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
                __flush_tlb_all();
        }
 #endif
-       this_cpu_write(cpu_tlbstate.state, TLBSTATE_OK);
-
        /*
         * The membarrier system call requires a full memory barrier and
         * core serialization before returning to user-space, after

@@ -236,8 +235,11 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
         * memory barrier and core serializing instruction.
         */
        if (real_prev == next) {
+               int *tlbstate = this_cpu_ptr(&cpu_tlbstate.state);
+               int oldstate = *tlbstate;
+
                VM_WARN_ON(this_cpu_read(cpu_tlbstate.ctxs[prev_asid].ctx_id) !=
-                          next->context.ctx_id);
+                               next->context.ctx_id);
 
                /*
                 * We don't currently support having a real mm loaded without

@@ -250,6 +252,26 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
                                 !cpumask_test_cpu(cpu, mm_cpumask(next))))
                        cpumask_set_cpu(cpu, mm_cpumask(next));
 
+               /*
+                * In lazy TLB mode. The atomic exchange makes sure there was
+                * no TLB shootdown all the way to this context switch.
+                */
+               if (oldstate == TLBSTATE_LAZY)
+                       oldstate = cmpxchg(tlbstate, oldstate, TLBSTATE_OK);
+
+               /*
+                * There was a TLB invalidation while this CPU was idle.
+                * The code in choose_new_asid will figure out what kind
+                * of flush is needed.
+                */
+               if (oldstate == TLBSTATE_FLUSH)
+                       goto reload_cr3;
+
+               /*
+                * Nothing to do. Either we are switching between two
+                * threads in the same process, or we were in TLBSTATE_LAZY
+                * without any TLB invalidations happening.
+                */
                return;
        } else {
                u16 new_asid;

@@ -294,6 +316,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
                 * Start remote flushes and then read tlb_gen.
                 */
                cpumask_set_cpu(cpu, mm_cpumask(next));
+ reload_cr3:
+               this_cpu_write(cpu_tlbstate.state, TLBSTATE_OK);
                next_tlb_gen = atomic64_read(&next->context.tlb_gen);
 
                choose_new_asid(next, next_tlb_gen, &new_asid, &need_flush);

@@ -454,6 +478,9 @@ static void flush_tlb_func_common(const struct flush_tlb_info *f,
                 * paging-structure cache to avoid speculatively reading
                 * garbage into our TLB.  Since switching to init_mm is barely
                 * slower than a minimal flush, just switch to init_mm.
+                *
+                * This should be rare, with native_flush_tlb_others skipping
+                * IPIs to lazy TLB mode CPUs.
                 */
                switch_mm_irqs_off(NULL, &init_mm, NULL);
                return;

@@ -557,9 +584,57 @@ static void flush_tlb_func_remote(void *info)
        flush_tlb_func_common(f, false, TLB_REMOTE_SHOOTDOWN);
 }
 
+/*
+ * Determine whether a CPU's TLB needs to be flushed now, or whether the
+ * flush can be delayed until the next context switch, by changing the
+ * tlbstate from TLBSTATE_LAZY to TLBSTATE_FLUSH.
+ */
+static bool lazy_tlb_can_skip_flush(int cpu)
+{
+       int *tlbstate = &per_cpu(cpu_tlbstate.state, cpu);
+       int old;
+
+       switch (*tlbstate) {
+       case TLBSTATE_FLUSH:
+               /* The TLB will be flushed on the next context switch. */
+               return true;
+       case TLBSTATE_LAZY:
+               /*
+                * The CPU is in TLBSTATE_LAZY, which could context switch back
+                * to TLBSTATE_OK, re-using the old TLB state without a flush.
+                * if that happened, send a TLB flush IPI.
+                *
+                * Otherwise, the state is now TLBSTATE_FLUSH, and TLB will
+                * be flushed at the next context switch. Skip the IPI.
+                */
+               old = cmpxchg(tlbstate, TLBSTATE_LAZY, TLBSTATE_FLUSH);
+               return old != TLBSTATE_OK;
+       case TLBSTATE_OK:
+       default:
+               /* A task on the CPU is actively using the mm. Flush the TLB. */
+               return false;
+       }
+}
+
 void native_flush_tlb_others(const struct cpumask *cpumask,
                             const struct flush_tlb_info *info)
 {
+       cpumask_t *mask = (struct cpumask *)cpumask;
+       cpumask_var_t varmask;
+       bool can_lazy_flush = false;
+       unsigned int cpu;
+
+       /*
+        * A temporary cpumask allows the kernel to skip sending IPIs
+        * to CPUs in lazy TLB state, without also removing them from
+        * mm_cpumask(mm).
+        */
+       if (alloc_cpumask_var(&varmask, GFP_ATOMIC)) {
+               cpumask_copy(varmask, cpumask);
+               mask = varmask;
+               can_lazy_flush = true;
+       }
+
        count_vm_tlb_event(NR_TLB_REMOTE_FLUSH);
        if (info->end == TLB_FLUSH_ALL)
                trace_tlb_flush(TLB_REMOTE_SEND_IPI, TLB_FLUSH_ALL);

@@ -583,17 +658,30 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
                 * that UV should be updated so that smp_call_function_many(),
                 * etc, are optimal on UV.
                 */
-               unsigned int cpu;
-
                cpu = smp_processor_id();
-               cpumask = uv_flush_tlb_others(cpumask, info);
-               if (cpumask)
-                       smp_call_function_many(cpumask, flush_tlb_func_remote,
+               mask = uv_flush_tlb_others(mask, info);
+               if (mask)
+                       smp_call_function_many(mask, flush_tlb_func_remote,
                                               (void *)info, 1);
-               return;
+               goto out;
+       }
+
+       /*
+        * Instead of sending IPIs to CPUs in lazy TLB mode, move that
+        * CPU's TLB state to TLBSTATE_FLUSH, causing the TLB to be flushed
+        * at the next context switch, or at page table free time.
+        */
+       if (can_lazy_flush) {
+               for_each_cpu(cpu, mask) {
+                       if (lazy_tlb_can_skip_flush(cpu))
+                               cpumask_clear_cpu(cpu, mask);
+               }
        }
-       smp_call_function_many(cpumask, flush_tlb_func_remote,
+
+       smp_call_function_many(mask, flush_tlb_func_remote,
                               (void *)info, 1);
+ out:
+       free_cpumask_var(varmask);
 }
 
 /*

arch/x86/platform/uv/tlb_uv.c +1 -2

index b36caae0fb2f..42730acec8b7 100644
@@ -1102,7 +1102,7 @@ static int set_distrib_bits(struct cpumask *flush_mask, struct bau_control *bcp,
  * Returns pointer to cpumask if some remote flushing remains to be
  * done.  The returned pointer is valid till preemption is re-enabled.
  */
-const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
+struct cpumask *uv_flush_tlb_others(struct cpumask *cpumask,
                                          const struct flush_tlb_info *info)
 {
        unsigned int cpu = smp_processor_id();
--

2.14.4

Quoted 19 lines · Rik van Riel

Lazy TLB mode can result in an idle CPU being woken up by a TLB flush,
when all it really needs to do is reload %CR3 at the next context switch,
assuming no page table pages got freed.

This patch deals with that issue by introducing a third TLB state, TLBSTATE_FLUSH, which causes %CR3 to be reloaded at the next context
switch.

Atomic compare and exchange is used to close races between the TLB shootdown code and the context switch code. Keying off just the tlb_gen is likely to not be enough, since that would not give lazy_clb_can_skip_flush() information on when it is facing a race and has to send the IPI to a CPU in the middle of a LAZY -> OK switch.

Quoted 5 lines

Unlike the 2016 version of this patch, CPUs in TLBSTATE_LAZY are not
removed from the mm_cpumask(mm), since that would prevent the TLB flush IPIs at page table free time from being sent to all the CPUs that need them.

Eek, this is so complicated. In the 2016 version of the patches, you needed all this. But I rewrote the whole subsystem to make it easier now :) I think that you can get rid of all of this and instead just revert the relevant parts of:

b956575bed91ecfb136a8300742ecbbf451471ab

All the bookkeeping is already in place -- no need for new state.

Quoted 34 lines · Andy Lutomirski

Quoted 19 lines · Rik van Riel

Lazy TLB mode can result in an idle CPU being woken up by a TLB flush,
when all it really needs to do is reload %CR3 at the next context switch,
assuming no page table pages got freed.

This patch deals with that issue by introducing a third TLB state, TLBSTATE_FLUSH, which causes %CR3 to be reloaded at the next context
switch.

Atomic compare and exchange is used to close races between the TLB shootdown code and the context switch code. Keying off just the tlb_gen is likely to not be enough, since that would not give lazy_clb_can_skip_flush() information on when it is facing a race and has to send the IPI to a CPU in the middle of a LAZY -> OK switch.

Quoted 5 lines

Unlike the 2016 version of this patch, CPUs in TLBSTATE_LAZY are not
removed from the mm_cpumask(mm), since that would prevent the TLB flush IPIs at page table free time from being sent to all the CPUs that need them.

Eek, this is so complicated. In the 2016 version of the patches, you needed all this. But I rewrote the whole subsystem to make it easier now :) I think that you can get rid of all of this and instead just revert the relevant parts of:

b956575bed91ecfb136a8300742ecbbf451471ab

Quoted 1 line

All the bookkeeping is already in place -- no need for new state.

I looked at using your .tlb_gen stuff, but we need a way to do that race free. I suppose setting the tlbstate to !lazy before checking .tlb_gen might do the trick, if we get the ordering right at the tlb invalidation site, too?

Something like this:

context switch tlb invalidation

advance mm->context.tlb_gen send IPI to cpus with !is_lazy tlb

tlbstate.is_lazy = FALSE
*need_flush = .tlb_gen < next_tlb_gen

Do you see any holes in that?

This patch deals with that issue by introducing a third TLB state, TLBSTATE_FLUSH, which causes %CR3 to be reloaded at the next context switch.

Quoted 4 lines · Rik van Riel

This patch deals with that issue by introducing a third TLB state, TLBSTATE_FLUSH, which causes %CR3 to be reloaded at the next context
switch.

With PCIDs, do we need to be a bit more explicit about what kind of %CR3
reload we have? Because, with PCIDs, we do have non-TLB-flushing %CR3
writes.

CPUs in TLBSTATE_OK have either received TLB flush IPIs earlier on during the munmap (when the user memory was unmapped), or have context switched and reloaded during that stage of the munmap.

Page table free TLB flushes only need to be sent to CPUs in lazy TLB mode, which TLB contents might not yet be up to date yet.

Signed-off-by Rik van Riel <[email protected]>

Tested-by Song Liu <[email protected]>

---
 arch/x86/mm/tlb.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

arch/x86/mm/tlb.c +0 -0

index 4b27d8469848..40b00055c883 100644

arch/x86/mm/tlb.c +6 -1

 	/*
-	 * XXX: this really only needs to be called for CPUs in lazy TLB mode.
+	 * CPUs in TLBSTATE_OK either received a TLB flush IPI while the user
+	 * pages in this address range were unmapped, or have context switched
+	 * and reloaded %CR3 since then.
+	 *
+	 * Shootdown IPIs at page table freeing time only need to be sent to
+	 * CPUs that may have out of date TLB contents.
 	 */

All error/warnings (new ones prefixed by >>):

   arch/x86/xen/mmu_pv.c: In function 'drop_mm_ref_this_cpu':

Quoted 2 lines

Quoted 2 lines

arch/x86/xen/mmu_pv.c:987:3: error: implicit declaration of function 'leave_mm' [-Werror=implicit-function-declaration]

      leave_mm(smp_processor_id());
      ^~~~~~~~

cc1: some warnings being treated as errors

Do not call leave_mm when going into a cstate. Now that mprotect and madvise no longer send IPIs for TLB shootdowns to idle CPUs, there is no real reason to disable lazy TLB mode in idle states.

This seems to help performance on Broadwell systems. Haswell performance numbers are inconclusive.

Quoted 10 lines · Rik van Riel

Do not call leave_mm when going into a cstate. Now that mprotect and
madvise no longer send IPIs for TLB shootdowns to idle CPUs, there is
no real reason to disable lazy TLB mode in idle states.

This seems to help performance on Broadwell systems. Haswell performance
numbers are inconclusive.

I'm skeptical. The code you're removing is more about power consumption than about performance. If a task migrates from one cpu to another, runs for awhile, and exits, we don't want to IPI the old CPU if the old CPU is deeply asleep. The logic you're removing is a bit awkwardly written, but the intent is to only do the leave_mm() in deep sleep. We should arguably change the condition to check the expected sleep duration instead of the microarchitectural properties of the target state, though.

Quoted 13 lines · Andy Lutomirski

Quoted 10 lines · Rik van Riel

Do not call leave_mm when going into a cstate. Now that mprotect and
madvise no longer send IPIs for TLB shootdowns to idle CPUs, there is
no real reason to disable lazy TLB mode in idle states.

This seems to help performance on Broadwell systems. Haswell performance
numbers are inconclusive.

Quoted 8 lines

I'm skeptical. The code you're removing is more about power consumption than about performance. If a task migrates from one cpu to another, runs for awhile, and exits, we don't want to IPI the old CPU if the old CPU is deeply asleep. The logic you're removing is a bit awkwardly written, but the intent is to only do the leave_mm() in deep sleep. We should arguably change the condition to check the expected sleep duration instead of the microarchitectural properties of the target state, though.

That is fair.

A quick hack could be to just remove the tlb flush from C3 state (which gets called a lot), but keep it for C6 state and deeper (which get called more when a CPU is really idle).

--
All Rights Reversed.

Hmm, fair enough. I think a better heuristic would be if the estimated idle duration is more than, say, 10ms. I *think* the code has been cleaned up enough that this is easy now. (Using time instead
of C6 will make it a lot less dependent on which idle driver is in use.)

Quoted 6 lines · Andy Lutomirski

Hmm, fair enough. I think a better heuristic would be if the estimated idle duration is more than, say, 10ms. I *think* the code has been cleaned up enough that this is easy now. (Using time instead
of C6 will make it a lot less dependent on which idle driver is in use.)

This particular bit of code is only in intel_idle though, and not every cpuidle governor estimates an idle duration, nor does it get passed up the stack (presumably because it not always exists).

I will just drop this patch for now, and see if adding back in the patch that skips manipulation of the mm_cpumask(&init_mm), since that might make leave_mm() a little cheaper.

We would still have excess manipulation of the bitmask when re-entering the task from what should have been lazy TLB mode, but total cache line contention would likely still be down from where it is before that patch.

I think the right solution if you want that last little bit of performance is to get rid of the code in intel_idle and to add it in the core idle code. We have fancy scheduler code to estimate the idle
time, and we should use it here IMO.