From patchwork Thu May 1 19:23:09 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 4100151 Return-Path: X-Original-To: patchwork-linux-parisc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5E8489F271 for ; Thu, 1 May 2014 19:23:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 73AA020340 for ; Thu, 1 May 2014 19:23:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5174620328 for ; Thu, 1 May 2014 19:23:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751269AbaEATXL (ORCPT ); Thu, 1 May 2014 15:23:11 -0400 Received: from mail-vc0-f176.google.com ([209.85.220.176]:54161 "EHLO mail-vc0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751259AbaEATXK (ORCPT ); Thu, 1 May 2014 15:23:10 -0400 Received: by mail-vc0-f176.google.com with SMTP id lg15so1591727vcb.21 for ; Thu, 01 May 2014 12:23:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=fQN+Y+yIJfsqxLERPn1jtcJ7EzrpXDToQpys0MNq7Bc=; b=P56AUELbzoiJSc8A3WbMSYHURjMkgmKJh/Ve7vgFsa8xBWlfLhu32LOjPeVklrZcLO UtPNOKLCKqkOjN2WWf9b4aSsaA1i4I8IWfI4HCtLai07R+OjMZJ0+P9Xv6S3ea0PVC4/ iDSlbodlAn/J9hoXH5jueQCaT6hls772NogQMRMJbqWx/zuge2FGk6fNsEGAHIStmnlC X3RbXVV7MsOrhEpgX+y6OOCfWg3TtME5H36P+98ITUN/wC0F9H317Bebto/vaGdD3gqZ LgnHRHVd20x++XRwuVjGe58E5H+aKgH97otdkg9vTcO0Gfg/so+DsRVTGr/O8Mc1L6YH YM5Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=fQN+Y+yIJfsqxLERPn1jtcJ7EzrpXDToQpys0MNq7Bc=; b=bebOFTjXSm46DwMkyzzOcKUjDgujnUc9C+H3nCY8cw6f6jBfkZ8SaCh7ujh3ZQcHfO SMw57wUVh9ferWcziOG94dO2fq9Gac+Lm320qeJKoacC/C8VN/UOhFNWJaifD0IQv1rA 9Nj8qSnwsypTeDIvhJ2ahWLxxFvgXJYTSJeJw= MIME-Version: 1.0 X-Received: by 10.220.2.142 with SMTP id 14mr895893vcj.48.1398972189821; Thu, 01 May 2014 12:23:09 -0700 (PDT) Received: by 10.220.13.2 with HTTP; Thu, 1 May 2014 12:23:09 -0700 (PDT) In-Reply-To: <20140501183822.GA24823@p100.fritz.box> References: <20140501183822.GA24823@p100.fritz.box> Date: Thu, 1 May 2014 12:23:09 -0700 X-Google-Sender-Auth: ZPmp1NSKGpdVNrx73S2xPMuAEC0 Message-ID: Subject: Re: [GIT PULL] parisc updates for v3.15 From: Linus Torvalds To: Helge Deller Cc: Linux Kernel Mailing List , Parisc List , James Bottomley Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, May 1, 2014 at 11:38 AM, Helge Deller wrote: > > - Make mmap() behave similiar to other architectures: If a file hasn't been > mapped yet, we can now map it at any given page-aligned address. Ugh, so I pulled this, but I'm going to unpull it, because I dislike your new "i_mmap_lastmap" field. Now, the i_mmap_lastmap field itself, I could probably live with, but this change to generic code: + if (vma->vm_flags & VM_SHARED) { mapping->i_mmap_writable--; +#ifdef CONFIG_MMAP_TRACKING + if (mapping->i_mmap_writable == 0) + mapping->i_mmap_lastmmap = 0; +#endif + } makes me just gouge my eyes out. It's not only uglifying generic code, it's _stupid_ even when it's used. What's magic about "i_mmap_lastmmap" having the value zero? Nothing. Maybe somebody wants to map stuff at that zero value, and has the permissions to do so. So zeroing it is wrong. It's also entirely unnecessary, since you can just look at the "mapping->i_mmap_writable" value instead. So instead of checking "is i_mmap_lastmmap zero" as a way to check whether you can now use any virtual address, which is wrong _anyway_, you should have checked "is i_mmap_writable zero". Now, I *also* suspect that you could just get rid of i_mmap_lastmmap _entirely_, and instead just make "GET_LAST_MMAP()" just look up the first shared mapping it can find in the rb tree (if i_mmap_writable is non-null). But if it was just that one (unnecessary) field in the "struct address_space", I probably wouldn't mind, and say "ok, parisc has broken virtual caches, what else is new". But the fact that it adds code to the generic file just adds insult to injury and makes me go "no, I don't want to pull this". Mind fixing this? Here's a TOTALLY UNTESTED patch that may or may not work, but might at least act as a starting point. Hmm? Linus arch/parisc/kernel/sys_parisc.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c index 31ffa9b55322..dd643459cbef 100644 --- a/arch/parisc/kernel/sys_parisc.c +++ b/arch/parisc/kernel/sys_parisc.c @@ -36,12 +36,32 @@ #include #include -/* we construct an artificial offset for the mapping based on the physical - * address of the kernel mapping variable */ -#define GET_LAST_MMAP(filp) \ - (filp ? ((unsigned long) filp->f_mapping) >> 8 : 0UL) -#define SET_LAST_MMAP(filp, val) \ - { /* nothing */ } +static inline unsigned long find_shared_mapping_address(struct address_space *mapping) +{ + struct rb_node *nd; + + for (nd = rb_first(root); nd; nd = rb_next(nd)) { + struct vm_area_struct *vma; + vma = rb_entry(nd, struct vm_area_struct, vm_rb); + if (vma->vm_flags & VM_SHARED) + return vma->start; + } + /* Shouldn't happen */ + return 0; +} + +/* the address_space struct holds a field i_mmap_lastmmap with the last mapping + * of this file for us */ +static inline unsigned long GET_LAST_MMAP(struct file *filp) +{ + if (filp) { + struct address_space *mapping = filp->f_mapping; + if (mapping->i_mmap_writable) + return find_shared_mapping_address(mapping); + } + return 0; +} + static int get_offset(unsigned int last_mmap) { @@ -127,9 +147,6 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, addr = vm_unmapped_area(&info); found_addr: - if (do_color_align && !last_mmap && !(addr & ~PAGE_MASK)) - SET_LAST_MMAP(filp, addr - (pgoff << PAGE_SHIFT)); - return addr; } @@ -198,9 +215,6 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, return arch_get_unmapped_area(filp, addr0, len, pgoff, flags); found_addr: - if (do_color_align && !last_mmap && !(addr & ~PAGE_MASK)) - SET_LAST_MMAP(filp, addr - (pgoff << PAGE_SHIFT)); - return addr; }