On 17 November 2016 at 15:18, Robert Richter <robert.richter@xxxxxxxxxx> 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? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
