Re: [PATCH 1/1] x86/ioremap: Use is_vmalloc_addr in iounmap

[Dan Williams]

Thomas Gleixner wrote:
> On Thu, Aug 10 2023 at 13:00, Max Ramanouski wrote:
> 
> > On systems that use HMM (most notably amdgpu driver)
> > high_memory can jump over VMALLOC_START. That causes
> > some iounmap to exit early. This in addition to leaking,
> > causes problems with rebinding devices to vfio_pci from
> > other drivers with error of conflicting memtypes,
> > as they aren't freed in iounmap.
> >
> > Replace comparison against high_memory with is_vmalloc_addr to
> > fix the issue and make x86 iounmap implementation more similar
> > to generic one, it also uses is_vmalloc_addr to validate pointer.
> 
> So this lacks a Fixes tag and some deep analysis of similar potential
> problems. While at it please use func() notation for functions. In the
> middle of a sentence iounmap does not immediately stand out, but
> iounmap() does. It's documented ...
> 
> This add_pages() hackery in pagemap_range() is really nasty as it ends
> up violating historical assumptions about max_pfn and high_memory.
> 
> Dan, who did the analysis of this when the device private memory muck
> was added?

So that plain add_pages() usage originated here:

    4ef589dc9b10 mm/hmm/devmem: device memory hotplug using ZONE_DEVICE

The original memremap_pages() only ever used arch_add_memory()

    41e94a851304 add devm_memremap_pages

When HMM merged into memremap_pages() I indeed did not pick up on the
nuance that HMM was wrong to use add_pages() instead of
arch_add_memory() which updates the high_memory variable:

    69324b8f4833 mm, devm_memremap_pages: add MEMORY_DEVICE_PRIVATE support