On Sun, Jan 30, 2022 at 05:53:24PM +0000, Matthew Wilcox wrote: > On Sun, Jan 30, 2022 at 06:10:00PM +0200, Mike Rapoport wrote: > > On Fri, Jan 28, 2022 at 12:21:59PM +0100, Karolina Drobnik wrote: > > > On Thu, 2022-01-27 at 14:09 +0000, Matthew Wilcox wrote: > > > > On Thu, Jan 27, 2022 at 02:21:25PM +0100, Karolina Drobnik wrote: > > > > > Add a dummy io.h header. > > > > > > > > Rather begs the question of what memblock.c needs from linux/io.h. > > > > > > > > Wouldn't it be better to: > > > > > > > > +++ b/mm/memblock.c > > > > @@ -18,7 +18,6 @@ > > > > #include <linux/memblock.h> > > > > > > > > #include <asm/sections.h> > > > > -#include <linux/io.h> > > > > > > > > #include "internal.h" > > > > > > > > > > That was something I considered in the very beginning, but didn't have > > > a chance to verify it works for all architectures. I can take a look > > > after I'm finished with other v2 changes. > > > > > > > (allmodconfig on x86-64 builds fine with this; I have not done an > > > > extended sweep of other arches / build options). > > > > I did a sweep for defconfigs for all arches and all were fine. > > Thanks for doing the sweep, Mike. > > I think I found a deeper problem which is masked due to our maze of > header files: > > include/asm-generic/io.h:#ifndef virt_to_phys > include/asm-generic/io.h:#define virt_to_phys virt_to_phys > > so there's an assumption that <asm/io.h> defines virt_to_phys(). > You can see that in a number of architectures, eg: > > arch/alpha/include/asm/io.h:static inline unsigned long virt_to_phys(volatile void *address) > arch/ia64/include/asm/io.h:#define virt_to_phys virt_to_phys > arch/mips/include/asm/io.h:#define virt_to_phys virt_to_phys > arch/nios2/include/asm/io.h:#define virt_to_phys(vaddr) \ > arch/parisc/include/asm/io.h:#define virt_to_phys(a) ((unsigned long)__pa(a)) > arch/powerpc/include/asm/io.h:#define virt_to_phys virt_to_phys > arch/sh/include/asm/io.h:#define virt_to_phys(address) ((unsigned long)(address)) > arch/x86/include/asm/io.h:#define virt_to_phys virt_to_phys > > That's clearly not the right place to define it. Two architectures > put it in asm/memory.h: > > arch/arm/include/asm/memory.h:#define virt_to_phys virt_to_phys > arch/arm64/include/asm/memory.h:#define virt_to_phys virt_to_phys > > then: > > arch/m68k/include/asm/virtconvert.h:#define virt_to_phys virt_to_phys > arch/sparc/include/asm/page_32.h:#define virt_to_phys __pa > arch/sparc/include/asm/page_64.h:#define virt_to_phys __pa I'd say sparc picked the most appropriate header for it. memory.h could also work fine, but it's only present on some arches. I'll take a deeper look, thanks for checking this. > This needs to be properly sorted out, but I don't want to tell Karolina > that's now her job as a prerequisite for merging this patchset; that > would be unfair. Totally agree. > Cc'ing Arnd. This is the kind of awful mess that he loves fixing ;-) Heh, me too :) -- Sincerely yours, Mike.
