Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752139AbbGMQzZ (ORCPT ); Mon, 13 Jul 2015 12:55:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36671 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752038AbbGMQzE (ORCPT ); Mon, 13 Jul 2015 12:55:04 -0400 Date: Mon, 13 Jul 2015 18:53:23 +0200 From: Oleg Nesterov To: "Kirill A. Shutemov" Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Dave Hansen , Andy Lutomirski , Thomas Gleixner Subject: Re: [PATCH 2/5] x86, mpx: do not set ->vm_ops on mpx VMAs Message-ID: <20150713165323.GA7906@redhat.com> References: <1436784852-144369-1-git-send-email-kirill.shutemov@linux.intel.com> <1436784852-144369-3-git-send-email-kirill.shutemov@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436784852-144369-3-git-send-email-kirill.shutemov@linux.intel.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4917 Lines: 156 On 07/13, Kirill A. Shutemov wrote: > > We don't really need ->vm_ops here: MPX VMA can be detected with VM_MPX > flag. And vma_merge() will not merge MPX VMA with non-MPX VMA, because > ->vm_flags won't match. Agreed. I am wondering if something like the patch below (on top of yours) makes sense... Not sure, but mpx_mmap() doesn't look nice too, and with this change we can unexport mmap_region(). Oleg. arch/x86/mm/mpx.c | 51 +++++++-------------------------------------------- include/linux/mm.h | 3 +++ mm/mmap.c | 16 +++++++++++----- 3 files changed, 21 insertions(+), 49 deletions(-) diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index 4d1c11c..da8b713 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -24,58 +24,21 @@ */ static unsigned long mpx_mmap(unsigned long len) { - unsigned long ret; - unsigned long addr, pgoff; + unsigned long addr, populate; struct mm_struct *mm = current->mm; - vm_flags_t vm_flags; - struct vm_area_struct *vma; /* Only bounds table and bounds directory can be allocated here */ if (len != MPX_BD_SIZE_BYTES && len != MPX_BT_SIZE_BYTES) return -EINVAL; down_write(&mm->mmap_sem); - - /* Too many mappings? */ - if (mm->map_count > sysctl_max_map_count) { - ret = -ENOMEM; - goto out; - } - - /* Obtain the address to map to. we verify (or select) it and ensure - * that it represents a valid section of the address space. - */ - addr = get_unmapped_area(NULL, 0, len, 0, MAP_ANONYMOUS | MAP_PRIVATE); - if (addr & ~PAGE_MASK) { - ret = addr; - goto out; - } - - vm_flags = VM_READ | VM_WRITE | VM_MPX | - mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC; - - /* Set pgoff according to addr for anon_vma */ - pgoff = addr >> PAGE_SHIFT; - - ret = mmap_region(NULL, addr, len, vm_flags, pgoff); - if (IS_ERR_VALUE(ret)) - goto out; - - vma = find_vma(mm, ret); - if (!vma) { - ret = -ENOMEM; - goto out; - } - - if (vm_flags & VM_LOCKED) { - up_write(&mm->mmap_sem); - mm_populate(ret, len); - return ret; - } - -out: + addr = __do_mmap_pgoff(NULL, 0, len, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, &populate, VM_MPX); up_write(&mm->mmap_sem); - return ret; + if (populate) + mm_populate(addr, populate); + + return addr; } enum reg_type { diff --git a/include/linux/mm.h b/include/linux/mm.h index 0207ffa..910d475 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1849,6 +1849,9 @@ extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned lo extern unsigned long mmap_region(struct file *file, unsigned long addr, unsigned long len, vm_flags_t vm_flags, unsigned long pgoff); +extern unsigned long __do_mmap_pgoff(struct file *file, unsigned long addr, + unsigned long len, unsigned long prot, unsigned long flags, + unsigned long pgoff, unsigned long *populate, vm_flags_t vm_flags); extern unsigned long do_mmap_pgoff(struct file *file, unsigned long addr, unsigned long len, unsigned long prot, unsigned long flags, unsigned long pgoff, unsigned long *populate); diff --git a/mm/mmap.c b/mm/mmap.c index 2185cd9..88bc961 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1247,14 +1247,12 @@ static inline int mlock_future_check(struct mm_struct *mm, /* * The caller must hold down_write(¤t->mm->mmap_sem). */ - -unsigned long do_mmap_pgoff(struct file *file, unsigned long addr, +unsigned long __do_mmap_pgoff(struct file *file, unsigned long addr, unsigned long len, unsigned long prot, unsigned long flags, unsigned long pgoff, - unsigned long *populate) + unsigned long *populate, vm_flags_t vm_flags) { struct mm_struct *mm = current->mm; - vm_flags_t vm_flags; *populate = 0; @@ -1298,7 +1296,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr, * to. we assume access permissions have been handled by the open * of the memory object, so we don't do any here. */ - vm_flags = calc_vm_prot_bits(prot) | calc_vm_flag_bits(flags) | + vm_flags |= calc_vm_prot_bits(prot) | calc_vm_flag_bits(flags) | mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC; if (flags & MAP_LOCKED) @@ -1396,6 +1394,14 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr, return addr; } +unsigned long do_mmap_pgoff(struct file *file, unsigned long addr, + unsigned long len, unsigned long prot, + unsigned long flags, unsigned long pgoff, + unsigned long *populate) +{ + return __do_mmap_pgoff(file, addr, len, prot, flags, pgoff, populate, 0); +} + SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len, unsigned long, prot, unsigned long, flags, unsigned long, fd, unsigned long, pgoff) -- 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/