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


Groups > linux.kernel > #1330207

Re: [PATCH 01/31] mm, gup: introduce concept of "foreign" get_user_pages()

From Ingo Molnar <mingo@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 01/31] mm, gup: introduce concept of "foreign" get_user_pages()
Date 2016-02-09 13:50 +0100
Message-ID <r0fTY-44E-15@gated-at.bofh.it> (permalink)
References <qWlOi-75l-7@gated-at.bofh.it> <qWlXY-79P-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


* Dave Hansen <dave@sr71.net> wrote:

> 
> OK, so I've fixed up my build process to _actually_ build the
> nommu code.
> 
> One of Vlastimil's comments made me go dig back in to the uprobes
> code's use of get_user_pages().  I decided to change both of them
> to be "foreign" accesses.
> 
> This also fixes the nommu breakage that Vlastimil noted last time.
> 
> Srikar, I'd appreciate if you can have a look at the uprobes.c
> modifications, especially the comment.  I don't think this will
> change any behavior, but I want to make sure the comment is
> accurate.
> 
> ---
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> For protection keys, we need to understand whether protections
> should be enforced in software or not.  In general, we enforce
> protections when working on our own task, but not when on others.
> We call these "current" and "foreign" operations.
> 
> This patch introduces a new get_user_pages() variant:
> 
> 	get_user_pages_foreign()
> 
> We modify the vanilla get_user_pages() so it can no longer be
> used on mm/tasks other than 'current/current->mm', which is by
> far the most common way it is called.  Using it makes a few of
> the call sites look a bit nicer.
> 
> In other words, get_user_pages_foreign() is a replacement for
> when get_user_pages() is called on non-current tsk/mm.
> 
> This also switches get_user_pages_(un)locked() over to be like
> get_user_pages() and not take a tsk/mm.  There is no
> get_user_pages_foreign_(un)locked().  If someone wants that
> behavior they just have to use "__" variant and pass in
> FOLL_FOREIGN explicitly.
> 
> The uprobes is_trap_at_addr() location holds mmap_sem and
> calls get_user_pages(current->mm) on an instruction address.  This
> makes it a pretty unique gup caller.  Being an instruction access
> and also really originating from the kernel (vs. the app), I opted
> to consider this a 'foreign' access where protection keys will not
> be enforced.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: jack@suse.cz
> ---
> 
>  b/arch/cris/arch-v32/drivers/cryptocop.c        |    8 ---
>  b/arch/ia64/kernel/err_inject.c                 |    3 -
>  b/arch/mips/mm/gup.c                            |    3 -
>  b/arch/s390/mm/gup.c                            |    4 -
>  b/arch/sh/mm/gup.c                              |    2 
>  b/arch/sparc/mm/gup.c                           |    2 
>  b/arch/x86/mm/gup.c                             |    2 
>  b/arch/x86/mm/mpx.c                             |    4 -
>  b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       |    3 -
>  b/drivers/gpu/drm/etnaviv/etnaviv_gem.c         |    2 
>  b/drivers/gpu/drm/i915/i915_gem_userptr.c       |    2 
>  b/drivers/gpu/drm/radeon/radeon_ttm.c           |    3 -
>  b/drivers/gpu/drm/via/via_dmablit.c             |    3 -
>  b/drivers/infiniband/core/umem.c                |    2 
>  b/drivers/infiniband/core/umem_odp.c            |    8 +--
>  b/drivers/infiniband/hw/mthca/mthca_memfree.c   |    3 -
>  b/drivers/infiniband/hw/qib/qib_user_pages.c    |    3 -
>  b/drivers/infiniband/hw/usnic/usnic_uiom.c      |    2 
>  b/drivers/media/pci/ivtv/ivtv-udma.c            |    4 -
>  b/drivers/media/pci/ivtv/ivtv-yuv.c             |   10 +---
>  b/drivers/media/v4l2-core/videobuf-dma-sg.c     |    3 -
>  b/drivers/misc/mic/scif/scif_rma.c              |    2 
>  b/drivers/misc/sgi-gru/grufault.c               |    3 -
>  b/drivers/scsi/st.c                             |    2 
>  b/drivers/staging/rdma/ipath/ipath_user_pages.c |    3 -
>  b/drivers/video/fbdev/pvr2fb.c                  |    4 -
>  b/drivers/virt/fsl_hypervisor.c                 |    5 --
>  b/fs/exec.c                                     |    8 ++-
>  b/include/linux/mm.h                            |   21 +++++----
>  b/kernel/events/uprobes.c                       |   10 +++-
>  b/mm/frame_vector.c                             |    2 
>  b/mm/gup.c                                      |   52 +++++++++++++++---------
>  b/mm/ksm.c                                      |    2 
>  b/mm/memory.c                                   |    2 
>  b/mm/mempolicy.c                                |    6 +-
>  b/mm/nommu.c                                    |   30 ++++++++-----
>  b/mm/process_vm_access.c                        |   11 +++--
>  b/mm/util.c                                     |    4 -
>  b/net/ceph/pagevec.c                            |    2 
>  b/security/tomoyo/domain.c                      |    9 +++-
>  b/virt/kvm/async_pf.c                           |    7 ++-
>  b/virt/kvm/kvm_main.c                           |   10 ++--
>  42 files changed, 148 insertions(+), 123 deletions(-)

