Date:   Mon, 30 Jul 2018 17:46:10 -0400
From:   Rik van Riel <riel@...riel.com>
To:     Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-team <kernel-team@...com>, X86 ML <x86@...nel.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Ingo Molnar <mingo@...nel.org>, Mike Galbraith <efault@....de>,
        Dave Hansen <dave.hansen@...el.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH v2 11/11] mm,sched: conditionally skip lazy TLB mm
 refcounting

On Mon, 2018-07-30 at 12:49 -0700, Andy Lutomirski wrote:

> 
> I think it's a big step in the right direction, but it still makes be
> nervous.  I'd be more comfortable with it if you at least had a
> functional set of patches that result in active_mm being gone,
> because
> that will mean that you actually audited the whole mess and fixed
> anything that might rely on active_mm pointing somewhere or that
> might
> be putting a value you didn't take into account into active_mm.  IOW
> I'm not totally thrilled by applying the patches as is if we're still
> a bit unsure as to what might have gotten missed.
> 
> I don't think it's at all necessary to redo the patches.
> 
> Does that seem reasonable?

Absolutely. I tried to keep ->active_mm very similar
to before for exactly that reason.

Lets go through all the places where it is used, in
x86 and architecture independent code. I have not
checked other architectures.

It looks like we should be able to get rid of
->active_mm at some point, but a lot of it depends
on other architecture maintainers.


arch/x86/events/core.c:
- get_segment_base: get current->active_mm->context.ldt,
  this appears to be for TIF_IA32 user programs only, so
  we should be able to use current->mm here

arch/x86/kernel/cpu/common.c:
- current task's ->active_mm assigned in two places,
  never read

arch/x86/lib/insn-eval.c:
- get_desc() gets current->active_mm->context.ldt, this
  appears to be only for user space programs

arch/x86/mm/tlb.c:
- this series adds two places where current->active_mm is
  written, it is never read

arch/x86/platform/efi/efi_64.c:
- current->active_mm is set to efi_mm for a little bit,
  with irqs disabled, and then changed back, with irqs still
  disabled; we should be able to get rid of ->active_mm here
- in the init code, ->active_mm is set to efi_mm as well,
  presumably the kernel automatically switches that back on
  the next context switch; this may be buggy, since preemption
  is enabled and a GFP_KERNEL allocation is just a few lines
  below

arch/x86/power/cpu.c:
- fix_processor_context() calls load_mm_ldt(current->active_mm);,
  we should be able to use cpu_tlbstate.loaded_mm instead

drivers/cpufreq/pmac32-cpufreq.c:
- pmu_set_cpu_speed() restores current->active_mm - don't know if
  anyone still cares about 32 bit PPC :)

drivers/firmware/efi/arm-runtime.c:
- efi_virtmap_unload switches back the pgd to current->active_mm
  from &efi_mm; that mm could be stored elsewhere if we excised
  ->active_mm everywhere

drivers/macintosh/via-pmu.c:
- same deal as pmap32-cpufreq.c above

mm/mmu_context.c:
- use_mm() tracks the ->active_mm a kernel thread is pointing to,
  but the mm is also tracked in ->mm
- unuse_mm() is the same deal as use_mm(), we should be able to
  get rid of ->active_mm if everybody stops using it, and we
  no longer refcount it anywhere

init/init_task.c:
- init_task.active_mm = &init_mm

fs/exec.c:
- exec_mmap() juggles both ->mm and ->active_mm, in order to
  get refcounting right; without refcounting we can lose ->active_mm


-- 
All Rights Reversed.
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists