Re: [PATCH v6 0/6] mm/vmalloc: Speed up ioremap, vmalloc and vmap with contiguous memory

[Dev Jain <dev.jain@arm.com>]

On 13/07/26 11:00 pm, Uladzislau Rezki wrote:
> On Mon, Jul 13, 2026 at 08:43:14PM +0530, Dev Jain wrote:
>>
>>
>> On 10/07/26 2:24 pm, Wen Jiang wrote:
>>> On Fri, 10 Jul 2026 at 07:08, Andrew Morton <akpm@linux-foundation.org> wrote:
>>>>
>>>> On Thu,  9 Jul 2026 15:38:17 +0800 Wen Jiang <jiangwenxiaomi@gmail.com> wrote:
>>>>
>>>>> This patchset accelerates ioremap, vmalloc, and vmap when the memory
>>>>> is physically fully or partially contiguous.
>>>>
>>>> Thanks, I added this to mm.git's mm-new branch for wider testing.
>>>>
>>>> AI review asked some questions, and some of them are new since the v5
>>>> series:
>>>>         https://sashiko.dev/#/patchset/20260709073823.6643-1-jiangwen6@xiaomi.com
>>>
>>> Hi Andrew,
>>>
>>> I've gone through the Sashiko findings:
>>>
>>> - Patch 1 (find_num_contig): Over-interpretation. No new hugetlbfs hstate
>>>   is added. The extra sizes are only used by init_mm kernel mappings via.
>>>
>>> - Patch 5/6 (NULL page): Invalid input. vmap() expects a fully populated
>>>     array of valid struct page pointers.
>>
>> Correct, but vmap_pages_pte_range has !page and !pfn_valid checks.
>>
>> I really hate those checks - if those checks have any remote possibility of
>> firing, then we already have a bug at
>>
>> vm_map_ram -> vmap_pages_range -> vmap_pages_range_noflush -> kmsan_vmap_pages_range_noflush
>>
>> because the last function dereferences the struct page pointers.
>>
>> It is painful to do the page array sanity check deep into vmap - it implies
>> we simply cannot play with the page array before that.
>>
>> But since vmap is an exported function, doing a sanity check for the page array
>> in the vmap code makes sense.
>>
>> So how about the following:
>>
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index afaa14ebf17bb..0c44bb7a45b5d 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -566,14 +566,6 @@ static int vmap_pages_pte_range(pmd_t *pmd, unsigned long addr,
>>  			err = -EBUSY;
>>  			break;
>>  		}
>> -		if (WARN_ON(!page)) {
>> -			err = -ENOMEM;
>> -			break;
>> -		}
>> -		if (WARN_ON(!pfn_valid(page_to_pfn(page)))) {
>> -			err = -EINVAL;
>> -			break;
>> -		}
>>
>>  		pfn = page_to_pfn(page);
>>  		size = vmap_set_ptes(pte, addr, end, pfn, prot, shift);
>> @@ -603,11 +595,6 @@ static int vmap_pages_pmd_range(pud_t *pud, unsigned long addr,
>>  			struct page *page = pages[*nr];
>>  			phys_addr_t phys_addr;
>>
>> -			if (WARN_ON(!page))
>> -				return -ENOMEM;
>> -			if (WARN_ON(!pfn_valid(page_to_pfn(page))))
>> -				return -EINVAL;
>> -
>>  			phys_addr = page_to_phys(page);
>>
>>  			if (vmap_try_huge_pmd(pmd, addr, next, phys_addr, prot,
>> @@ -3663,6 +3650,19 @@ static struct vm_struct *vmap_get_aligned_vm_area(unsigned long size,
>>  	return __get_vm_area_node_aligned_caller(size, PAGE_SIZE, flags, caller);
>>  }
>>
>> +static inline bool vmap_page_sanity_checks(struct page **pages, unsigned int count)
>> +{
>> +	for (int i = 0; i < count; ++i) {
>> +		if (WARN_ON(!pages[i]))
>> +			return true;
>> +
>> +		if (WARN_ON(!pfn_valid(page_to_pfn(pages[i]))))
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>  /**
>>   * vmap - map an array of pages into virtually contiguous space
>>   * @pages: array of page pointers
>> @@ -3706,6 +3706,9 @@ void *vmap(struct page **pages, unsigned int count,
>>  	if (!area)
>>  		return NULL;
>>
>> +	if (unlikely(vmap_page_sanity_checks(pages, count)))
>> +		return NULL;
>> +
>>  	addr = (unsigned long)area->addr;
>>  	if (vmap_pages_range_batched(addr, addr + size, pgprot_nx(prot),
>>  				pages) < 0) {
>>
>>
>>
>> Reasoning for calling vmap_page_sanity_checks() before vmap_pages_range_batched,
>> and not at the start of vmap: I am worried that since vmap() is already very
>> fast, we may cause a regression:
>>
>> vmap() -> scan page array with linear map pointers -> vmap_get_aligned_vm_area (does
>> memory allocation, throwing out the linear map VAs from cache and TLB) -> walk
>> the pgtables and again access cold page array.
>>
>> Perhaps I am being very pedantic here. What do you think?
>>
> Sanity check adds extra CPU cycles and it adds overhead. The concern about cache
> to be cold on second iteration looks valid. You can get some perf figures to see
> the cost.
> 
> I would just keep the original approach. But no strong opinion here.

Given you don't have a strong opinion here, I would also prefer keeping the original
approach. This feels more like a code structure problem than a correctness problem,
*and* given that with kmsan builds we have this problem for years now in the kernel.

So perhaps we can look at how to solve this later.


> 
> --
> Uladzislau Rezki