From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 68FE519D8A8 for ; Mon, 13 Jul 2026 17:30:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783963847; cv=none; b=kqOPqfJkNadkxl6e6BEmnrdGBgvw3VxeStTavEU7s4iSAdIiBaZ2gDq8wELBJW1gdsj+yHs1Vo+ECPnz4cxAQw2j+g+Ovns9guln1FpDwL+Jk32I0P4VBFAorlU8ltyyZaqgXS3tvab/JyReXaW6/plXPALBxE4yUoGuQoqeVg0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783963847; c=relaxed/simple; bh=6/5psBgONqqKs0AfwGXzrYyskoZ8vUsJLnC2jecdjBg=; h=From:Date:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Jm3t8Yv52FD/MfZXV4mv8S0D+A5l3NAhA2hXRT8UU+Pa/y2RewgaHWzwKSoylqwl6VpAr88j8jA7CZX8tG7xwurs3T2U2EE176z3KwXKE7tyZ5ba1wXi6MTLBvIZWxLNLaABwJiSZGP0VCsGHf9TDvRu07TAC/shwtNBV9ftaog= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=U2rDW2EJ; arc=none smtp.client-ip=209.85.167.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="U2rDW2EJ" Received: by mail-lf1-f46.google.com with SMTP id 2adb3069b0e04-5b01146b205so2507696e87.2 for ; Mon, 13 Jul 2026 10:30:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783963843; x=1784568643; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:date:from:from:to:cc:subject :date:message-id:reply-to:content-type; bh=51jkDGqcYSlWtUq0/oF4m7PdKEgEv1ezQg2syRvYV9I=; b=U2rDW2EJCaK6oKRYZNQh03rwCUzb/tJIXSbeiJejNJXF3+2qksbphcdK86+wZao8gn gwsrG/zNIljrBCkGvfOe1NgCTOPJs6+r5SyWYjXb6FrItgo5r5dsjkLq7pSQxExU91V5 NxsYNKzLyQlL6eBd7VhEc6WYVcJtNh/EIu+ERPEbkvFU+Luyr4At8h84L6xwNFY2VFLw hYjRsnM47rAoWR9n0Ya76TZdYXNq25vzxbNiQdlZs4iTKROya7yKZYN7Jh3/DwCsmm5V 8D9RpGFrughB0erR14hwJtZ4D4Q+DiUzzB356Udowurww7BE19xHIxWqYu66tpKbmKJH zs+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783963843; x=1784568643; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:date:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=51jkDGqcYSlWtUq0/oF4m7PdKEgEv1ezQg2syRvYV9I=; b=kVQkKZYxDmI/ZkH7ZlMuBiHwYnY9P0nGJhO8nimkrfefMsEFhFq1zix2wq5EWfCPcu ta8kFgBuzXUyuPpQp9KhzhUuOVcIzgbuTPV8h2qSy7Gl1Mxm5BPM+Qh+NbpeLjedC6/w UxIOQk9DL8PLhkEJ1f9oVAFxtLl8V0rO7MY/lIS6W9loeaRzHgxQcwp5NIO4DErWGaZ9 XGQ0OlmSGS0YjK1B8knrf7YuD8ffuQi36Vq79hBRz6UqgHk4yba6eG3VabaCYdvAj2Qp 1OScXKyrmq+27tv2Si85M59Lzg3DLdFu0sZdQA5+YoS5FhuOnLm3J1/45rEXjG2ASBnQ xmDA== X-Forwarded-Encrypted: i=1; AHgh+RqenOP+WCpd5Ge50qhhYVEoMibV8xLMB6eye9WzaotCPLnMdzrKwWGWOPnOCi/oK3oPqjMVW44ieKHzT7s=@vger.kernel.org X-Gm-Message-State: AOJu0YwnYHwlAzw6aTVz3ZPZ0CJLPKZP2MIwNM8fGk86SploeCQ0Yf0w NXjPH/1xwL8wuOEQiQdHl5mqT8fmrPD7RxPPvU7qHosu5Dc2Y5Wp69T1 X-Gm-Gg: AfdE7cnUOSWOXxgMoyZjUDKdxcuea6cYa5Zl89ODh+OcClIUppLrD760O9zsgC6mJ/Z PHpnSt4Xj3OCrHEVXRxNXPxVt7ZDWPXsV5CbTWdv+GCT2q+xWBb2GoZAzJ/wKc90LOR9rP/StDY Ju4Ytf1CYbdzrpggy3mh3O0wG1xwCGdjf1NM+Ekk7oVOaW9j52JgBTNdbMGTovHv4354AE0UpKC LqXXvzYqU1az0J8nXm25TpXYJrtMmFNJdCOBAPgP/Z8m1zIC5Gxft1U5r5EZYstqxnACa9KZ2TJ Ouy5+LV4Tq/2ibi77N0Alznj9C05CNmNPCoDqSU/292Cm6c3vuNMOAn7znL0bOfI1j9/iIVS89T ScE9CYgOkmvpEeJNxqnjufDNZ44UiNgiGDHFWX7Ot2CEJ8HXkj0LnIwZ+HsEHQ5bP X-Received: by 2002:a05:6512:3353:b0:5b0:ceb2:1974 with SMTP id 2adb3069b0e04-5b0ceb21b1dmr2073653e87.14.1783963843127; Mon, 13 Jul 2026 10:30:43 -0700 (PDT) Received: from milan ([2001:9b1:d5a0:a500::24b]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5b01ca5559csm2984970e87.36.2026.07.13.10.30.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jul 2026 10:30:42 -0700 (PDT) From: Uladzislau Rezki X-Google-Original-From: Uladzislau Rezki Date: Mon, 13 Jul 2026 19:30:40 +0200 To: Dev Jain Cc: Wen Jiang , Andrew Morton , catalin.marinas@arm.com, linux-mm@kvack.org, urezki@gmail.com, will@kernel.org, Xueyuan.chen21@gmail.com, ajd@linux.ibm.com, anshuman.khandual@arm.com, david@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, rppt@kernel.org, ryan.roberts@arm.com, Wen Jiang Subject: Re: [PATCH v6 0/6] mm/vmalloc: Speed up ioremap, vmalloc and vmap with contiguous memory Message-ID: References: <20260709073823.6643-1-jiangwen6@xiaomi.com> <20260709160805.26e63bae89dd03cf2951104e@linux-foundation.org> <54434a84-51b3-4e3f-988b-6bcbac5a2f42@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54434a84-51b3-4e3f-988b-6bcbac5a2f42@arm.com> 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 wrote: > >> > >> On Thu, 9 Jul 2026 15:38:17 +0800 Wen Jiang 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. -- Uladzislau Rezki