* [PATCH v2 05/11] x86/mm: Change the leave_mm() condition for local TLB flushes
From: Andy Lutomirski @ 2017-05-22 22:30 UTC (permalink / raw)
To: X86 ML
Cc: linux-kernel@vger.kernel.org, Borislav Petkov, Linus Torvalds,
Andrew Morton, Mel Gorman, linux-mm@kvack.org, Nadav Amit,
Andy Lutomirski, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko, Arjan van de Ven
In-Reply-To: <cover.1495492063.git.luto@kernel.org>
On a remote TLB flush, we leave_mm() if we're TLBSTATE_LAZY. For a
local flush_tlb_mm_range(), we leave_mm() if !current->mm. These
are approximately the same condition -- the scheduler sets lazy TLB
mode when switching to a thread with no mm.
I'm about to merge the local and remote flush code, but for ease of
verifying and bisecting the patch, I want the local and remote flush
behavior to match first. This patch changes the local code to match
the remote code.
Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/mm/tlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 776469cc54e0..3143c9a180e5 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -311,7 +311,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
goto out;
}
- if (!current->mm) {
+ if (this_cpu_read(cpu_tlbstate.state) != TLBSTATE_OK) {
leave_mm(smp_processor_id());
/* Synchronize with switch_mm. */
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 04/11] x86/mm: Pass flush_tlb_info to flush_tlb_others() etc
From: Andy Lutomirski @ 2017-05-22 22:30 UTC (permalink / raw)
To: X86 ML
Cc: linux-kernel@vger.kernel.org, Borislav Petkov, Linus Torvalds,
Andrew Morton, Mel Gorman, linux-mm@kvack.org, Nadav Amit,
Andy Lutomirski, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko
In-Reply-To: <cover.1495492063.git.luto@kernel.org>
Rather than passing all the contents of flush_tlb_info to
flush_tlb_others(), pass a pointer to the structure directly. For
consistency, this also removes the unnecessary cpu parameter from
uv_flush_tlb_others() to make its signature match the other
*flush_tlb_others() functions.
This serves two purposes:
- It will dramatically simplify future patches that change struct
flush_tlb_info, which I'm planning to do.
- struct flush_tlb_info is an adequate description of what to do
for a local flush, too, so by reusing it we can remove duplicated
code between local and remove flushes in a future patch.
Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/include/asm/paravirt.h | 6 ++--
arch/x86/include/asm/paravirt_types.h | 5 ++-
arch/x86/include/asm/tlbflush.h | 19 ++++++-----
arch/x86/include/asm/uv/uv.h | 9 ++---
arch/x86/mm/tlb.c | 64 +++++++++++++++++------------------
arch/x86/platform/uv/tlb_uv.c | 10 +++---
arch/x86/xen/mmu.c | 10 +++---
7 files changed, 59 insertions(+), 64 deletions(-)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 55fa56fe4e45..9a15739d9f4b 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -312,11 +312,9 @@ static inline void __flush_tlb_single(unsigned long addr)
}
static inline void flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm,
- unsigned long start,
- unsigned long end)
+ const struct flush_tlb_info *info)
{
- PVOP_VCALL4(pv_mmu_ops.flush_tlb_others, cpumask, mm, start, end);
+ PVOP_VCALL2(pv_mmu_ops.flush_tlb_others, cpumask, info);
}
static inline int paravirt_pgd_alloc(struct mm_struct *mm)
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 7465d6fe336f..cb976bab6299 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -51,6 +51,7 @@ struct mm_struct;
struct desc_struct;
struct task_struct;
struct cpumask;
+struct flush_tlb_info;
/*
* Wrapper type for pointers to code which uses the non-standard
@@ -223,9 +224,7 @@ struct pv_mmu_ops {
void (*flush_tlb_kernel)(void);
void (*flush_tlb_single)(unsigned long addr);
void (*flush_tlb_others)(const struct cpumask *cpus,
- struct mm_struct *mm,
- unsigned long start,
- unsigned long end);
+ const struct flush_tlb_info *info);
/* Hooks for allocating and freeing a pagetable top-level */
int (*pgd_alloc)(struct mm_struct *mm);
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 8f6e2f87511b..6f439ac92026 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -220,12 +220,18 @@ static inline void __flush_tlb_one(unsigned long addr)
* - flush_tlb_page(vma, vmaddr) flushes one page
* - flush_tlb_range(vma, start, end) flushes a range of pages
* - flush_tlb_kernel_range(start, end) flushes a range of kernel pages
- * - flush_tlb_others(cpumask, mm, start, end) flushes TLBs on other cpus
+ * - flush_tlb_others(cpumask, info) flushes TLBs on other cpus
*
* ..but the i386 has somewhat limited tlb flushing capabilities,
* and page-granular flushes are available only on i486 and up.
*/
+struct flush_tlb_info {
+ struct mm_struct *mm;
+ unsigned long start;
+ unsigned long end;
+};
+
#ifndef CONFIG_SMP
/* "_up" is for UniProcessor.
@@ -279,9 +285,7 @@ static inline void flush_tlb_mm_range(struct mm_struct *mm,
}
static inline void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm,
- unsigned long start,
- unsigned long end)
+ const struct flush_tlb_info *info)
{
}
@@ -317,8 +321,7 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
}
void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm,
- unsigned long start, unsigned long end);
+ const struct flush_tlb_info *info);
#define TLBSTATE_OK 1
#define TLBSTATE_LAZY 2
@@ -340,8 +343,8 @@ extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
#endif /* SMP */
#ifndef CONFIG_PARAVIRT
-#define flush_tlb_others(mask, mm, start, end) \
- native_flush_tlb_others(mask, mm, start, end)
+#define flush_tlb_others(mask, info) \
+ native_flush_tlb_others(mask, info)
#endif
#endif /* _ASM_X86_TLBFLUSH_H */
diff --git a/arch/x86/include/asm/uv/uv.h b/arch/x86/include/asm/uv/uv.h
index 6686820feae9..d2db4517930c 100644
--- a/arch/x86/include/asm/uv/uv.h
+++ b/arch/x86/include/asm/uv/uv.h
@@ -15,10 +15,7 @@ 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,
- struct mm_struct *mm,
- unsigned long start,
- unsigned long end,
- unsigned int cpu);
+ const struct flush_tlb_info *info);
#else /* X86_UV */
@@ -28,8 +25,8 @@ 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, struct mm_struct *mm,
- unsigned long start, unsigned long end, unsigned int cpu)
+uv_flush_tlb_others(const struct cpumask *cpumask,
+ const struct flush_tlb_info *info)
{ return cpumask; }
#endif /* X86_UV */
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 743e4c6b4529..776469cc54e0 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -30,12 +30,6 @@
#ifdef CONFIG_SMP
-struct flush_tlb_info {
- struct mm_struct *flush_mm;
- unsigned long flush_start;
- unsigned long flush_end;
-};
-
/*
* We cannot call mmdrop() because we are in interrupt context,
* instead update mm->cpu_vm_mask.
@@ -229,11 +223,11 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
*/
static void flush_tlb_func(void *info)
{
- struct flush_tlb_info *f = info;
+ const struct flush_tlb_info *f = info;
inc_irq_stat(irq_tlb_count);
- if (f->flush_mm && f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm))
+ if (f->mm && f->mm != this_cpu_read(cpu_tlbstate.active_mm))
return;
count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
@@ -243,15 +237,15 @@ static void flush_tlb_func(void *info)
return;
}
- if (f->flush_end == TLB_FLUSH_ALL) {
+ if (f->end == TLB_FLUSH_ALL) {
local_flush_tlb();
trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
} else {
unsigned long addr;
unsigned long nr_pages =
- (f->flush_end - f->flush_start) / PAGE_SIZE;
- addr = f->flush_start;
- while (addr < f->flush_end) {
+ (f->end - f->start) / PAGE_SIZE;
+ addr = f->start;
+ while (addr < f->end) {
__flush_tlb_single(addr);
addr += PAGE_SIZE;
}
@@ -260,33 +254,27 @@ static void flush_tlb_func(void *info)
}
void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long start,
- unsigned long end)
+ const struct flush_tlb_info *info)
{
- struct flush_tlb_info info;
-
- info.flush_mm = mm;
- info.flush_start = start;
- info.flush_end = end;
-
count_vm_tlb_event(NR_TLB_REMOTE_FLUSH);
- if (end == TLB_FLUSH_ALL)
+ if (info->end == TLB_FLUSH_ALL)
trace_tlb_flush(TLB_REMOTE_SEND_IPI, TLB_FLUSH_ALL);
else
trace_tlb_flush(TLB_REMOTE_SEND_IPI,
- (end - start) >> PAGE_SHIFT);
+ (info->end - info->start) >> PAGE_SHIFT);
if (is_uv_system()) {
unsigned int cpu;
cpu = smp_processor_id();
- cpumask = uv_flush_tlb_others(cpumask, mm, start, end, cpu);
+ cpumask = uv_flush_tlb_others(cpumask, info);
if (cpumask)
smp_call_function_many(cpumask, flush_tlb_func,
- &info, 1);
+ (void *)info, 1);
return;
}
- smp_call_function_many(cpumask, flush_tlb_func, &info, 1);
+ smp_call_function_many(cpumask, flush_tlb_func,
+ (void *)info, 1);
}
/*
@@ -305,6 +293,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
unsigned long end, unsigned long vmflag)
{
unsigned long addr;
+ struct flush_tlb_info info;
/* do a global flush by default */
unsigned long base_pages_to_flush = TLB_FLUSH_ALL;
@@ -347,15 +336,20 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
}
trace_tlb_flush(TLB_LOCAL_MM_SHOOTDOWN, base_pages_to_flush);
out:
+ info.mm = mm;
if (base_pages_to_flush == TLB_FLUSH_ALL) {
- start = 0UL;
- end = TLB_FLUSH_ALL;
+ info.start = 0UL;
+ info.end = TLB_FLUSH_ALL;
+ } else {
+ info.start = start;
+ info.end = end;
}
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, start, end);
+ flush_tlb_others(mm_cpumask(mm), &info);
preempt_enable();
}
+
static void do_flush_tlb_all(void *info)
{
count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
@@ -376,7 +370,7 @@ static void do_kernel_range_flush(void *info)
unsigned long addr;
/* flush range by one by one 'invlpg' */
- for (addr = f->flush_start; addr < f->flush_end; addr += PAGE_SIZE)
+ for (addr = f->start; addr < f->end; addr += PAGE_SIZE)
__flush_tlb_single(addr);
}
@@ -389,14 +383,20 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end)
on_each_cpu(do_flush_tlb_all, NULL, 1);
} else {
struct flush_tlb_info info;
- info.flush_start = start;
- info.flush_end = end;
+ info.start = start;
+ info.end = end;
on_each_cpu(do_kernel_range_flush, &info, 1);
}
}
void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
{
+ struct flush_tlb_info info = {
+ .mm = NULL,
+ .start = 0UL,
+ .end = TLB_FLUSH_ALL,
+ };
+
int cpu = get_cpu();
if (cpumask_test_cpu(cpu, &batch->cpumask)) {
@@ -406,7 +406,7 @@ void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
}
if (cpumask_any_but(&batch->cpumask, cpu) < nr_cpu_ids)
- flush_tlb_others(&batch->cpumask, NULL, 0, TLB_FLUSH_ALL);
+ flush_tlb_others(&batch->cpumask, &info);
cpumask_clear(&batch->cpumask);
put_cpu();
diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index f25982cdff90..cf621e8a4ea9 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1109,11 +1109,9 @@ static int set_distrib_bits(struct cpumask *flush_mask, struct bau_control *bcp,
* done. The returned pointer is valid till preemption is re-enabled.
*/
const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm,
- unsigned long start,
- unsigned long end,
- unsigned int cpu)
+ const struct flush_tlb_info *info)
{
+ unsigned int cpu = smp_processor_id();
int locals = 0;
int remotes = 0;
int hubs = 0;
@@ -1170,8 +1168,8 @@ const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
record_send_statistics(stat, locals, hubs, remotes, bau_desc);
- if (!end || (end - start) <= PAGE_SIZE)
- bau_desc->payload.address = start;
+ if (!info->end || (info->end - info->start) <= PAGE_SIZE)
+ bau_desc->payload.address = info->start;
else
bau_desc->payload.address = TLB_FLUSH_ALL;
bau_desc->payload.sending_cpu = cpu;
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index f226038a39ca..894daefd6958 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1427,8 +1427,7 @@ static void xen_flush_tlb_single(unsigned long addr)
}
static void xen_flush_tlb_others(const struct cpumask *cpus,
- struct mm_struct *mm, unsigned long start,
- unsigned long end)
+ const struct flush_tlb_info *info)
{
struct {
struct mmuext_op op;
@@ -1440,7 +1439,7 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
} *args;
struct multicall_space mcs;
- trace_xen_mmu_flush_tlb_others(cpus, mm, start, end);
+ trace_xen_mmu_flush_tlb_others(cpus, info->mm, info->start, info->end);
if (cpumask_empty(cpus))
return; /* nothing to do */
@@ -1454,9 +1453,10 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));
args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- if (end != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
+ if (info->end != TLB_FLUSH_ALL &&
+ (info->end - info->start) <= PAGE_SIZE) {
args->op.cmd = MMUEXT_INVLPG_MULTI;
- args->op.arg1.linear_addr = start;
+ args->op.arg1.linear_addr = info->start;
}
MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 03/11] x86/mm: Make the batched unmap TLB flush API more generic
From: Andy Lutomirski @ 2017-05-22 22:30 UTC (permalink / raw)
To: X86 ML
Cc: linux-kernel@vger.kernel.org, Borislav Petkov, Linus Torvalds,
Andrew Morton, Mel Gorman, linux-mm@kvack.org, Nadav Amit,
Andy Lutomirski, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko, Sasha Levin
In-Reply-To: <cover.1495492063.git.luto@kernel.org>
try_to_unmap_flush() used to open-code a rather x86-centric flush
sequence: local_flush_tlb() + flush_tlb_others(). Rearrange the
code so that the arch (only x86 for now) provides
arch_tlbbatch_add_mm() and arch_tlbbatch_flush() and the core code
calls those functions instead.
I'll want this for x86 because, to enable address space ids, I can't
support the flush_tlb_others() mode used by exising
try_to_unmap_flush() implementation with good performance. I can
support the new API fairly easily, though.
I imagine that other architectures may be in a similar position.
Architectures with strong remote flush primitives (arm64?) may have
even worse performance problems with flush_tlb_others() the way that
try_to_unmap_flush() uses it.
Cc: Mel Gorman <mgorman@suse.de>
Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/include/asm/tlbbatch.h | 16 ++++++++++++++++
arch/x86/include/asm/tlbflush.h | 8 ++++++++
arch/x86/mm/tlb.c | 17 +++++++++++++++++
include/linux/mm_types_task.h | 15 +++++++++++----
mm/rmap.c | 16 ++--------------
5 files changed, 54 insertions(+), 18 deletions(-)
create mode 100644 arch/x86/include/asm/tlbbatch.h
diff --git a/arch/x86/include/asm/tlbbatch.h b/arch/x86/include/asm/tlbbatch.h
new file mode 100644
index 000000000000..01a6de16fb96
--- /dev/null
+++ b/arch/x86/include/asm/tlbbatch.h
@@ -0,0 +1,16 @@
+#ifndef _ARCH_X86_TLBBATCH_H
+#define _ARCH_X86_TLBBATCH_H
+
+#include <linux/cpumask.h>
+
+#ifdef CONFIG_SMP
+struct arch_tlbflush_unmap_batch {
+ /*
+ * Each bit set is a CPU that potentially has a TLB entry for one of
+ * the PFNs being flushed..
+ */
+ struct cpumask cpumask;
+};
+#endif
+
+#endif /* _ARCH_X86_TLBBATCH_H */
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index b9db0f8fef55..8f6e2f87511b 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -329,6 +329,14 @@ static inline void reset_lazy_tlbstate(void)
this_cpu_write(cpu_tlbstate.active_mm, &init_mm);
}
+static inline void arch_tlbbatch_add_mm(struct arch_tlbflush_unmap_batch *batch,
+ struct mm_struct *mm)
+{
+ cpumask_or(&batch->cpumask, &batch->cpumask, mm_cpumask(mm));
+}
+
+extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
+
#endif /* SMP */
#ifndef CONFIG_PARAVIRT
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 4d303864b310..743e4c6b4529 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -395,6 +395,23 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end)
}
}
+void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
+{
+ int cpu = get_cpu();
+
+ if (cpumask_test_cpu(cpu, &batch->cpumask)) {
+ count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
+ local_flush_tlb();
+ trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
+ }
+
+ if (cpumask_any_but(&batch->cpumask, cpu) < nr_cpu_ids)
+ flush_tlb_others(&batch->cpumask, NULL, 0, TLB_FLUSH_ALL);
+ cpumask_clear(&batch->cpumask);
+
+ put_cpu();
+}
+
static ssize_t tlbflush_read_file(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h
index 136dfdf63ba1..fc412fbd80bd 100644
--- a/include/linux/mm_types_task.h
+++ b/include/linux/mm_types_task.h
@@ -14,6 +14,10 @@
#include <asm/page.h>
+#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
+#include <asm/tlbbatch.h>
+#endif
+
#define USE_SPLIT_PTE_PTLOCKS (NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS)
#define USE_SPLIT_PMD_PTLOCKS (USE_SPLIT_PTE_PTLOCKS && \
IS_ENABLED(CONFIG_ARCH_ENABLE_SPLIT_PMD_PTLOCK))
@@ -67,12 +71,15 @@ struct page_frag {
struct tlbflush_unmap_batch {
#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
/*
- * Each bit set is a CPU that potentially has a TLB entry for one of
- * the PFNs being flushed. See set_tlb_ubc_flush_pending().
+ * The arch code makes the following promise: generic code can modify a
+ * PTE, then call arch_tlbbatch_add_mm() (which internally provides all
+ * needed barriers), then call arch_tlbbatch_flush(), and the entries
+ * will be flushed on all CPUs by the time that arch_tlbbatch_flush()
+ * returns.
*/
- struct cpumask cpumask;
+ struct arch_tlbflush_unmap_batch arch;
- /* True if any bit in cpumask is set */
+ /* True if a flush is needed. */
bool flush_required;
/*
diff --git a/mm/rmap.c b/mm/rmap.c
index f6838015810f..f29d725528d2 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -579,25 +579,13 @@ void page_unlock_anon_vma_read(struct anon_vma *anon_vma)
void try_to_unmap_flush(void)
{
struct tlbflush_unmap_batch *tlb_ubc = ¤t->tlb_ubc;
- int cpu;
if (!tlb_ubc->flush_required)
return;
- cpu = get_cpu();
-
- if (cpumask_test_cpu(cpu, &tlb_ubc->cpumask)) {
- count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
- local_flush_tlb();
- trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
- }
-
- if (cpumask_any_but(&tlb_ubc->cpumask, cpu) < nr_cpu_ids)
- flush_tlb_others(&tlb_ubc->cpumask, NULL, 0, TLB_FLUSH_ALL);
- cpumask_clear(&tlb_ubc->cpumask);
+ arch_tlbbatch_flush(&tlb_ubc->arch);
tlb_ubc->flush_required = false;
tlb_ubc->writable = false;
- put_cpu();
}
/* Flush iff there are potentially writable TLB entries that can race with IO */
@@ -613,7 +601,7 @@ static void set_tlb_ubc_flush_pending(struct mm_struct *mm, bool writable)
{
struct tlbflush_unmap_batch *tlb_ubc = ¤t->tlb_ubc;
- cpumask_or(&tlb_ubc->cpumask, &tlb_ubc->cpumask, mm_cpumask(mm));
+ arch_tlbbatch_add_mm(&tlb_ubc->arch, mm);
tlb_ubc->flush_required = true;
/*
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 02/11] x86/mm: Reduce indentation in flush_tlb_func()
From: Andy Lutomirski @ 2017-05-22 22:30 UTC (permalink / raw)
To: X86 ML
Cc: linux-kernel@vger.kernel.org, Borislav Petkov, Linus Torvalds,
Andrew Morton, Mel Gorman, linux-mm@kvack.org, Nadav Amit,
Andy Lutomirski, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko
In-Reply-To: <cover.1495492063.git.luto@kernel.org>
The leave_mm() case can just exit the function early so we don't
need to indent the entire remainder of the function.
Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/mm/tlb.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index fe6471132ea3..4d303864b310 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -237,24 +237,26 @@ static void flush_tlb_func(void *info)
return;
count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
- if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
- if (f->flush_end == TLB_FLUSH_ALL) {
- local_flush_tlb();
- trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
- } else {
- unsigned long addr;
- unsigned long nr_pages =
- (f->flush_end - f->flush_start) / PAGE_SIZE;
- addr = f->flush_start;
- while (addr < f->flush_end) {
- __flush_tlb_single(addr);
- addr += PAGE_SIZE;
- }
- trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, nr_pages);
- }
- } else
+
+ if (this_cpu_read(cpu_tlbstate.state) != TLBSTATE_OK) {
leave_mm(smp_processor_id());
+ return;
+ }
+ if (f->flush_end == TLB_FLUSH_ALL) {
+ local_flush_tlb();
+ trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
+ } else {
+ unsigned long addr;
+ unsigned long nr_pages =
+ (f->flush_end - f->flush_start) / PAGE_SIZE;
+ addr = f->flush_start;
+ while (addr < f->flush_end) {
+ __flush_tlb_single(addr);
+ addr += PAGE_SIZE;
+ }
+ trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, nr_pages);
+ }
}
void native_flush_tlb_others(const struct cpumask *cpumask,
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 01/11] x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range()
From: Andy Lutomirski @ 2017-05-22 22:30 UTC (permalink / raw)
To: X86 ML
Cc: linux-kernel@vger.kernel.org, Borislav Petkov, Linus Torvalds,
Andrew Morton, Mel Gorman, linux-mm@kvack.org, Nadav Amit,
Andy Lutomirski, Rik van Riel, Dave Hansen, Nadav Amit,
Michal Hocko
In-Reply-To: <cover.1495492063.git.luto@kernel.org>
flush_tlb_page() was very similar to flush_tlb_mm_range() except that
it had a couple of issues:
- It was missing an smp_mb() in the case where
current->active_mm != mm. (This is a longstanding bug reported by
Nadav Amit.)
- It was missing tracepoints and vm counter updates.
The only reason that I can see for keeping it at as a separate
function is that it could avoid a few branches that
flush_tlb_mm_range() needs to decide to flush just one page. This
hardly seems worthwhile. If we decide we want to get rid of those
branches again, a better way would be to introduce an
__flush_tlb_mm_range() helper and make both flush_tlb_page() and
flush_tlb_mm_range() use it.
Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/include/asm/tlbflush.h | 6 +++++-
arch/x86/mm/tlb.c | 27 ---------------------------
2 files changed, 5 insertions(+), 28 deletions(-)
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 6ed9ea469b48..b9db0f8fef55 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -307,11 +307,15 @@ static inline void flush_tlb_kernel_range(unsigned long start,
flush_tlb_mm_range(vma->vm_mm, start, end, vma->vm_flags)
extern void flush_tlb_all(void);
-extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
extern void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
unsigned long end, unsigned long vmflag);
extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
+static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
+{
+ flush_tlb_mm_range(vma->vm_mm, a, a + PAGE_SIZE, VM_NONE);
+}
+
void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
unsigned long start, unsigned long end);
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 6e7bedf69af7..fe6471132ea3 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -354,33 +354,6 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
preempt_enable();
}
-void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
-{
- struct mm_struct *mm = vma->vm_mm;
-
- preempt_disable();
-
- if (current->active_mm == mm) {
- if (current->mm) {
- /*
- * Implicit full barrier (INVLPG) that synchronizes
- * with switch_mm.
- */
- __flush_tlb_one(start);
- } else {
- leave_mm(smp_processor_id());
-
- /* Synchronize with switch_mm. */
- smp_mb();
- }
- }
-
- if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, start, start + PAGE_SIZE);
-
- preempt_enable();
-}
-
static void do_flush_tlb_all(void *info)
{
count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 00/11] x86 TLB flush cleanups, moving toward PCID support
From: Andy Lutomirski @ 2017-05-22 22:30 UTC (permalink / raw)
To: X86 ML
Cc: linux-kernel@vger.kernel.org, Borislav Petkov, Linus Torvalds,
Andrew Morton, Mel Gorman, linux-mm@kvack.org, Nadav Amit,
Andy Lutomirski
As I've been working on polishing my PCID code, a major problem I've
encountered is that there are too many x86 TLB flushing code paths and
that they have too many inconsequential differences. The result was
that earlier versions of the PCID code were a colossal mess and very
difficult to understand.
This series goes a long way toward cleaning up the mess. With all the
patches applied, there is a single function that contains the meat of
the code to flush the TLB on a given CPU, and all the tlb flushing
APIs call it for both local and remote CPUs.
This series should only adversely affect the kernel in a couple of
minor ways:
- It makes smp_mb() unconditional when flushing TLBs. We used to
use the TLB flush itself to mostly avoid smp_mb() on the initiating
CPU.
- On UP kernels, we lose the dubious optimization of inlining nerfed
variants of all the TLB flush APIs. This bloats the kernel a tiny
bit, although it should increase performance, since the SMP
versions were better.
Patch 10 in here is a little bit off topic. It's a cleanup that's
also needed before PCID can go in, but it's not directly about
TLB flushing.
Changes from RFC:
- Fixed missing call to arch_tlbbatch_flush().
- "Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush code" is new
- Misc typos fixed.
- Actually compiles when UV is enabled.
Andy Lutomirski (11):
x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range()
x86/mm: Reduce indentation in flush_tlb_func()
x86/mm: Make the batched unmap TLB flush API more generic
x86/mm: Pass flush_tlb_info to flush_tlb_others() etc
x86/mm: Change the leave_mm() condition for local TLB flushes
x86/mm: Refactor flush_tlb_mm_range() to merge local and remote cases
x86/mm: Use new merged flush logic in arch_tlbbatch_flush()
x86/mm: Remove the UP tlbflush code; always use the formerly SMP code
x86/mm: Rework lazy TLB to track the actual loaded mm
x86/mm: Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush
code
x86,kvm: Teach KVM's VMX code that CR3 isn't a constant
arch/x86/Kconfig | 2 +-
arch/x86/events/core.c | 3 +-
arch/x86/include/asm/hardirq.h | 2 +-
arch/x86/include/asm/mmu.h | 6 -
arch/x86/include/asm/mmu_context.h | 21 +-
arch/x86/include/asm/paravirt.h | 6 +-
arch/x86/include/asm/paravirt_types.h | 5 +-
arch/x86/include/asm/tlbbatch.h | 14 ++
arch/x86/include/asm/tlbflush.h | 116 +++------
arch/x86/include/asm/uv/uv.h | 9 +-
arch/x86/kernel/ldt.c | 7 +-
arch/x86/kvm/vmx.c | 21 +-
arch/x86/mm/init.c | 4 +-
arch/x86/mm/tlb.c | 429 +++++++++++++++-------------------
arch/x86/platform/uv/tlb_uv.c | 10 +-
arch/x86/xen/mmu.c | 61 +++--
include/linux/mm_types_task.h | 15 +-
mm/rmap.c | 16 +-
18 files changed, 336 insertions(+), 411 deletions(-)
create mode 100644 arch/x86/include/asm/tlbbatch.h
--
2.9.3
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH v2 2/2] dax: Fix race between colliding PMD & PTE entries
From: Ross Zwisler @ 2017-05-22 21:57 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Ross Zwisler, Darrick J. Wong, Alexander Viro, Christoph Hellwig,
Dan Williams, Dave Hansen, Jan Kara, Matthew Wilcox,
linux-fsdevel, linux-mm, linux-nvdimm, Kirill A . Shutemov,
Pawel Lebioda, Dave Jiang, Xiong Zhou, Eryu Guan, stable
In-Reply-To: <20170522215749.23516-1-ross.zwisler@linux.intel.com>
We currently have two related PMD vs PTE races in the DAX code. These can
both be easily triggered by having two threads reading and writing
simultaneously to the same private mapping, with the key being that private
mapping reads can be handled with PMDs but private mapping writes are
always handled with PTEs so that we can COW.
Here is the first race:
CPU 0 CPU 1
(private mapping write)
__handle_mm_fault()
create_huge_pmd() - FALLBACK
handle_pte_fault()
passes check for pmd_devmap()
(private mapping read)
__handle_mm_fault()
create_huge_pmd()
dax_iomap_pmd_fault() inserts PMD
dax_iomap_pte_fault() does a PTE fault, but we already have a DAX PMD
installed in our page tables at this spot.
Here's the second race:
CPU 0 CPU 1
(private mapping read)
__handle_mm_fault()
passes check for pmd_none()
create_huge_pmd()
dax_iomap_pmd_fault() inserts PMD
(private mapping write)
__handle_mm_fault()
create_huge_pmd() - FALLBACK
(private mapping read)
__handle_mm_fault()
passes check for pmd_none()
create_huge_pmd()
handle_pte_fault()
dax_iomap_pte_fault() inserts PTE
dax_iomap_pmd_fault() inserts PMD,
but we already have a PTE at
this spot.
The core of the issue is that while there is isolation between faults to
the same range in the DAX fault handlers via our DAX entry locking, there
is no isolation between faults in the code in mm/memory.c. This means for
instance that this code in __handle_mm_fault() can run:
if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
ret = create_huge_pmd(&vmf);
But by the time we actually get to run the fault handler called by
create_huge_pmd(), the PMD is no longer pmd_none() because a racing PTE
fault has installed a normal PMD here as a parent. This is the cause of
the 2nd race. The first race is similar - there is the following check in
handle_pte_fault():
} else {
/* See comment in pte_alloc_one_map() */
if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
return 0;
So if a pmd_devmap() PMD (a DAX PMD) has been installed at vmf->pmd, we
will bail and retry the fault. This is correct, but there is nothing
preventing the PMD from being installed after this check but before we
actually get to the DAX PTE fault handlers.
In my testing these races result in the following types of errors:
BUG: Bad rss-counter state mm:ffff8800a817d280 idx:1 val:1
BUG: non-zero nr_ptes on freeing mm: 15
Fix this issue by having the DAX fault handlers verify that it is safe to
continue their fault after they have taken an entry lock to block other
racing faults.
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: Pawel Lebioda <pawel.lebioda@intel.com>
Cc: stable@vger.kernel.org
---
Changes from v1:
- Handle the failure case in dax_iomap_pte_fault() by retrying the fault
(Jan).
This series has survived my new xfstest (generic/437) and full xfstest
regression testing runs.
---
fs/dax.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/fs/dax.c b/fs/dax.c
index c22eaf1..fc62f36 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1155,6 +1155,17 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
}
/*
+ * It is possible, particularly with mixed reads & writes to private
+ * mappings, that we have raced with a PMD fault that overlaps with
+ * the PTE we need to set up. If so just return and the fault will be
+ * retried.
+ */
+ if (pmd_devmap(*vmf->pmd)) {
+ vmf_ret = VM_FAULT_NOPAGE;
+ goto unlock_entry;
+ }
+
+ /*
* Note that we don't bother to use iomap_apply here: DAX required
* the file system block size to be equal the page size, which means
* that we never have to deal with more than a single extent here.
@@ -1398,6 +1409,15 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf,
goto fallback;
/*
+ * It is possible, particularly with mixed reads & writes to private
+ * mappings, that we have raced with a PTE fault that overlaps with
+ * the PMD we need to set up. If so we just fall back to a PTE fault
+ * ourselves.
+ */
+ if (!pmd_none(*vmf->pmd))
+ goto unlock_entry;
+
+ /*
* Note that we don't use iomap_apply here. We aren't doing I/O, only
* setting up a mapping, so really we're using iomap_begin() as a way
* to look up our filesystem block.
--
2.9.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v2 1/2] mm: avoid spurious 'bad pmd' warning messages
From: Ross Zwisler @ 2017-05-22 21:57 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Ross Zwisler, Darrick J. Wong, Alexander Viro, Christoph Hellwig,
Dan Williams, Dave Hansen, Jan Kara, Matthew Wilcox,
linux-fsdevel, linux-mm, linux-nvdimm, Kirill A . Shutemov,
Pawel Lebioda, Dave Jiang, Xiong Zhou, Eryu Guan, stable
When the pmd_devmap() checks were added by:
commit 5c7fb56e5e3f ("mm, dax: dax-pmd vs thp-pmd vs hugetlbfs-pmd")
to add better support for DAX huge pages, they were all added to the end of
if() statements after existing pmd_trans_huge() checks. So, things like:
- if (pmd_trans_huge(*pmd))
+ if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd))
When further checks were added after pmd_trans_unstable() checks by:
commit 7267ec008b5c ("mm: postpone page table allocation until we have page
to map")
they were also added at the end of the conditional:
+ if (pmd_trans_unstable(fe->pmd) || pmd_devmap(*fe->pmd))
This ordering is fine for pmd_trans_huge(), but doesn't work for
pmd_trans_unstable(). This is because DAX huge pages trip the bad_pmd()
check inside of pmd_none_or_trans_huge_or_clear_bad() (called by
pmd_trans_unstable()), which prints out a warning and returns 1. So, we do
end up doing the right thing, but only after spamming dmesg with suspicious
looking messages:
mm/pgtable-generic.c:39: bad pmd ffff8808daa49b88(84000001006000a5)
Reorder these checks in a helper so that pmd_devmap() is checked first,
avoiding the error messages, and add a comment explaining why the ordering
is important.
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Fixes: commit 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
Cc: stable@vger.kernel.org
---
Changes from v1:
- Break the checks out into the new pmd_devmap_trans_unstable() helper and
add a comment about the ordering (Dave). I ended up keeping this helper
in mm/memory.c because I didn't see an obvious header where it would
live happily. pmd_devmap() is either defined in
arch/x86/include/asm/pgtable.h or in include/linux/mm.h depending on
__HAVE_ARCH_PTE_DEVMAP and CONFIG_TRANSPARENT_HUGEPAGE, and
pmd_trans_unstable() is defined in include/asm-generic/pgtable.h.
- Add a comment explaining why pte_alloc_one_map() doesn't suffer from races.
This was the result of a conversation with Dave Hansen.
---
mm/memory.c | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 6ff5d72..2e65df1 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3029,6 +3029,17 @@ static int __do_fault(struct vm_fault *vmf)
return ret;
}
+/*
+ * The ordering of these checks is important for pmds with _PAGE_DEVMAP set.
+ * If we check pmd_trans_unstable() first we will trip the bad_pmd() check
+ * inside of pmd_none_or_trans_huge_or_clear_bad(). This will end up correctly
+ * returning 1 but not before it spams dmesg with the pmd_clear_bad() output.
+ */
+static int pmd_devmap_trans_unstable(pmd_t *pmd)
+{
+ return pmd_devmap(*pmd) || pmd_trans_unstable(pmd);
+}
+
static int pte_alloc_one_map(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
@@ -3052,18 +3063,27 @@ static int pte_alloc_one_map(struct vm_fault *vmf)
map_pte:
/*
* If a huge pmd materialized under us just retry later. Use
- * pmd_trans_unstable() instead of pmd_trans_huge() to ensure the pmd
- * didn't become pmd_trans_huge under us and then back to pmd_none, as
- * a result of MADV_DONTNEED running immediately after a huge pmd fault
- * in a different thread of this mm, in turn leading to a misleading
- * pmd_trans_huge() retval. All we have to ensure is that it is a
- * regular pmd that we can walk with pte_offset_map() and we can do that
- * through an atomic read in C, which is what pmd_trans_unstable()
- * provides.
+ * pmd_trans_unstable() via pmd_devmap_trans_unstable() instead of
+ * pmd_trans_huge() to ensure the pmd didn't become pmd_trans_huge
+ * under us and then back to pmd_none, as a result of MADV_DONTNEED
+ * running immediately after a huge pmd fault in a different thread of
+ * this mm, in turn leading to a misleading pmd_trans_huge() retval.
+ * All we have to ensure is that it is a regular pmd that we can walk
+ * with pte_offset_map() and we can do that through an atomic read in
+ * C, which is what pmd_trans_unstable() provides.
*/
- if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
+ if (pmd_devmap_trans_unstable(vmf->pmd))
return VM_FAULT_NOPAGE;
+ /*
+ * At this point we know that our vmf->pmd points to a page of ptes
+ * and it cannot become pmd_none(), pmd_devmap() or pmd_trans_huge()
+ * for the duration of the fault. If a racing MADV_DONTNEED runs and
+ * we zap the ptes pointed to by our vmf->pmd, the vmf->ptl will still
+ * be valid and we will re-check to make sure the vmf->pte isn't
+ * pte_none() under vmf->ptl protection when we return to
+ * alloc_set_pte().
+ */
vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
&vmf->ptl);
return 0;
@@ -3690,7 +3710,7 @@ static int handle_pte_fault(struct vm_fault *vmf)
vmf->pte = NULL;
} else {
/* See comment in pte_alloc_one_map() */
- if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
+ if (pmd_devmap_trans_unstable(vmf->pmd))
return 0;
/*
* A regular pmd is established and it can't morph into a huge
--
2.9.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
From: Andrew Morton @ 2017-05-22 21:45 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David Rientjes, Christoph Lameter, Pekka Enberg, Joonsoo Kim,
linux-mm, linux-kernel
In-Reply-To: <20170522205621.GL141096@google.com>
On Mon, 22 May 2017 13:56:21 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:
> El Mon, May 22, 2017 at 01:39:26PM -0700 David Rientjes ha dit:
>
> > On Fri, 19 May 2017, Matthias Kaehlcke wrote:
> >
> > > The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> > > block fixes the following warning when building with clang:
> > >
> > > mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> > > [-Werror,-Wunused-function]
> > >
> >
> > Is clang not inlining kmalloc_large_node_hook() for some reason? I don't
> > think this should ever warn on gcc.
>
> clang warns about unused static inline functions outside of header
> files, in difference to gcc.
I wish it wouldn't. These patches just add clutter.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/1] Sealable memory support
From: Kees Cook @ 2017-05-22 21:38 UTC (permalink / raw)
To: Igor Stoppa
Cc: Michal Hocko, Dave Hansen, Laura Abbott, Linux-MM,
kernel-hardening@lists.openwall.com, LKML, Daniel Micay
In-Reply-To: <20170519103811.2183-2-igor.stoppa@huawei.com>
On Fri, May 19, 2017 at 3:38 AM, Igor Stoppa <igor.stoppa@huawei.com> wrote:
> Dynamically allocated variables can be made read only,
> after they have been initialized, provided that they reside in memory
> pages devoid of any RW data.
>
> The implementation supplies means to create independent pools of memory,
> which can be individually created, sealed/unsealed and destroyed.
This would be a welcome addition, thanks for posting it! I have a
bunch of feedback, here and below:
For the first bit of bikeshedding, should this really be called
seal/unseal? My mind is probably just broken from having read TPM
documentation, but this isn't really "sealing" as I'd understand it
(it's not tied to a credential, for example). It's "only" rw/ro.
Perhaps "protect/unprotect" or just simply "readonly/writable", and
call the base function "romalloc"?
This is fundamentally a heap allocator, with linked lists, etc. I'd
like to see as much attention as possible given to hardening it
against attacks, especially adding redzoning around the metadata at
least, and perhaps requiring that CONFIG_DEBUG_LIST be enabled. And as
part of that, I'd like hardened usercopy to grow knowledge of these
allocations so we can bounds-check objects. Right now, mm/usercopy.c
just looks at PageSlab(page) to decide if it should do slab checks. I
think adding a check for this type of object would be very important
there.
The ro/rw granularity here is the _entire_ pool, not a specific
allocation (or page containing the allocation). I'm concerned that
makes this very open to race conditions where, especially in the
global pool, one thread can be trying to write to ro data in a pool
and another has made the pool writable.
> A global pool is made available for those kernel modules that do not
> need to manage an independent pool.
>
> Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
> ---
> mm/Makefile | 2 +-
> mm/smalloc.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> mm/smalloc.h | 61 ++++++++++++++++++
> 3 files changed, 262 insertions(+), 1 deletion(-)
> create mode 100644 mm/smalloc.c
> create mode 100644 mm/smalloc.h
>
> diff --git a/mm/Makefile b/mm/Makefile
> index 026f6a8..737c42a 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -39,7 +39,7 @@ obj-y := filemap.o mempool.o oom_kill.o \
> mm_init.o mmu_context.o percpu.o slab_common.o \
> compaction.o vmacache.o swap_slots.o \
> interval_tree.o list_lru.o workingset.o \
> - debug.o $(mmu-y)
> + debug.o smalloc.o $(mmu-y)
>
> obj-y += init-mm.o
>
> diff --git a/mm/smalloc.c b/mm/smalloc.c
> new file mode 100644
> index 0000000..fa04cc5
> --- /dev/null
> +++ b/mm/smalloc.c
> @@ -0,0 +1,200 @@
> +/*
> + * smalloc.c: Sealable Memory Allocator
> + *
> + * (C) Copyright 2017 Huawei Technologies Co. Ltd.
> + * Author: Igor Stoppa <igor.stoppa@huawei.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +#include <linux/init.h>
> +#include <linux/fs.h>
> +#include <linux/string.h>
> +
> +
> +#include <linux/vmalloc.h>
> +#include <asm/cacheflush.h>
> +#include "smalloc.h"
Shouldn't this just be <linux/smalloc.h> ?
> +#define page_roundup(size) (((size) + !(size) - 1 + PAGE_SIZE) & PAGE_MASK)
> +
> +#define pages_nr(size) (page_roundup(size) / PAGE_SIZE)
> +
> +static struct smalloc_pool *global_pool;
> +
> +struct smalloc_node *__smalloc_create_node(unsigned long words)
> +{
> + struct smalloc_node *node;
> + unsigned long size;
> +
> + /* Calculate the size to ask from vmalloc, page aligned. */
> + size = page_roundup(NODE_HEADER_SIZE + words * sizeof(align_t));
> + node = vmalloc(size);
> + if (!node) {
> + pr_err("No memory for allocating smalloc node.");
> + return NULL;
> + }
> + /* Initialize the node.*/
> + INIT_LIST_HEAD(&node->list);
> + node->free = node->data;
> + node->available_words = (size - NODE_HEADER_SIZE) / sizeof(align_t);
> + return node;
> +}
> +
> +static __always_inline
> +void *node_alloc(struct smalloc_node *node, unsigned long words)
> +{
> + register align_t *old_free = node->free;
> +
> + node->available_words -= words;
> + node->free += words;
> + return old_free;
> +}
> +
> +void *smalloc(unsigned long size, struct smalloc_pool *pool)
> +{
> + struct list_head *pos;
> + struct smalloc_node *node;
> + void *ptr;
> + unsigned long words;
> +
> + /* If no pool specified, use the global one. */
> + if (!pool)
> + pool = global_pool;
It should be impossible, but this should check for global_pool == NULL too, IMO.
> + mutex_lock(&pool->lock);
> +
> + /* If the pool is sealed, then return NULL. */
> + if (pool->seal == SMALLOC_SEALED) {
> + mutex_unlock(&pool->lock);
> + return NULL;
> + }
> +
> + /* Calculate minimum number of words required. */
> + words = (size + sizeof(align_t) - 1) / sizeof(align_t);
> +
> + /* Look for slot that is large enough, in the existing pool.*/
> + list_for_each(pos, &pool->list) {
> + node = list_entry(pos, struct smalloc_node, list);
> + if (node->available_words >= words) {
> + ptr = node_alloc(node, words);
> + mutex_unlock(&pool->lock);
> + return ptr;
> + }
> + }
> +
> + /* No slot found, get a new chunk of virtual memory. */
> + node = __smalloc_create_node(words);
> + if (!node) {
> + mutex_unlock(&pool->lock);
> + return NULL;
> + }
> +
> + list_add(&node->list, &pool->list);
> + ptr = node_alloc(node, words);
> + mutex_unlock(&pool->lock);
> + return ptr;
> +}
> +
> +static __always_inline
> +unsigned long get_node_size(struct smalloc_node *node)
> +{
> + if (!node)
> + return 0;
> + return page_roundup((((void *)node->free) - (void *)node) +
> + node->available_words * sizeof(align_t));
> +}
> +
> +static __always_inline
> +unsigned long get_node_pages_nr(struct smalloc_node *node)
> +{
> + return pages_nr(get_node_size(node));
> +}
> +void smalloc_seal_set(enum seal_t seal, struct smalloc_pool *pool)
> +{
> + struct list_head *pos;
> + struct smalloc_node *node;
> +
> + if (!pool)
> + pool = global_pool;
> + mutex_lock(&pool->lock);
> + if (pool->seal == seal) {
> + mutex_unlock(&pool->lock);
> + return;
I actually think this should be a BUG condition, since this means a
mismatched seal/unseal happened. The pool should never be left
writable at rest, a user should create a pool, write to it, seal. Any
updates should unseal, write, seal. To attempt an unseal and find it
already unsealed seems bad.
Finding a user for this would help clarify its protection properties,
too. (The LSM example is likely not the best starting point for that,
as it would depend on other changes that are under discussion.)
> + }
> + list_for_each(pos, &pool->list) {
> + node = list_entry(pos, struct smalloc_node, list);
> + if (seal == SMALLOC_SEALED)
> + set_memory_ro((unsigned long)node,
> + get_node_pages_nr(node));
> + else if (seal == SMALLOC_UNSEALED)
> + set_memory_rw((unsigned long)node,
> + get_node_pages_nr(node));
> + }
> + pool->seal = seal;
> + mutex_unlock(&pool->lock);
> +}
> +
> +int smalloc_initialize(struct smalloc_pool *pool)
> +{
> + if (!pool)
> + return -EINVAL;
> + INIT_LIST_HEAD(&pool->list);
> + pool->seal = SMALLOC_UNSEALED;
> + mutex_init(&pool->lock);
> + return 0;
> +}
> +
> +struct smalloc_pool *smalloc_create(void)
> +{
> + struct smalloc_pool *pool = vmalloc(sizeof(struct smalloc_pool));
> +
> + if (!pool) {
> + pr_err("No memory for allocating pool.");
It might be handy to have pools named like they are for the slab allocator.
> + return NULL;
> + }
> + smalloc_initialize(pool);
> + return pool;
> +}
> +
> +int smalloc_destroy(struct smalloc_pool *pool)
> +{
> + struct list_head *pos, *q;
> + struct smalloc_node *node;
> +
> + if (!pool)
> + return -EINVAL;
> + list_for_each_safe(pos, q, &pool->list) {
> + node = list_entry(pos, struct smalloc_node, list);
> + list_del(pos);
> + vfree(node);
> + }
> + return 0;
> +}
> +
> +static int __init smalloc_init(void)
> +{
> + global_pool = smalloc_create();
> + if (!global_pool) {
> + pr_err("Module smalloc initialization failed: no memory.\n");
> + return -ENOMEM;
> + }
> + pr_info("Module smalloc initialized successfully.\n");
> + return 0;
> +}
> +
> +static void __exit smalloc_exit(void)
> +{
> + pr_info("Module smalloc un initialized successfully.\n");
typo: space after "un"
> +}
> +
> +module_init(smalloc_init);
> +module_exit(smalloc_exit);
> +MODULE_LICENSE("GPL");
> diff --git a/mm/smalloc.h b/mm/smalloc.h
> new file mode 100644
> index 0000000..344d962
> --- /dev/null
> +++ b/mm/smalloc.h
> @@ -0,0 +1,61 @@
> +/*
> + * smalloc.h: Header for Sealable Memory Allocator
> + *
> + * (C) Copyright 2017 Huawei Technologies Co. Ltd.
> + * Author: Igor Stoppa <igor.stoppa@huawei.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +#ifndef _SMALLOC_H
> +#define _SMALLOC_H
> +
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +
> +typedef uint64_t align_t;
> +
> +enum seal_t {
> + SMALLOC_UNSEALED,
> + SMALLOC_SEALED,
> +};
> +
> +#define __SMALLOC_ALIGNED__ __aligned(sizeof(align_t))
> +
> +#define NODE_HEADER \
> + struct { \
> + __SMALLOC_ALIGNED__ struct { \
> + struct list_head list; \
> + align_t *free; \
> + unsigned long available_words; \
> + }; \
> + }
> +
> +#define NODE_HEADER_SIZE sizeof(NODE_HEADER)
> +
> +struct smalloc_pool {
> + struct list_head list;
> + struct mutex lock;
> + enum seal_t seal;
> +};
> +
> +struct smalloc_node {
> + NODE_HEADER;
> + __SMALLOC_ALIGNED__ align_t data[];
> +};
> +
> +#define smalloc_seal(pool) \
> + smalloc_seal_set(SMALLOC_SEALED, pool)
> +
> +#define smalloc_unseal(pool) \
> + smalloc_seal_set(SMALLOC_UNSEALED, pool)
> +
> +struct smalloc_pool *smalloc_create(void);
> +int smalloc_destroy(struct smalloc_pool *pool);
> +int smalloc_initialize(struct smalloc_pool *pool);
> +void *smalloc(unsigned long size, struct smalloc_pool *pool);
> +void smalloc_seal_set(enum seal_t seal, struct smalloc_pool *pool);
I'd really like to see kernel-doc for the API functions (likely in the .c file).
> +#endif
> --
> 2.9.3
Thanks again for working on this! If you can find examples of file
operations living in the heap, those would be great examples for using
this API (assuming the other properties can be improved).
-Kees
--
Kees Cook
Pixel Security
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 07/15] mm/ZONE_DEVICE: new type of ZONE_DEVICE for unaddressable memory v2
From: Dan Williams @ 2017-05-22 21:17 UTC (permalink / raw)
To: Jérôme Glisse
Cc: Andrew Morton, linux-kernel@vger.kernel.org, Linux MM,
John Hubbard, David Nellans, Ross Zwisler
In-Reply-To: <20170522165206.6284-8-jglisse@redhat.com>
On Mon, May 22, 2017 at 9:51 AM, Jérôme Glisse <jglisse@redhat.com> wrote:
> HMM (heterogeneous memory management) need struct page to support migration
> from system main memory to device memory. Reasons for HMM and migration to
> device memory is explained with HMM core patch.
>
> This patch deals with device memory that is un-addressable memory (ie CPU
> can not access it). Hence we do not want those struct page to be manage
> like regular memory. That is why we extend ZONE_DEVICE to support different
> types of memory.
>
> A persistent memory type is define for existing user of ZONE_DEVICE and a
> new device un-addressable type is added for the un-addressable memory type.
> There is a clear separation between what is expected from each memory type
> and existing user of ZONE_DEVICE are un-affected by new requirement and new
> use of the un-addressable type. All specific code path are protect with
> test against the memory type.
>
> Because memory is un-addressable we use a new special swap type for when
> a page is migrated to device memory (this reduces the number of maximum
> swap file).
>
> The main two additions beside memory type to ZONE_DEVICE is two callbacks.
> First one, page_free() is call whenever page refcount reach 1 (which means
> the page is free as ZONE_DEVICE page never reach a refcount of 0). This
> allow device driver to manage its memory and associated struct page.
>
> The second callback page_fault() happens when there is a CPU access to
> an address that is back by a device page (which are un-addressable by the
> CPU). This callback is responsible to migrate the page back to system
> main memory. Device driver can not block migration back to system memory,
> HMM make sure that such page can not be pin into device memory.
>
> If device is in some error condition and can not migrate memory back then
> a CPU page fault to device memory should end with SIGBUS.
>
> Changed since v1:
> - rename to device private memory (from device unaddressable)
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
[..]
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 229afe3..d49d816 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -737,6 +737,19 @@ config ZONE_DEVICE
>
> If FS_DAX is enabled, then say Y.
>
> +config DEVICE_UNADDRESSABLE
> + bool "Unaddressable device memory (GPU memory, ...)"
> + depends on X86_64
> + depends on ZONE_DEVICE
> + depends on MEMORY_HOTPLUG
> + depends on MEMORY_HOTREMOVE
> + depends on SPARSEMEM_VMEMMAP
> +
> + help
> + Allows creation of struct pages to represent unaddressable device
> + memory; i.e., memory that is only accessible from the device (or
> + group of devices).
Lets change config symbol naming from "device un-addressable memory"
to "device private memory" the same way we did for the code symbols.
With that change you can add:
Acked-by: Dan Williams <dan.j.williams@intel.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Jerome Glisse @ 2017-05-22 21:17 UTC (permalink / raw)
To: Hugh Dickins
Cc: Dan Williams, Andrew Morton, linux-kernel@vger.kernel.org,
Linux MM, John Hubbard, David Nellans, Kirill A . Shutemov,
Ross Zwisler
In-Reply-To: <alpine.LSU.2.11.1705221317280.4687@eggly.anvils>
On Mon, May 22, 2017 at 01:22:22PM -0700, Hugh Dickins wrote:
> On Mon, 22 May 2017, Jerome Glisse wrote:
> > On Mon, May 22, 2017 at 12:29:53PM -0700, Dan Williams wrote:
> > > On Mon, May 22, 2017 at 9:51 AM, Jerome Glisse <jglisse@redhat.com> wrote:
> > > > A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
> > > > have any user. For device private pages this is important to catch
> > > > and thus we need to special case put_page() for this.
> > > >
> > > > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> > > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > > ---
> > > > include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> > > > kernel/memremap.c | 1 -
> > > > 2 files changed, 30 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > > index a825dab..11f7bac 100644
> > > > --- a/include/linux/mm.h
> > > > +++ b/include/linux/mm.h
> > > > @@ -23,6 +23,7 @@
> > > > #include <linux/page_ext.h>
> > > > #include <linux/err.h>
> > > > #include <linux/page_ref.h>
> > > > +#include <linux/memremap.h>
> > > >
> > > > struct mempolicy;
> > > > struct anon_vma;
> > > > @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
> > > > return ((page_zonenum(page) == ZONE_DEVICE) &&
> > > > (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
> > > > }
> > > > +
> > > > +static inline void put_zone_device_private_page(struct page *page)
> > > > +{
> > > > + int count = page_ref_dec_return(page);
> > > > +
> > > > + /*
> > > > + * If refcount is 1 then page is freed and refcount is stable as nobody
> > > > + * holds a reference on the page.
> > > > + */
> > > > + if (count == 1)
> > > > + page->pgmap->page_free(page, page->pgmap->data);
> > > > + else if (!count)
> > > > + __put_page(page);
> > > > +}
>
> Is there something else in this patchset that guarantees
> that get_page_unless_zero() is never used on thse pages?
> We have plenty of code that knows that refcount 0 is special:
> having to know that refcount 1 may be special is worrying.
>
> Hugh
ZONE_DEVICE pages always had this extra refcount since their
inception. All the place that use get_page_unless_zero() should
be unreachable by a ZONE_DEVICE pages (hwpoison, lru, isolate,
ksm, ...). So if that happens it is a bug.
Jerome
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Jerome Glisse @ 2017-05-22 21:14 UTC (permalink / raw)
To: Dan Williams
Cc: Andrew Morton, linux-kernel@vger.kernel.org, Linux MM,
John Hubbard, David Nellans, Kirill A . Shutemov, Ross Zwisler
In-Reply-To: <CAPcyv4ipiDwPB7V72dpzF3FwKRCd8m0JHDLezrUm0=oQ4q2VeA@mail.gmail.com>
On Mon, May 22, 2017 at 01:19:29PM -0700, Dan Williams wrote:
> On Mon, May 22, 2017 at 1:14 PM, Jerome Glisse <jglisse@redhat.com> wrote:
> > On Mon, May 22, 2017 at 12:29:53PM -0700, Dan Williams wrote:
> >> On Mon, May 22, 2017 at 9:51 AM, Jerome Glisse <jglisse@redhat.com> wrote:
> >> > A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
> >> > have any user. For device private pages this is important to catch
> >> > and thus we need to special case put_page() for this.
> >> >
> >> > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> >> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> >> > Cc: Dan Williams <dan.j.williams@intel.com>
> >> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> > ---
> >> > include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> >> > kernel/memremap.c | 1 -
> >> > 2 files changed, 30 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> >> > index a825dab..11f7bac 100644
> >> > --- a/include/linux/mm.h
> >> > +++ b/include/linux/mm.h
> >> > @@ -23,6 +23,7 @@
> >> > #include <linux/page_ext.h>
> >> > #include <linux/err.h>
> >> > #include <linux/page_ref.h>
> >> > +#include <linux/memremap.h>
> >> >
> >> > struct mempolicy;
> >> > struct anon_vma;
> >> > @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
> >> > return ((page_zonenum(page) == ZONE_DEVICE) &&
> >> > (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
> >> > }
> >> > +
> >> > +static inline void put_zone_device_private_page(struct page *page)
> >> > +{
> >> > + int count = page_ref_dec_return(page);
> >> > +
> >> > + /*
> >> > + * If refcount is 1 then page is freed and refcount is stable as nobody
> >> > + * holds a reference on the page.
> >> > + */
> >> > + if (count == 1)
> >> > + page->pgmap->page_free(page, page->pgmap->data);
> >> > + else if (!count)
> >> > + __put_page(page);
> >> > +}
> >> > #else
> >> > static inline bool is_zone_device_page(const struct page *page)
> >> > {
> >> > @@ -805,6 +820,10 @@ static inline bool is_device_private_page(const struct page *page)
> >> > {
> >> > return false;
> >> > }
> >> > +
> >> > +static inline void put_zone_device_private_page(struct page *page)
> >> > +{
> >> > +}
> >> > #endif
> >> >
> >> > static inline void get_page(struct page *page)
> >> > @@ -822,6 +841,17 @@ static inline void put_page(struct page *page)
> >> > {
> >> > page = compound_head(page);
> >> >
> >> > + /*
> >> > + * For private device pages we need to catch refcount transition from
> >> > + * 2 to 1, when refcount reach one it means the private device page is
> >> > + * free and we need to inform the device driver through callback. See
> >> > + * include/linux/memremap.h and HMM for details.
> >> > + */
> >> > + if (unlikely(is_device_private_page(page))) {
> >>
> >> Since I presume HMM is a niche use case can we make this a
> >> "static_branch_unlikely(&hmm_key) && is_device_private_page(page))"?
> >> That way non-hmm platforms see minimal overhead.
> >
> > Like i said in the cover letter i am bit anxious about doing for
>
> I don't think you copied me on the cover letter.
>
> > an inline function. I don't see any existing case for inline
> > function and static key. Is that suppose to work ?
> >
> > How widespread HMM use will be is hard to guess. Usual chicken
> > and egg plus adoption thing. If GPGPU compte keeps growing and
> > it seems it does then HMM likely gonna be enable and actively
> > use for large chunk of those computer that have GPGPU workload.
> >
> > I will test a static key of that branch and see if it explodes
> > because put_page() is an inline function.
>
> memcpy_mcsafe() is an existing example of a static inline with a
> static branch. Hasn't seemed to have caused any problems to date.
Ok i will post a new version of 08 with static keys, i shouldn't
need to repost any other patches for that. Andrew that's fine
with you or do you prefer me to repost a full updated patchset ?
Cheers,
Jerome
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [patch 2/2] MM: allow per-cpu vmstat_threshold and vmstat_worker configuration
From: Marcelo Tosatti @ 2017-05-22 21:13 UTC (permalink / raw)
To: Christoph Lameter
Cc: Luiz Capitulino, linux-kernel, linux-mm, Rik van Riel,
Linux RT Users, cmetcalf
In-Reply-To: <alpine.DEB.2.20.1705221137001.12282@east.gentwo.org>
On Mon, May 22, 2017 at 11:38:02AM -0500, Christoph Lameter wrote:
> On Sat, 20 May 2017, Marcelo Tosatti wrote:
>
> > > And you can configure the interval of vmstat updates freely.... Set
> > > the vmstat_interval to 60 seconds instead of 2 for a try? Is that rare
> > > enough?
> >
> > Not rare enough. Never is rare enough.
>
> Ok what about the other stuff that must be going on if you allow OS
> activity like f.e. the tick, scheduler etc etc.
Yes these are also problems... but we're either getting rid of them or
reducing their impact as much as possible.
vmstat_update is one member of the problematic set.
I'll get you the detailed IPI measures, hold on...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
From: Andrew Morton @ 2017-05-22 21:11 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: linux-mm, linux-kernel
In-Reply-To: <20170522111742.29433-1-khandual@linux.vnet.ibm.com>
On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> There are many places where we define size either left shifting integers
> or multiplying 1024s without any generic definition to fall back on. But
> there are couples of (powerpc and lz4) attempts to define these standard
> memory sizes. Lets move these definitions to core VM to make sure that
> all new usage come from these definitions eventually standardizing it
> across all places.
Grep further - there are many more definitions and some may now
generate warnings.
Newly including mm.h for these things seems a bit heavyweight. I can't
immediately think of a more appropriate place. Maybe printk.h or
kernel.h.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
From: Matthias Kaehlcke @ 2017-05-22 20:56 UTC (permalink / raw)
To: David Rientjes
Cc: Christoph Lameter, Pekka Enberg, Joonsoo Kim, Andrew Morton,
linux-mm, linux-kernel
In-Reply-To: <alpine.DEB.2.10.1705221338100.30407@chino.kir.corp.google.com>
El Mon, May 22, 2017 at 01:39:26PM -0700 David Rientjes ha dit:
> On Fri, 19 May 2017, Matthias Kaehlcke wrote:
>
> > The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> > block fixes the following warning when building with clang:
> >
> > mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> > [-Werror,-Wunused-function]
> >
>
> Is clang not inlining kmalloc_large_node_hook() for some reason? I don't
> think this should ever warn on gcc.
clang warns about unused static inline functions outside of header
files, in difference to gcc.
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>
> Acked-by: David Rientjes <rientjes@google.com>
>
> > ---
> > mm/slub.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 57e5156f02be..66e1046435b7 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
> > * Hooks for other subsystems that check memory allocations. In a typical
> > * production configuration these hooks all should produce no code at all.
> > */
> > +
> > +#ifdef CONFIG_NUMA
> > static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
> > {
> > kmemleak_alloc(ptr, size, 1, flags);
> > kasan_kmalloc_large(ptr, size, flags);
> > }
> > +#endif
> >
> > static inline void kfree_hook(const void *x)
> > {
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] LSM: Make security_hook_heads a local variable.
From: Tetsuo Handa @ 2017-05-22 20:43 UTC (permalink / raw)
To: casey, igor.stoppa, hch
Cc: linux-security-module, linux-mm, kernel-hardening, linux-kernel,
gregkh, james.l.morris, keescook, paul, sds
In-Reply-To: <af26581e-6f5a-3fc2-dc58-8376328a0ad9@schaufler-ca.com>
Casey Schaufler wrote:
> On 5/22/2017 12:50 PM, Igor Stoppa wrote:
> > On 22/05/17 18:09, Casey Schaufler wrote:
> >> On 5/22/2017 7:03 AM, Christoph Hellwig wrote:
> > [...]
> >
> >>> But even with those we can still chain
> >>> them together with a list with external linkage.
> >> I gave up that approach in 2012. Too many unnecessary calls to
> >> null functions, and massive function vectors with a tiny number
> >> of non-null entries. From a data structure standpoint, it was
> >> just wrong. The list scheme is exactly right for the task at
> >> hand.
> > I understand this as a green light, for me to continue with the plan of
> > using LSM Hooks as example for making dynamically allocated data become
> > read-only, using also Tetsuo's patch (thanks, btw).
>
> I still don't like the assumption that a structure of
> N elements can be assumed to be the same as an array
> of N elements.
I think we can use "enum" and call via index numbers while preserving
current "union" for type checking purpose.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/3] mm/slub: Only define kmalloc_large_node_hook() for NUMA systems
From: David Rientjes @ 2017-05-22 20:39 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Christoph Lameter, Pekka Enberg, Joonsoo Kim, Andrew Morton,
linux-mm, linux-kernel
In-Reply-To: <20170519210036.146880-2-mka@chromium.org>
On Fri, 19 May 2017, Matthias Kaehlcke wrote:
> The function is only used when CONFIG_NUMA=y. Placing it in an #ifdef
> block fixes the following warning when building with clang:
>
> mm/slub.c:1246:20: error: unused function 'kmalloc_large_node_hook'
> [-Werror,-Wunused-function]
>
Is clang not inlining kmalloc_large_node_hook() for some reason? I don't
think this should ever warn on gcc.
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Acked-by: David Rientjes <rientjes@google.com>
> ---
> mm/slub.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 57e5156f02be..66e1046435b7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1313,11 +1313,14 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node,
> * Hooks for other subsystems that check memory allocations. In a typical
> * production configuration these hooks all should produce no code at all.
> */
> +
> +#ifdef CONFIG_NUMA
> static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
> {
> kmemleak_alloc(ptr, size, 1, flags);
> kasan_kmalloc_large(ptr, size, flags);
> }
> +#endif
>
> static inline void kfree_hook(const void *x)
> {
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: dm ioctl: Restore __GFP_HIGH in copy_params()
From: David Rientjes @ 2017-05-22 20:35 UTC (permalink / raw)
To: Mike Snitzer
Cc: Michal Hocko, Mikulas Patocka, Junaid Shahid, Alasdair Kergon,
Andrew Morton, linux-mm, andreslc, gthelen, vbabka, linux-kernel
In-Reply-To: <20170522180415.GA25340@redhat.com>
On Mon, 22 May 2017, Mike Snitzer wrote:
> > > The lvm2 was designed this way - it is broken, but there is not much that
> > > can be done about it - fixing this would mean major rewrite. The only
> > > thing we can do about it is to lower the deadlock probability with
> > > __GFP_HIGH (or PF_MEMALLOC that was used some times ago).
>
> Yes, lvm2 was originally designed to to have access to memory reserves
> to ensure forward progress. But if the mm subsystem has improved to
> allow for the required progress without lvm2 trying to stake a claim on
> those reserves then we'll gladly avoid (ab)using them.
>
There is no such improvement to the page allocator when allocating at
runtime. A persistent amount of memory in a mempool could be set aside as
a preallocation and unavailable from the rest of the system forever as an
alternative to dynamically allocating with memory reserves, but that has
obvious downsides. This patch is the exact right thing to do.
> > But let me repeat. GFP_KERNEL allocation for order-0 page will not fail.
>
> OK, but will it be serviced immediately? Not failing isn't useful if it
> never completes.
>
No, and you can use __GFP_HIGH, which this patch does, to have a
reasonable expectation of forward progress in the very near term.
> While adding the __GFP_NOFAIL flag would serve to document expectations
> I'm left unconvinced that the memory allocator will _not fail_ for an
> order-0 page -- as Mikulas said most ioctls don't need more than 4K.
__GFP_NOFAIL would make no sense in kvmalloc() calls, ever, it would never
fallback to vmalloc :)
I'm hoping this can get merged during the 4.12 window to fix the broken
commit d224e9381897.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] LSM: Make security_hook_heads a local variable.
From: Casey Schaufler @ 2017-05-22 20:32 UTC (permalink / raw)
To: Igor Stoppa, Christoph Hellwig, Tetsuo Handa
Cc: linux-security-module, linux-mm, kernel-hardening, linux-kernel,
Greg KH, James Morris, Kees Cook, Paul Moore, Stephen Smalley
In-Reply-To: <d25e2fd3-da11-4ec0-8edc-f1327c04fa6e@huawei.com>
On 5/22/2017 12:50 PM, Igor Stoppa wrote:
> On 22/05/17 18:09, Casey Schaufler wrote:
>> On 5/22/2017 7:03 AM, Christoph Hellwig wrote:
> [...]
>
>>> But even with those we can still chain
>>> them together with a list with external linkage.
>> I gave up that approach in 2012. Too many unnecessary calls to
>> null functions, and massive function vectors with a tiny number
>> of non-null entries. From a data structure standpoint, it was
>> just wrong. The list scheme is exactly right for the task at
>> hand.
> I understand this as a green light, for me to continue with the plan of
> using LSM Hooks as example for making dynamically allocated data become
> read-only, using also Tetsuo's patch (thanks, btw).
I still don't like the assumption that a structure of
N elements can be assumed to be the same as an array
of N elements. Putting on my hardening hat, however, I
like the smalloc() solution to keeping the hook lists
safe, so I am willing to swallow the objection to using
offsets to address the existing exposure.
>
> Is that correct?
>
> ---
> thanks, igor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3.1 4/6] mm/hugetlb: Allow architectures to override huge_pte_clear()
From: Arnd Bergmann @ 2017-05-22 20:34 UTC (permalink / raw)
To: Punit Agrawal
Cc: Andrew Morton, Linux-MM, Linux Kernel Mailing List, Linux ARM,
Catalin Marinas, Will Deacon, n-horiguchi, Kirill A . Shutemov,
mike.kravetz, steve.capper, Mark Rutland, linux-arch,
Aneesh Kumar K.V, Martin Schwidefsky, Heiko Carstens
In-Reply-To: <20170522162555.4313-1-punit.agrawal@arm.com>
On Mon, May 22, 2017 at 6:25 PM, Punit Agrawal <punit.agrawal@arm.com> wrote:
> When unmapping a hugepage range, huge_pte_clear() is used to clear the
> page table entries that are marked as not present. huge_pte_clear()
> internally just ends up calling pte_clear() which does not correctly
> deal with hugepages consisting of contiguous page table entries.
>
> Add a size argument to address this issue and allow architectures to
> override huge_pte_clear() by wrapping it in a #ifndef block.
>
> Update s390 implementation with the size parameter as well.
>
> Note that the change only affects huge_pte_clear() - the other generic
> hugetlb functions don't need any change.
>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Hugh Dickins @ 2017-05-22 20:22 UTC (permalink / raw)
To: Jerome Glisse
Cc: Dan Williams, Andrew Morton, linux-kernel@vger.kernel.org,
Linux MM, John Hubbard, David Nellans, Kirill A . Shutemov,
Ross Zwisler
In-Reply-To: <20170522201416.GA8168@redhat.com>
On Mon, 22 May 2017, Jerome Glisse wrote:
> On Mon, May 22, 2017 at 12:29:53PM -0700, Dan Williams wrote:
> > On Mon, May 22, 2017 at 9:51 AM, Jerome Glisse <jglisse@redhat.com> wrote:
> > > A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
> > > have any user. For device private pages this is important to catch
> > > and thus we need to special case put_page() for this.
> > >
> > > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > ---
> > > include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> > > kernel/memremap.c | 1 -
> > > 2 files changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index a825dab..11f7bac 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -23,6 +23,7 @@
> > > #include <linux/page_ext.h>
> > > #include <linux/err.h>
> > > #include <linux/page_ref.h>
> > > +#include <linux/memremap.h>
> > >
> > > struct mempolicy;
> > > struct anon_vma;
> > > @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
> > > return ((page_zonenum(page) == ZONE_DEVICE) &&
> > > (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
> > > }
> > > +
> > > +static inline void put_zone_device_private_page(struct page *page)
> > > +{
> > > + int count = page_ref_dec_return(page);
> > > +
> > > + /*
> > > + * If refcount is 1 then page is freed and refcount is stable as nobody
> > > + * holds a reference on the page.
> > > + */
> > > + if (count == 1)
> > > + page->pgmap->page_free(page, page->pgmap->data);
> > > + else if (!count)
> > > + __put_page(page);
> > > +}
Is there something else in this patchset that guarantees
that get_page_unless_zero() is never used on thse pages?
We have plenty of code that knows that refcount 0 is special:
having to know that refcount 1 may be special is worrying.
Hugh
> > > #else
> > > static inline bool is_zone_device_page(const struct page *page)
> > > {
> > > @@ -805,6 +820,10 @@ static inline bool is_device_private_page(const struct page *page)
> > > {
> > > return false;
> > > }
> > > +
> > > +static inline void put_zone_device_private_page(struct page *page)
> > > +{
> > > +}
> > > #endif
> > >
> > > static inline void get_page(struct page *page)
> > > @@ -822,6 +841,17 @@ static inline void put_page(struct page *page)
> > > {
> > > page = compound_head(page);
> > >
> > > + /*
> > > + * For private device pages we need to catch refcount transition from
> > > + * 2 to 1, when refcount reach one it means the private device page is
> > > + * free and we need to inform the device driver through callback. See
> > > + * include/linux/memremap.h and HMM for details.
> > > + */
> > > + if (unlikely(is_device_private_page(page))) {
> >
> > Since I presume HMM is a niche use case can we make this a
> > "static_branch_unlikely(&hmm_key) && is_device_private_page(page))"?
> > That way non-hmm platforms see minimal overhead.
>
> Like i said in the cover letter i am bit anxious about doing for
> an inline function. I don't see any existing case for inline
> function and static key. Is that suppose to work ?
>
> How widespread HMM use will be is hard to guess. Usual chicken
> and egg plus adoption thing. If GPGPU compte keeps growing and
> it seems it does then HMM likely gonna be enable and actively
> use for large chunk of those computer that have GPGPU workload.
>
> I will test a static key of that branch and see if it explodes
> because put_page() is an inline function.
>
> Cheers,
> Jerome
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Dan Williams @ 2017-05-22 20:19 UTC (permalink / raw)
To: Jerome Glisse
Cc: Andrew Morton, linux-kernel@vger.kernel.org, Linux MM,
John Hubbard, David Nellans, Kirill A . Shutemov, Ross Zwisler
In-Reply-To: <20170522201416.GA8168@redhat.com>
On Mon, May 22, 2017 at 1:14 PM, Jerome Glisse <jglisse@redhat.com> wrote:
> On Mon, May 22, 2017 at 12:29:53PM -0700, Dan Williams wrote:
>> On Mon, May 22, 2017 at 9:51 AM, Jérôme Glisse <jglisse@redhat.com> wrote:
>> > A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
>> > have any user. For device private pages this is important to catch
>> > and thus we need to special case put_page() for this.
>> >
>> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
>> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> > Cc: Dan Williams <dan.j.williams@intel.com>
>> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> > ---
>> > include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
>> > kernel/memremap.c | 1 -
>> > 2 files changed, 30 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/include/linux/mm.h b/include/linux/mm.h
>> > index a825dab..11f7bac 100644
>> > --- a/include/linux/mm.h
>> > +++ b/include/linux/mm.h
>> > @@ -23,6 +23,7 @@
>> > #include <linux/page_ext.h>
>> > #include <linux/err.h>
>> > #include <linux/page_ref.h>
>> > +#include <linux/memremap.h>
>> >
>> > struct mempolicy;
>> > struct anon_vma;
>> > @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
>> > return ((page_zonenum(page) == ZONE_DEVICE) &&
>> > (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
>> > }
>> > +
>> > +static inline void put_zone_device_private_page(struct page *page)
>> > +{
>> > + int count = page_ref_dec_return(page);
>> > +
>> > + /*
>> > + * If refcount is 1 then page is freed and refcount is stable as nobody
>> > + * holds a reference on the page.
>> > + */
>> > + if (count == 1)
>> > + page->pgmap->page_free(page, page->pgmap->data);
>> > + else if (!count)
>> > + __put_page(page);
>> > +}
>> > #else
>> > static inline bool is_zone_device_page(const struct page *page)
>> > {
>> > @@ -805,6 +820,10 @@ static inline bool is_device_private_page(const struct page *page)
>> > {
>> > return false;
>> > }
>> > +
>> > +static inline void put_zone_device_private_page(struct page *page)
>> > +{
>> > +}
>> > #endif
>> >
>> > static inline void get_page(struct page *page)
>> > @@ -822,6 +841,17 @@ static inline void put_page(struct page *page)
>> > {
>> > page = compound_head(page);
>> >
>> > + /*
>> > + * For private device pages we need to catch refcount transition from
>> > + * 2 to 1, when refcount reach one it means the private device page is
>> > + * free and we need to inform the device driver through callback. See
>> > + * include/linux/memremap.h and HMM for details.
>> > + */
>> > + if (unlikely(is_device_private_page(page))) {
>>
>> Since I presume HMM is a niche use case can we make this a
>> "static_branch_unlikely(&hmm_key) && is_device_private_page(page))"?
>> That way non-hmm platforms see minimal overhead.
>
> Like i said in the cover letter i am bit anxious about doing for
I don't think you copied me on the cover letter.
> an inline function. I don't see any existing case for inline
> function and static key. Is that suppose to work ?
>
> How widespread HMM use will be is hard to guess. Usual chicken
> and egg plus adoption thing. If GPGPU compte keeps growing and
> it seems it does then HMM likely gonna be enable and actively
> use for large chunk of those computer that have GPGPU workload.
>
> I will test a static key of that branch and see if it explodes
> because put_page() is an inline function.
memcpy_mcsafe() is an existing example of a static inline with a
static branch. Hasn't seemed to have caused any problems to date.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Jerome Glisse @ 2017-05-22 20:14 UTC (permalink / raw)
To: Dan Williams
Cc: Andrew Morton, linux-kernel@vger.kernel.org, Linux MM,
John Hubbard, David Nellans, Kirill A . Shutemov, Ross Zwisler
In-Reply-To: <CAPcyv4hodnCFEy8iyb3jQPJ=TNj-L2uZQKJqb7JTqSv=YE0BDg@mail.gmail.com>
On Mon, May 22, 2017 at 12:29:53PM -0700, Dan Williams wrote:
> On Mon, May 22, 2017 at 9:51 AM, Jerome Glisse <jglisse@redhat.com> wrote:
> > A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
> > have any user. For device private pages this is important to catch
> > and thus we need to special case put_page() for this.
> >
> > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> > ---
> > include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> > kernel/memremap.c | 1 -
> > 2 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index a825dab..11f7bac 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -23,6 +23,7 @@
> > #include <linux/page_ext.h>
> > #include <linux/err.h>
> > #include <linux/page_ref.h>
> > +#include <linux/memremap.h>
> >
> > struct mempolicy;
> > struct anon_vma;
> > @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
> > return ((page_zonenum(page) == ZONE_DEVICE) &&
> > (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
> > }
> > +
> > +static inline void put_zone_device_private_page(struct page *page)
> > +{
> > + int count = page_ref_dec_return(page);
> > +
> > + /*
> > + * If refcount is 1 then page is freed and refcount is stable as nobody
> > + * holds a reference on the page.
> > + */
> > + if (count == 1)
> > + page->pgmap->page_free(page, page->pgmap->data);
> > + else if (!count)
> > + __put_page(page);
> > +}
> > #else
> > static inline bool is_zone_device_page(const struct page *page)
> > {
> > @@ -805,6 +820,10 @@ static inline bool is_device_private_page(const struct page *page)
> > {
> > return false;
> > }
> > +
> > +static inline void put_zone_device_private_page(struct page *page)
> > +{
> > +}
> > #endif
> >
> > static inline void get_page(struct page *page)
> > @@ -822,6 +841,17 @@ static inline void put_page(struct page *page)
> > {
> > page = compound_head(page);
> >
> > + /*
> > + * For private device pages we need to catch refcount transition from
> > + * 2 to 1, when refcount reach one it means the private device page is
> > + * free and we need to inform the device driver through callback. See
> > + * include/linux/memremap.h and HMM for details.
> > + */
> > + if (unlikely(is_device_private_page(page))) {
>
> Since I presume HMM is a niche use case can we make this a
> "static_branch_unlikely(&hmm_key) && is_device_private_page(page))"?
> That way non-hmm platforms see minimal overhead.
Like i said in the cover letter i am bit anxious about doing for
an inline function. I don't see any existing case for inline
function and static key. Is that suppose to work ?
How widespread HMM use will be is hard to guess. Usual chicken
and egg plus adoption thing. If GPGPU compte keeps growing and
it seems it does then HMM likely gonna be enable and actively
use for large chunk of those computer that have GPGPU workload.
I will test a static key of that branch and see if it explodes
because put_page() is an inline function.
Cheers,
Jerome
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] LSM: Make security_hook_heads a local variable.
From: Igor Stoppa @ 2017-05-22 19:50 UTC (permalink / raw)
To: Casey Schaufler, Christoph Hellwig, Tetsuo Handa
Cc: linux-security-module, linux-mm, kernel-hardening, linux-kernel,
Greg KH, James Morris, Kees Cook, Paul Moore, Stephen Smalley
In-Reply-To: <d98f4cd5-3f21-3f7b-2842-12b9a009e453@schaufler-ca.com>
On 22/05/17 18:09, Casey Schaufler wrote:
> On 5/22/2017 7:03 AM, Christoph Hellwig wrote:
[...]
>> But even with those we can still chain
>> them together with a list with external linkage.
>
> I gave up that approach in 2012. Too many unnecessary calls to
> null functions, and massive function vectors with a tiny number
> of non-null entries. From a data structure standpoint, it was
> just wrong. The list scheme is exactly right for the task at
> hand.
I understand this as a green light, for me to continue with the plan of
using LSM Hooks as example for making dynamically allocated data become
read-only, using also Tetsuo's patch (thanks, btw).
Is that correct?
---
thanks, igor
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox