Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1229914 > unrolled thread

[PATCH 05/11] mm: Introduce arch_pgd_init_late()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 05/11] mm: Introduce arch_pgd_init_late() Ingo Molnar <mingo@kernel.org> - 2015-09-22 08:30 +0200
    Re: [PATCH 05/11] mm: Introduce arch_pgd_init_late() Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-22 20:00 +0200
      Re: [PATCH 05/11] mm: Introduce arch_pgd_init_late() Andy Lutomirski <luto@amacapital.net> - 2015-09-22 20:10 +0200
        Re: [PATCH 05/11] mm: Introduce arch_pgd_init_late() Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-22 20:30 +0200
          Re: [PATCH 05/11] mm: Introduce arch_pgd_init_late() Andy Lutomirski <luto@amacapital.net> - 2015-09-22 20:40 +0200
            Re: [PATCH 05/11] mm: Introduce arch_pgd_init_late() Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-22 20:50 +0200
              Re: [PATCH 05/11] mm: Introduce arch_pgd_init_late() Andy Lutomirski <luto@amacapital.net> - 2015-09-22 21:00 +0200

#1229914 — [PATCH 05/11] mm: Introduce arch_pgd_init_late()

Add a late PGD init callback to places that allocate a new MM
with a new PGD: copy_process() and exec().

The purpose of this callback is to allow architectures to implement
lockless initialization of task PGDs, to remove the scalability
limit of pgd_list/pgd_lock.

Architectures can opt in to this callback via the ARCH_HAS_PGD_INIT_LATE
Kconfig flag. There's zero overhead on architectures that are not using it.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <Waiman.Long@hp.com>
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/Kconfig          |  9 ++++++++
 arch/x86/Kconfig      |  1 +
 arch/x86/mm/init_64.c | 12 +++++++++++
 arch/x86/mm/pgtable.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/exec.c             |  3 +++
 include/linux/mm.h    |  6 ++++++
 kernel/fork.c         | 16 ++++++++++++++
 7 files changed, 106 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 4e949e58b192..671810ce6fe0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -503,6 +503,15 @@ config PGTABLE_LEVELS
 	int
 	default 2
 
+config ARCH_HAS_PGD_INIT_LATE
+	bool
+	help
+	  Architectures that want a late PGD initialization can define
+	  the arch_pgd_init_late() callback and it will be called
+	  by the generic new task (fork()) code after a new task has
+	  been made visible on the task list, but before it has been
+	  first scheduled.
+
 config ARCH_HAS_ELF_RANDOMIZE
 	bool
 	help
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 328c8352480c..3e97b6cfdb60 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -27,6 +27,7 @@ config X86
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FAST_MULTIPLIER
 	select ARCH_HAS_GCOV_PROFILE_ALL
+	select ARCH_HAS_PGD_INIT_LATE
 	select ARCH_HAS_PMEM_API		if X86_64
 	select ARCH_HAS_MMIO_FLUSH
 	select ARCH_HAS_SG_CHAIN
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 467c4f66ded9..429362f8d6ca 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -177,6 +177,18 @@ void sync_global_pgds(unsigned long start, unsigned long end)
 			continue;
 
 		rcu_read_lock(); /* Task list walk */
+
+		/*
+		 * Since this is x86, this spin_lock() is also a full memory barrier that
+		 * is required for correct operation of the lockless reading of PGDs
+		 * in arch_pgd_init_late(). If you ever move this code to another
+		 * architecture or to generic code you need to make sure this is
+		 * an:
+		 *
+		 *	smp_mb();
+		 *
+		 * before looking at PGDs in the loop below.
+		 */
 		spin_lock(&pgd_lock);
 
 		for_each_process(g) {
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index fb0a9dd1d6e4..c7038b6e51bf 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -391,6 +391,65 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 	return NULL;
 }
 
+/*
+ * Initialize the kernel portion of the PGD.
+ *
+ * This is done separately, because pgd_alloc() happens when
+ * the task is not on the task list yet - and PGD updates
+ * happen by walking the task list.
+ *
+ * No locking is needed here, as we just copy over the reference
+ * PGD. The reference PGD (pgtable_init) is only ever expanded
+ * at the highest, PGD level. Thus any other task extending it
+ * will first update the reference PGD, then modify the task PGDs.
+ */
+void arch_pgd_init_late(struct mm_struct *mm)
+{
+	/*
+	 * This function is called after a new MM has been made visible
+	 * in fork() or exec() via:
+	 *
+	 *   tsk->mm = mm;
+	 *
+	 * This barrier makes sure the MM is visible to new RCU
+	 * walkers before we read and initialize the pagetables below,
+	 * so that we don't miss updates:
+	 */
+	smp_mb();
+
+	/*
+	 * If the pgd points to a shared pagetable level (either the
+	 * ptes in non-PAE, or shared PMD in PAE), then just copy the
+	 * references from swapper_pg_dir:
+	 */
+	if ( CONFIG_PGTABLE_LEVELS == 2 ||
+	    (CONFIG_PGTABLE_LEVELS == 3 && SHARED_KERNEL_PMD) ||
+	     CONFIG_PGTABLE_LEVELS == 4) {
+
+		pgd_t *pgd_src = swapper_pg_dir + KERNEL_PGD_BOUNDARY;
+		pgd_t *pgd_dst =        mm->pgd + KERNEL_PGD_BOUNDARY;
+		int i;
+
+		for (i = 0; i < KERNEL_PGD_PTRS; i++, pgd_src++, pgd_dst++) {
+			/*
+			 * This is lock-less, so it can race with PGD updates
+			 * coming from vmalloc() or CPA methods, but it's safe,
+			 * because:
+			 *
+			 * 1) this PGD is not in use yet, we have still not
+			 *    scheduled this task.
+			 * 2) we only ever extend PGD entries
+			 *
+			 * So if we observe a non-zero PGD entry we can copy it,
+			 * it won't change from under us. Parallel updates (new
+			 * allocations) will modify our (already visible) PGD:
+			 */
+			if (!pgd_none(*pgd_src))
+				set_pgd(pgd_dst, *pgd_src);
+		}
+	}
+}
+
 void pgd_free(struct mm_struct *mm, pgd_t *pgd)
 {
 	pgd_mop_up_pmds(mm, pgd);
diff --git a/fs/exec.c b/fs/exec.c
index b06623a9347f..0a77a6991d0e 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -866,7 +866,10 @@ static int exec_mmap(struct mm_struct *mm)
 	}
 	task_lock(tsk);
 	active_mm = tsk->active_mm;
+
 	tsk->mm = mm;
+	arch_pgd_init_late(mm);
+
 	tsk->active_mm = mm;
 	activate_mm(active_mm, mm);
 	tsk->mm->vmacache_seqnum = 0;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 91c08f6f0dc9..8d008dfa9d73 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1152,6 +1152,12 @@ int follow_phys(struct vm_area_struct *vma, unsigned long address,
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write);
 
