Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752900AbcDUXfX (ORCPT ); Thu, 21 Apr 2016 19:35:23 -0400 Received: from g4t3428.houston.hp.com ([15.201.208.56]:39824 "EHLO g4t3428.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751791AbcDUXfW (ORCPT ); Thu, 21 Apr 2016 19:35:22 -0400 Subject: Re: [PATCH v3 0/2] Align mmap address for DAX pmd mappings To: Matthew Wilcox References: <1460652511-19636-1-git-send-email-toshi.kani@hpe.com> <20160415220531.c7b55adb5b26eb749fae3186@linux-foundation.org> <20160418202610.GA17889@quack2.suse.cz> <20160419182347.GA29068@linux.intel.com> <571844A1.5080703@hpe.com> <20160421070625.GB29068@linux.intel.com> Cc: Jan Kara , Andrew Morton , dan.j.williams@intel.com, viro@zeniv.linux.org.uk, ross.zwisler@linux.intel.com, kirill.shutemov@linux.intel.com, david@fromorbit.com, tytso@mit.edu, adilger.kernel@dilger.ca, linux-nvdimm@ml01.01.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org From: Toshi Kani Message-ID: <571963B5.7050009@hpe.com> Date: Thu, 21 Apr 2016 19:35:17 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <20160421070625.GB29068@linux.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2315 Lines: 67 On 4/21/2016 3:06 AM, Matthew Wilcox wrote: > On Wed, Apr 20, 2016 at 11:10:25PM -0400, Toshi Kani wrote: >> How about moving the function (as is) to mm/huge_memory.c, rename it to >> get_hugepage_unmapped_area(), which is defined to NULL in huge_mm.h >> when TRANSPARENT_HUGEPAGE is unset? > Great idea. Perhaps it should look something like this? Yes, it looks good! I will use it. :-) > > unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr, > unsigned long len, unsigned long pgoff, unsigned long flags) > { > loff_t off, off_end, off_pmd; > unsigned long len_pmd, addr_pmd; > > if (addr) > goto out; > if (IS_DAX(filp->f_mapping->host) && !IS_ENABLED(CONFIG_FS_DAX_PMD)) > goto out; > /* Kirill, please fill in the right condition here for THP pagecache */ > > off = (loff_t)pgoff << PAGE_SHIFT; > off_end = off + len; > off_pmd = round_up(off, PMD_SIZE); /* pmd-aligned start offset */ > > if ((off_end <= off_pmd) || ((off_end - off_pmd) < PMD_SIZE)) > goto out; > > len_pmd = len + PMD_SIZE; > if ((off + len_pmd) < off) > goto out; > > addr_pmd = current->mm->get_unmapped_area(filp, NULL, len_pmd, > pgoff, flags); > if (!IS_ERR_VALUE(addr_pmd)) { > addr_pmd += (off - addr_pmd) & (PMD_SIZE - 1); > return addr_pmd; > } > out: > return current->mm->get_unmapped_area(filp, addr, len, pgoff, flags); > } > > - I deleted the check for filp == NULL. It can't be NULL ... this is a > file_operation ;-) Right. > - Why is len_pmd len + PMD_SIZE instead of round_up(len, PMD_SIZE)? The length is padded with an extra-PMD size so that any assigned address 'addr_pmd' can be aligned by PMD. IOW, it does not make an assumption that addr_pmd is aligned by the length. > - I'm still in two minds about passing 'addr' to the first call to > get_unmapped_area() instead of NULL. When 'addr' is specified, we need to use 'len' since user may be managing free VMA range by itself. So, I think falling back with the original args is correct. Thanks, -Toshi