Re: [PATCH 07/16] tools/include: Add io.h stub

From: Arnd Bergmann <arnd@arndb.de>
To: Matthew Wilcox <willy@infradead.org>
Cc: Mike Rapoport <rppt@kernel.org>,
	Karolina Drobnik <karolinadrobnik@gmail.com>,
	 Linux-MM <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Mike Rapoport <mike.rapoport@gmail.com>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,  Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH 07/16] tools/include: Add io.h stub
Date: Mon, 31 Jan 2022 14:30:32 +0100	[thread overview]
Message-ID: <CAK8P3a1UcY7Lkm0U5igQCq2K5kpqpjVi-sW3R=JzpciS-+c-gQ@mail.gmail.com> (raw)
In-Reply-To: <YfbQlMyohx31FhSW@casper.infradead.org>

On Sun, Jan 30, 2022 at 6:53 PM Matthew Wilcox <willy@infradead.org> wrote:

> 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
>
> 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.
>
> Cc'ing Arnd.  This is the kind of awful mess that he loves fixing ;-)

Adding Ingo as well. I'm in the middle of getting his fast-headers tree
to work well on a couple of other architectures, and the memory.h/page.h/io.h
mess is one of the  tricky bits in there, both in his series and in my
follow-ups.

What makes this bit even worse is that the architectures also not just
inconsistent about where they put __va/__pa and
virt_to_phys/phys_to_virt, they are also inconsistent about which of the
two pairs is based on the other, so any way you touch it means you
will break something, and changing it now will likely require a tricky
rebase of Ingo's patches.

Ingo, do you happen to have patches already that could be isolated
from your series to address this? Maybe we can add the
linux/mm_page_address.h header first and require that each
architecture puts these macros into asm/page_address.h.
We need to isolate these anyway, because the page addresses
are used in a lot of places that don't need to include any of the
remaining headers (page.h, mm.h, memory.h, io.h) that pull in
hundreds more.

          Arnd