So this patch conflicts with recent upstream changes:

  patching file drivers/scsi/st.c
  can't find file to patch at input line 463

mind respinning it against v4.5-rc3 or so?

Also, please split this into three patches:

 - one patch adds the _foreign() GUP variant and applies it to code that uses it
   on remote tasks.

 - introduce the new get_user_pages() but also add macros so that both 8-parameter 
   and 7-parameter variants work without breaking the build. We can remove the 
   compatibility wrapping on v4.6 or so.

 - the third will be a large but trivial patch, which will change 8-parameter GUP 
   usage to 7-parameter usage.

... this should reduce the pain from the GUP interface change churn.

Agreed?

Thanks,

	Ingo

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/31] x86: Memory Protection Keys (v9) Dave Hansen <dave@sr71.net> - 2016-01-29 19:20 +0100
  [PATCH 21/31] x86, pkeys: dump PKRU with other kernel registers Dave Hansen <dave@sr71.net> - 2016-01-29 19:20 +0100
  [PATCH 26/31] x86, pkeys: add arch_validate_pkey() Dave Hansen <dave@sr71.net> - 2016-01-29 19:20 +0100
  [PATCH 29/31] x86, pkeys: allow kernel to modify user pkey rights register Dave Hansen <dave@sr71.net> - 2016-01-29 19:20 +0100
  [PATCH 03/31] x86, pkeys: Add Kconfig option Dave Hansen <dave@sr71.net> - 2016-01-29 19:20 +0100
  [PATCH 09/31] x86, pkeys: store protection in high VMA flags Dave Hansen <dave@sr71.net> - 2016-01-29 19:20 +0100
  [PATCH 05/31] x86, pkeys: define new CR4 bit Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 23/31] x86, pkeys: add Kconfig prompt to existing config option Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 12/31] signals, pkeys: notify userspace about protection key faults Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 24/31] x86, pkeys: actually enable Memory Protection Keys in CPU Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 15/31] mm: factor out VMA fault permission checking Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 20/31] x86, pkeys: differentiate instruction fetches Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 18/31] mm: do not enforce PKEY permissions on "foreign" mm access Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 08/31] x86, pkeys: new page fault error code bit: PF_PK Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 14/31] x86, pkeys: add functions to fetch PKRU Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 04/31] x86, pkeys: cpuid bit definition Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 10/31] x86, pkeys: arch-specific protection bits Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 13/31] x86, pkeys: fill in pkey field in siginfo Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 25/31] mm, multi-arch: pass a protection key in to calc_vm_flag_bits() Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 17/31] x86, pkeys: check VMAs and PTEs for protection keys Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 07/31] x86, pkeys: PTE bits for storing protection key Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 02/31] x86, fpu: add placeholder for Processor Trace XSAVE state Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 16/31] x86, mm: simplify get_user_pages() PTE bit handling Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 11/31] x86, pkeys: pass VMA down in to fault signal generation code Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  [PATCH 22/31] x86, pkeys: dump pkey from VMA in /proc/pid/smaps Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
    Re: [PATCH 22/31] x86, pkeys: dump pkey from VMA in /proc/pid/smaps Vlastimil Babka <vbabka@suse.cz> - 2016-02-02 17:20 +0100
      Re: [PATCH 22/31] x86, pkeys: dump pkey from VMA in /proc/pid/smaps Dave Hansen <dave@sr71.net> - 2016-02-02 20:10 +0100
  [PATCH 06/31] x86, pkeys: add PKRU xsave fields and data structure(s) Dave Hansen <dave@sr71.net> - 2016-01-29 19:30 +0100
  Re: [PATCH 01/31] mm, gup: introduce concept of "foreign"  get_user_pages() Ingo Molnar <mingo@kernel.org> - 2016-02-09 13:50 +0100
    Re: [PATCH 01/31] mm, gup: introduce concept of "foreign"  get_user_pages() Ingo Molnar <mingo@kernel.org> - 2016-02-09 14:10 +0100
    Re: [PATCH 01/31] mm, gup: introduce concept of "foreign"  get_user_pages() Dave Hansen <dave@sr71.net> - 2016-02-09 16:20 +0100

csiph-web