Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753222AbcKTRHs (ORCPT ); Sun, 20 Nov 2016 12:07:48 -0500 Received: from mail-io0-f179.google.com ([209.85.223.179]:33985 "EHLO mail-io0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752145AbcKTRHq (ORCPT ); Sun, 20 Nov 2016 12:07:46 -0500 MIME-Version: 1.0 In-Reply-To: <20161117151805.GJ2151@rric.localdomain> References: <1475747527-32387-1-git-send-email-rrichter@cavium.com> <20161006161114.GH22012@rric.localdomain> <20161017185801.GT25086@rric.localdomain> <20161027160136.GD24290@arm.com> <20161028091905.GM22012@rric.localdomain> <20161107210514.GP20591@arm.com> <20161109195132.GZ22012@rric.localdomain> <20161117142528.GJ22855@arm.com> <20161117151805.GJ2151@rric.localdomain> From: Ard Biesheuvel Date: Sun, 20 Nov 2016 17:07:44 +0000 Message-ID: Subject: Re: [PATCH] arm64: mm: Fix memmap to be initialized for the entire section To: Robert Richter Cc: Will Deacon , Robert Richter , Mark Rutland , Catalin Marinas , David Daney , Hanjun Guo , "linux-arm-kernel@lists.infradead.org" , "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2136 Lines: 45 On 17 November 2016 at 15:18, Robert Richter wrote: > Thanks for your answer. > > On 17.11.16 14:25:29, Will Deacon wrote: >> On Wed, Nov 09, 2016 at 08:51:32PM +0100, Robert Richter wrote: >> > Thus, I don't see where my patch breaks code. Even acpi_os_ioremap() >> > keeps the same behaviour as before since it still uses memblock_is_ >> > memory(). Could you more describe your concerns why do you think this >> > patch breaks the kernel and moves the problem somewhere else? I >> > believe it fixes the problem at all. >> >> acpi_os_ioremap always ends up in __ioremap_caller, regardless of >> memblock_is_memory(). __ioremap_caller then fails if pfn_valid is true. > > But that's the reason my patch changed the code to use memblock_is_ > map_memory() instead. I was looking into the users of pfn_valid() esp. > in arm64 code and changed it where required. > > This week I looked into the kernel again for code that might break by > a pfn_valid() change. I found try_ram_remap() in memremap.c that has > changed behaviour now, but this is explicit for MEMREMAP_WB, so it > should be fine. > > Maybe it might be better to use page_is_ram() in addition to > pfn_valid() where necessary. This should work now after commit: > > e7cd190385d1 arm64: mark reserved memblock regions explicitly in iomem > > I still think pfn_valid() is not the correct use to determine the mem > attributes for mappings, there are further checks required. > > The risk of breaking something with my patch is small and limited only > to the mapping of efi reserved regions (which is the state of 4.4). If > something breaks anyway it can easily be fixed by adding more checks > to pfn_valid() as suggested above. > As I noted before, it looks to me like setting CONFIG_HOLES_IN_ZONE is the correct way to address this. However, doing that does uncover a bug in move_freepages() where the VM_BUG_ON_PAGE() dereferences struct page fields before the pfn_valid_within() check, so it seems those need to be switched around. Robert, you mentioned that CONFIG_HOLES_IN_ZONE seems inappropriate for sparsemem. Care to elaborate why?