From patchwork Sat Sep 19 06:49:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Baker X-Patchwork-Id: 7221921 Return-Path: X-Original-To: patchwork-linux-nvdimm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 4AFE19F336 for ; Sat, 19 Sep 2015 06:49:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 386CB2097E for ; Sat, 19 Sep 2015 06:49:14 +0000 (UTC) Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5CB3B20995 for ; Sat, 19 Sep 2015 06:49:12 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 1F829613F2; Fri, 18 Sep 2015 23:49:12 -0700 (PDT) X-Original-To: linux-nvdimm@lists.01.org Delivered-To: linux-nvdimm@lists.01.org Received: from mail-qg0-f45.google.com (mail-qg0-f45.google.com [209.85.192.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E9DB061379 for ; Fri, 18 Sep 2015 23:49:10 -0700 (PDT) Received: by qgev79 with SMTP id v79so55329980qge.0 for ; Fri, 18 Sep 2015 23:49:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=MSuNxst37R2GWOI5ttj8Er0OxB+IdEvX0Lp3G1mUiXI=; b=ZnM5a+8u8eFWRJM1WMPdBuOY3CXdrsWTjS5+36FQk03V0bHJ5TOytlN8H6wN2IqLAL TZ9tIGqeWJTm00q2OzywGIqgO91uTflvi6v4Np20+UzZdWXdgROFIjOql+i9JaG/AI2l jjgU6dui5XDllRS38X13DqUBirpYvcCRSzAZMg8inAQxFGrWH5uBZGPQKNY7sge97k/Z fJK3YbP6Pe62MDhkUQU6xDi8dna+yvPmgzqVeqcskW6Xg5cqJplz3Oezr7Bv0Y/zvJqa ySpsVCRpuRAkp0Dk4wk7QzS2RI4bCURcjZrASlXHb89WSssPGbYICl44k7zgZPUYBj7m urAA== X-Gm-Message-State: ALoCoQkPx2wSfG8FgMfGDmqV5Adl6N4HzbsDSx2RW16bM8tIIJOc9bvktdp4zVdkNO40i4hqHG/w MIME-Version: 1.0 X-Received: by 10.140.99.49 with SMTP id p46mr10613706qge.76.1442645349865; Fri, 18 Sep 2015 23:49:09 -0700 (PDT) Received: by 10.55.3.21 with HTTP; Fri, 18 Sep 2015 23:49:09 -0700 (PDT) In-Reply-To: References: <20150826010220.8851.18077.stgit@dwillia2-desk3.amr.corp.intel.com> <20150826012735.8851.49787.stgit@dwillia2-desk3.amr.corp.intel.com> Date: Fri, 18 Sep 2015 23:49:09 -0700 Message-ID: Subject: Re: [PATCH v2 2/9] mm: move __phys_to_pfn and __pfn_to_phys to asm/generic/memory_model.h From: Tyler Baker To: Dan Williams Cc: "linux-nvdimm@lists.01.org" , Kevin's boot bot , david , "linux-kernel@vger.kernel.org" , Ingo Molnar , Linux MM , "H. Peter Anvin" , Christoph Hellwig X-BeenThere: linux-nvdimm@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Linux-nvdimm developer list." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 18 September 2015 at 16:59, Dan Williams wrote: > On Fri, Sep 18, 2015 at 4:42 PM, Tyler Baker wrote: >> Hi, >> >> On 25 August 2015 at 18:27, Dan Williams wrote: >>> From: Christoph Hellwig >>> >>> Three architectures already define these, and we'll need them genericly >>> soon. >>> >>> Signed-off-by: Christoph Hellwig >>> Signed-off-by: Dan Williams >>> --- >>> arch/arm/include/asm/memory.h | 6 ------ >>> arch/arm64/include/asm/memory.h | 6 ------ >>> arch/unicore32/include/asm/memory.h | 6 ------ >>> include/asm-generic/memory_model.h | 6 ++++++ >>> 4 files changed, 6 insertions(+), 18 deletions(-) >>> >>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h >>> index b7f6fb462ea0..98d58bb04ac5 100644 >>> --- a/arch/arm/include/asm/memory.h >>> +++ b/arch/arm/include/asm/memory.h >>> @@ -119,12 +119,6 @@ >>> #endif >>> >>> /* >>> - * Convert a physical address to a Page Frame Number and back >>> - */ >>> -#define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT)) >>> -#define __pfn_to_phys(pfn) ((phys_addr_t)(pfn) << PAGE_SHIFT) >>> - >>> -/* >>> * Convert a page to/from a physical address >>> */ >>> #define page_to_phys(page) (__pfn_to_phys(page_to_pfn(page))) >>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h >>> index f800d45ea226..d808bb688751 100644 >>> --- a/arch/arm64/include/asm/memory.h >>> +++ b/arch/arm64/include/asm/memory.h >>> @@ -81,12 +81,6 @@ >>> #define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET + PAGE_OFFSET)) >>> >>> /* >>> - * Convert a physical address to a Page Frame Number and back >>> - */ >>> -#define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT)) >>> -#define __pfn_to_phys(pfn) ((phys_addr_t)(pfn) << PAGE_SHIFT) >>> - >>> -/* >>> * Convert a page to/from a physical address >>> */ >>> #define page_to_phys(page) (__pfn_to_phys(page_to_pfn(page))) >>> diff --git a/arch/unicore32/include/asm/memory.h b/arch/unicore32/include/asm/memory.h >>> index debafc40200a..3bb0a29fd2d7 100644 >>> --- a/arch/unicore32/include/asm/memory.h >>> +++ b/arch/unicore32/include/asm/memory.h >>> @@ -61,12 +61,6 @@ >>> #endif >>> >>> /* >>> - * Convert a physical address to a Page Frame Number and back >>> - */ >>> -#define __phys_to_pfn(paddr) ((paddr) >> PAGE_SHIFT) >>> -#define __pfn_to_phys(pfn) ((pfn) << PAGE_SHIFT) >>> - >>> -/* >>> * Convert a page to/from a physical address >>> */ >>> #define page_to_phys(page) (__pfn_to_phys(page_to_pfn(page))) >>> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h >>> index 14909b0b9cae..f20f407ce45d 100644 >>> --- a/include/asm-generic/memory_model.h >>> +++ b/include/asm-generic/memory_model.h >>> @@ -69,6 +69,12 @@ >>> }) >>> #endif /* CONFIG_FLATMEM/DISCONTIGMEM/SPARSEMEM */ >>> >>> +/* >>> + * Convert a physical address to a Page Frame Number and back >>> + */ >>> +#define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT)) >>> +#define __pfn_to_phys(pfn) ((pfn) << PAGE_SHIFT) >> >> The kernelci.org bot has been reporting complete boot failures[1] on >> ARM platforms with more than 4GB of memory and LPAE enabled. I've >> bisected[2] the failures down to this commit, and reverting it on top >> of the latest mainline resolves the boot issue. I took a closer look >> at this patch and noticed the cast to phys_addr_t was dropped in the >> generic function. Adding this to the new generic function solves the >> boot issue I'm reporting. >> >> diff --git a/include/asm-generic/memory_model.h >> b/include/asm-generic/memory_model.h >> index f20f407..db9f5c7 100644 >> --- a/include/asm-generic/memory_model.h >> +++ b/include/asm-generic/memory_model.h >> @@ -73,7 +73,7 @@ >> * Convert a physical address to a Page Frame Number and back >> */ >> #define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT)) >> -#define __pfn_to_phys(pfn) ((pfn) << PAGE_SHIFT) >> +#define __pfn_to_phys(pfn) ((phys_addr_t)(pfn) << PAGE_SHIFT) >> >> #define page_to_pfn __page_to_pfn >> #define pfn_to_page __pfn_to_page >> >> If this fix is valid, I can send a formal patch or it can be squashed >> into the original commit. > > This fix is valid, but I wonder if it should just use the existing > PFN_PHYS() definition in include/linux/pfn.h? Good suggestion, seems like the rational thing to do. FWIW, I gave the patch below a spin through the kernelci.org build/boot ci loop[1]. All went well, no new regressions were detected. From: Tyler Baker Date: Fri, 18 Sep 2015 17:56:26 -0700 Subject: [PATCH] mm: fix type cast in __pfn_to_phys() The various definitions of __pfn_to_phys() have been consolidated to use a generic macro in include/asm-generic/memory_model.h. This hit mainline in the form of 012dcef3f058 "mm: move __phys_to_pfn and __pfn_to_phys to asm/generic/memory_model.h". When the generic macro was implemented the type cast to phys_addr_t was dropped which caused boot regressions on ARM platforms with more than 4GB of memory and LPAE enabled. It was suggested to use PFN_PHYS() defined in include/linux/pfn.h as provides the correct logic and avoids further duplication. Reported-by: kernelci.org bot Suggested-by: Dan Williams Signed-off-by: Tyler Baker --- include/asm-generic/memory_model.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h index f20f407..4b4b056 100644 --- a/include/asm-generic/memory_model.h +++ b/include/asm-generic/memory_model.h @@ -73,7 +73,7 @@ * Convert a physical address to a Page Frame Number and back */ #define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT)) -#define __pfn_to_phys(pfn) ((pfn) << PAGE_SHIFT) +#define __pfn_to_phys(pfn) PFN_PHYS(pfn) #define page_to_pfn __page_to_pfn #define pfn_to_page __pfn_to_page