On Mon, May 04, 2020 at 01:17:41PM -0700, Ira Weiny wrote: > > || * arm: much, much worse. We have several files that pull linux/highmem.h: > > || arch/arm/mm/cache-feroceon-l2.c, arch/arm/mm/cache-xsc3l2.c, > > || arch/arm/mm/copypage-*.c, arch/arm/mm/dma-mapping.c, arch/arm/mm/flush.c, > > || arch/arm/mm/highmem.c, arch/arm/probes/uprobes/core.c, > > || arch/arm/include/asm/kvm_mmu.h (kmap_atomic_pfn()). > > || Those are fine, but we also have this: > > || arch/arm/include/asm/pgtable.h:200:#define __pte_map(pmd) (pte_t *)kmap_atomic(pmd_page(*(pmd))) > > || arch/arm/include/asm/pgtable.h:208:#define pte_offset_map(pmd,addr) (__pte_map(pmd) + pte_index(addr)) > > || and sure as hell, asm/pgtable.h does *NOT* pull linux/highmem.h. > > It does not pull asm/highmem.h either... No, but the users of those macros need to be considered. > > || #define pte_offset_map(dir, addr) \ > > || ((pte_t *) kmap_atomic(pmd_page(*(dir))) + pte_index(addr)) > > || One pte_offset_map user in arch/microblaze: > > || arch/microblaze/kernel/signal.c:207: ptep = pte_offset_map(pmdp, address); > > || Messy, but doesn't require any changes (we have asm/pgalloc.h included > > || there, and that pull linux/highmem.h). > > AFAICS asm/pgtable.h does not include asm/highmem.h here... > > So looks like arch/microblaze/kernel/signal.c will need linux/highmem.h See above - line 39 in there is #include <asm/pgalloc.h> and line 14 in arch/microblaze/include/asm/pgalloc.h is #include <linux/highmem.h> It's conditional upon CONFIG_MMU in there, but so's the use of pte_offset_map() in arch/microblaze/kernel/signal.c So it shouldn't be a problem. > > || * xtensa: users in arch/xtensa/kernel/pci-dma.c, arch/xtensa/mm/highmem.c, > > || arch/xtensa/mm/cache.c and arch/xtensa/platforms/iss/simdisk.c (all pull > > || linux/highmem.h). > > Actually > > arch/xtensa/mm/cache.c gets linux/highmem.h from linux/pagemap.h > > arch/xtensa/platforms/iss/simdisk.c may have an issue? > linux/blkdev.h -> CONFIG_BLOCK -> linux/pagemap.h -> linux/highmem.h > But simdisk.c requires BLK_DEV_SIMDISK -> CONFIG_BLOCK... > <sigh> Yep - see above re major chain of indirect includes conditional upon CONFIG_BLOCK and its uses in places that only build with such configs. There's a plenty of similar considerations outside of arch/*, unfortunately...