+#ifdef CONFIG_ARCH_HAS_PGD_INIT_LATE
+void arch_pgd_init_late(struct mm_struct *mm);
+#else
+static inline void arch_pgd_init_late(struct mm_struct *mm) { }
+#endif
+
 static inline void unmap_shared_mapping_range(struct address_space *mapping,
 		loff_t const holebegin, loff_t const holelen)
 {
diff --git a/kernel/fork.c b/kernel/fork.c
index 7d5f0f118a63..4668f8902b19 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1606,6 +1606,22 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	syscall_tracepoint_update(p);
 	write_unlock_irq(&tasklist_lock);
 
+	/*
+	 * If we have a new PGD then initialize it:
+	 *
+	 * This method is called after a task has been made visible
+	 * on the task list already.
+	 *
+	 * Architectures that manage per task kernel pagetables
+	 * might use this callback to initialize them after they
+	 * are already visible to new updates.
+	 *
+	 * NOTE: any user-space parts of the PGD are already initialized
+	 *       and must not be clobbered.
+	 */
+	if (!(clone_flags & CLONE_VM))
+		arch_pgd_init_late(p->mm);
+
 	proc_fork_connector(p);
 	cgroup_post_fork(p, cgrp_ss_priv);
 	if (clone_flags & CLONE_THREAD)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1230589

On Mon, Sep 21, 2015 at 11:23 PM, Ingo Molnar <mingo@kernel.org> wrote:
> Add a late PGD init callback to places that allocate a new MM
> with a new PGD: copy_process() and exec().
>
> The purpose of this callback is to allow architectures to implement
> lockless initialization of task PGDs, to remove the scalability
> limit of pgd_list/pgd_lock.

Do we really need this?

Can't we just initialize the pgd when we allocate it, knowing that
it's not in sync, but just depend on the vmalloc fault to add in any
kernel entries that we might have missed?

I liked the other patches in the series because they remove code and
simplify things. This patch I don't like.

There may be some reason we need it that I missed, and which makes me
go "Duh!" when you tell me. But please do tell me.

               Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230597

On Tue, Sep 22, 2015 at 10:55 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Sep 21, 2015 at 11:23 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> Add a late PGD init callback to places that allocate a new MM
>> with a new PGD: copy_process() and exec().
>>
>> The purpose of this callback is to allow architectures to implement
>> lockless initialization of task PGDs, to remove the scalability
>> limit of pgd_list/pgd_lock.
>
> Do we really need this?
>
> Can't we just initialize the pgd when we allocate it, knowing that
> it's not in sync, but just depend on the vmalloc fault to add in any
> kernel entries that we might have missed?

I really really hate the vmalloc fault thing.  It seems to work,
rather to my surprise.  It doesn't *deserve* to work, because of
things like the percpu TSS accesses in the entry code that happen
without a valid stack.

For all I know, there's a long history of this hitting on monster
non-SMAP systems that are all buggy and rootable but no one notices
because it's rare.  On SMAP with non-malicious userspace, it's an
instant double fault.  With malicious userspace, it's rootable
regardless of SMAP, but it's much harder with SMAP.

If we start every mm with a fully zeroed pgd (which is what I think
you're suggesting), then this starts affecting small systems as in
addition to monster systems.

I'd really rather go in the other directoin and completely eliminate
vmalloc faults.  We could do that by eagerly initializing all pgd, or
we could do it by tracking, per-pgd, how up-to-date it is and fixing
it up in switch_mm.  The latter is a bit nasty on SMP.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230672

On Tue, Sep 22, 2015 at 11:00 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> I really really hate the vmalloc fault thing.  It seems to work,
> rather to my surprise.  It doesn't *deserve* to work, because of
> things like the percpu TSS accesses in the entry code that happen
> without a valid stack.

The thing is, I think you're misguided in your hatred.

The reason I say that is because I think we should just embrace the
fact that faults can and do happen in the kernel in very inconvenient
places, and not just in code we "control".

Even if you get rid of the vmalloc fault, you'll still have debug
faults, and you'll still have NMI's and horrible crazy machine check
faults.

I actually think teh vmalloc fault is a good way to just let people
know "pretty much anything can trap, deal with it".

And I think trying to eliminate them is the wrong thing, because it
forces us to be so damn synchronized. This whole patch-series is a
prime example of why that is a bad bad things. We want to have _less_
synchronization.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230702

On Tue, Sep 22, 2015 at 11:26 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Sep 22, 2015 at 11:00 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> I really really hate the vmalloc fault thing.  It seems to work,
>> rather to my surprise.  It doesn't *deserve* to work, because of
>> things like the percpu TSS accesses in the entry code that happen
>> without a valid stack.
>
> The thing is, I think you're misguided in your hatred.
>
> The reason I say that is because I think we should just embrace the
> fact that faults can and do happen in the kernel in very inconvenient
> places, and not just in code we "control".
>
> Even if you get rid of the vmalloc fault, you'll still have debug
> faults, and you'll still have NMI's and horrible crazy machine check
> faults.
>
> I actually think teh vmalloc fault is a good way to just let people
> know "pretty much anything can trap, deal with it".
>
> And I think trying to eliminate them is the wrong thing, because it
> forces us to be so damn synchronized. This whole patch-series is a
> prime example of why that is a bad bad things. We want to have _less_
> synchronization.

Sure, pretty much anything can trap, but we need to do *something* to
deal with it.

Debug faults can't happen with bad stacks any more (now that we honor
the kprobe blacklist), which means that debug faults could, in theory,
move off the IST stack.  The SYSENTER + debug mess doesn't have any
stack problem.

NMIs and MCEs are special, and we deal with that using IST and all
kinds of mess.

I don't think that anyone really wants to move #PF to IST, which means
that we simply cannot handle vmalloc faults that happen when switching
stacks after SYSCALL, no matter what fanciness we shove into the
page_fault asm.  If we move #PF to IST, then we have to worry about
page_fault -> nmi -> page_fault, which would be a clusterf*ck.

AMD gave us a pile of misguided architectural turds, and we have to
deal with it.  My preference is to simplify dealing with it by getting
rid of vmalloc faults so that we can at least reliably touch percpu
memory without faulting.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230755

On Tue, Sep 22, 2015 at 11:37 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> kinds of mess.
>
> I don't think that anyone really wants to move #PF to IST, which means
> that we simply cannot handle vmalloc faults that happen when switching
> stacks after SYSCALL, no matter what fanciness we shove into the
> page_fault asm.

But that's fine. The kernel stack is special.  So yes, we want to make
sure that the kernel stack is always mapped in the thread whose stack
it is.

But that's not a big and onerous guarantee to make. Not when the
*real* problem is "random vmalloc allocations made by other processes
that we are not in the least interested in, and we don't want to add
synchronization for".

                         Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230762

On Tue, Sep 22, 2015 at 11:44 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Sep 22, 2015 at 11:37 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> kinds of mess.
>>
>> I don't think that anyone really wants to move #PF to IST, which means
>> that we simply cannot handle vmalloc faults that happen when switching
>> stacks after SYSCALL, no matter what fanciness we shove into the
>> page_fault asm.
>
> But that's fine. The kernel stack is special.  So yes, we want to make
> sure that the kernel stack is always mapped in the thread whose stack
> it is.
>
> But that's not a big and onerous guarantee to make. Not when the
> *real* problem is "random vmalloc allocations made by other processes
> that we are not in the least interested in, and we don't want to add
> synchronization for".
>

It's the kernel stack, the TSS (for sp0) and rsp_scratch at least.
But yes, that's not that onerous, and it's never lazily initialized
elsewhere.

How about this (long-term, not right now): Never free pgd entries.
For each pgd, track the number of populated kernel entries.  Also
track the global (init_mm) number of existing kernel entries.  At
context switch time, if new_pgd has fewer entries that the total, sync
it.

This hits *at most* 256 times per thread, and otherwise it's just a
single unlikely branch.  It guarantees that we only ever take a
vmalloc fault when accessing maps that didn't exist when we last
context switched, which gets us all of the important percpu stuff and
the kernel stack, even if we schedule onto a cpu that didn't exist
when the mm was created.

--Andy

>                          Linus



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web