From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-182.mta0.migadu.com (out-182.mta0.migadu.com [91.218.175.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 356343B05A1 for ; Wed, 29 Jul 2026 15:12:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785337976; cv=none; b=Vjo7vA9aKDnf0u7lCJ4AgzyjK0+97L2fEVPX7rKblhGLY7WsHBADFHmXRQsiVOofd/nr98v92+Nnk2qZawpXSondf8oTvAvqBNXRdLlpeCec5KPtW3WI5UAc8eEkrcnmthEjTeCJfPnydnTTKWa3NFDzcmwwInX+BhdvDdlUdQk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785337976; c=relaxed/simple; bh=Iu0P9FLA5KUqzCdmLusUapNuruRJd3xFXBp4QnNJJGk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Orf8GHGiRRck2W98e+PzHDfHLzhYxaOClfhjlCa/JAStjSSPSbjKhXFpHQxXvoHI8WDztgxW+d6qtnBdimHdQMxJAnx+bUmEplIlLlzjKqkIalyShIpIsTTZBhGnszEABSnV6918lciVkQR9+JYy+0biu/dUsKkQd95wJoihjJQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=lZHUDOqc; arc=none smtp.client-ip=91.218.175.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="lZHUDOqc" Date: Wed, 29 Jul 2026 23:12:43 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1785337971; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WGWZOVo9FQi4yCCJXT5thQck79C8dzo74UOUAl3PZa4=; b=lZHUDOqc9Js4vijztT02oY7lA4Da4gXntN16HH1UGEdUkL+YUTr0NztxsFcLQLrC2TsrwX EpC14ybVknneWacuyDF6bcdYECe1NjtdLZFKnivZTMKPUtDD9gY2C+1zrFuk4SjeWjmh4E eAqF8LGfbeOHh/VKOjcSXneWngpcg0U= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Baoquan He To: Nhat Pham Cc: linux-mm@kvack.org, akpm@linux-foundation.org, chrisl@kernel.org, kasong@tencent.com, baohua@kernel.org, youngjun.park@lge.com, hannes@cmpxchg.org, yosry@kernel.org, david@kernel.org, shikemeng@huaweicloud.com, chengming.zhou@linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 03/11] mm, swap: add xswap cluster grow via VM_SPARSE vmalloc Message-ID: References: <20260727135029.1059441-1-baoquan.he@linux.dev> <20260727140503.1060918-1-baoquan.he@linux.dev> <20260727140503.1060918-2-baoquan.he@linux.dev> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Migadu-Flow: FLOW_OUT On 07/27/26 at 08:05am, Nhat Pham wrote: > On Mon, Jul 27, 2026 at 7:05 AM Baoquan He wrote: > > > > Implement dynamic cluster_info array growth for xswap devices using a > > VM_SPARSE vmalloc area: > > > > 1. xswap_map_clusters(): Allocate physical pages and map them into > > the pre-reserved VM_SPARSE KVA region via vm_area_map_pages(). > > > > 2. xswap_unmap_clusters(): Unmap pages from the VM_SPARSE area via > > vm_area_unmap_pages() (used by the error/teardown paths, shrink > > comes later). > > > > 3. setup_swap_clusters_info() xswap path: Use get_vm_area(VM_SPARSE) > > for the cluster_info array, lazily mapping only the initial chunk. > > > > 4. free_swap_cluster_info(): Refactor to take swap_info_struct*. > > For xswap, unmap all clusters and free_vm_area(). > > > > 5. swapoff: Remove snapshot locals; move p->max/p->cluster_info > > clearing after free_swap_cluster_info(). > > > > Signed-off-by: Baoquan He > > I'll give my 2 cents on the direction we're pursuing later. I've done > a lot of thinking over the past weeks, and I have some new concerns > now regarding how we plan to land this joint venture :) > > That said, this is a new piece of extension, done per my request > (kernel-driven dynamic swap address space growth - thanks for taking > my concerns seriously), and I've also evaluated this vmalloc-based > data structure in the past week, so I'll put in my inquiries/concerns. > > > --- > > mm/swapfile.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 216 insertions(+), 12 deletions(-) > > > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > index 143088dae07d..6d9c95ed09bd 100644 > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -49,6 +49,24 @@ > > #include "internal.h" > > #include "swap.h" > > > > +#ifdef CONFIG_XSWAP > > +/* > > + * xswap: dynamically grow the cluster_info array via a VM_SPARSE area. > > + * > > + * XSWAP_GROW_CLUSTERS is the number of clusters to map in one grow > > + * operation. It is set to the number of cluster_info structs that > > + * fit in a single page (at least 16), so that the vmalloc page table > > + * overhead is proportional to the number of clusters mapped. > > + */ > > +#define XSWAP_GROW_CLUSTERS \ > > + max_t(unsigned long, PAGE_SIZE / sizeof(struct swap_cluster_info), 16) > > + > > +static int xswap_map_clusters(struct swap_info_struct *si, > > + unsigned long start_idx, unsigned long nr); > > +static void xswap_unmap_clusters(struct swap_info_struct *si, > > + unsigned long start_idx, unsigned long nr); > > +#endif > > + > > static void swap_range_alloc(struct swap_info_struct *si, > > unsigned int nr_entries); > > static bool folio_swapcache_freeable(struct folio *folio); > > @@ -3041,20 +3059,47 @@ static void wait_for_allocation(struct swap_info_struct *si) > > > > BUG_ON(si->flags & SWP_WRITEOK); > > > > +#ifdef CONFIG_XSWAP > > + /* > > + * xswap clusters beyond nr_clusters_mapped have been unmapped > > + * by the shrinker and their vmalloc pages are no longer > > + * accessible. Only iterate over currently mapped clusters. > > + */ > > + if (si->flags & SWP_XSWAP) > > + end = min(end, READ_ONCE(si->nr_clusters_mapped) * > > + SWAPFILE_CLUSTER); > > +#endif > > + > > for (offset = 0; offset < end; offset += SWAPFILE_CLUSTER) { > > ci = swap_cluster_lock(si, offset); > > swap_cluster_unlock(ci); > > } > > } > > > > -static void free_swap_cluster_info(struct swap_cluster_info *cluster_info, > > - unsigned long maxpages) > > +static void free_swap_cluster_info(struct swap_info_struct *si) > > { > > + struct swap_cluster_info *cluster_info = si->cluster_info; > > + unsigned long maxpages = si->max; > > struct swap_cluster_info *ci; > > - int i, nr_clusters = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER); > > + int i, nr_clusters; > > > > if (!cluster_info) > > return; > > + > > +#ifdef CONFIG_XSWAP > > + if (si->flags & SWP_XSWAP) { > > + /* Unmap all mapped clusters and free the VM_SPARSE area */ > > + if (si->nr_clusters_mapped > 0) > > + xswap_unmap_clusters(si, 0, si->nr_clusters_mapped); > > + free_vm_area(si->cluster_vm); > > + si->cluster_vm = NULL; > > + si->nr_clusters = 0; > > + si->nr_clusters_mapped = 0; > > + return; > > + } > > +#endif > > + > > + nr_clusters = DIV_ROUND_UP(maxpages, SWAPFILE_CLUSTER); > > for (i = 0; i < nr_clusters; i++) { > > ci = cluster_info + i; > > /* Cluster with bad marks count will have a remaining table */ > > @@ -3093,11 +3138,9 @@ static void flush_percpu_swap_cluster(struct swap_info_struct *si) > > SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) > > { > > struct swap_info_struct *p = NULL; > > - struct swap_cluster_info *cluster_info; > > struct file *swap_file, *victim; > > struct address_space *mapping; > > struct inode *inode; > > - unsigned int maxpages; > > int err, found = 0; > > > > if (!capable(CAP_SYS_ADMIN)) > > @@ -3189,10 +3232,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) > > > > swap_file = p->swap_file; > > p->swap_file = NULL; > > - maxpages = p->max; > > - cluster_info = p->cluster_info; > > - p->max = 0; > > - p->cluster_info = NULL; > > spin_unlock(&p->lock); > > spin_unlock(&swap_lock); > > arch_swap_invalidate_area(p->type); > > @@ -3200,7 +3239,9 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) > > mutex_unlock(&swapon_mutex); > > kfree(p->global_cluster); > > p->global_cluster = NULL; > > - free_swap_cluster_info(cluster_info, maxpages); > > + free_swap_cluster_info(p); > > + p->max = 0; > > + p->cluster_info = NULL; > > > > inode = mapping->host; > > > > @@ -3564,6 +3605,112 @@ static unsigned long read_swap_header(struct swap_info_struct *si, > > return maxpages; > > } > > > > +#ifdef CONFIG_XSWAP > > +static int xswap_map_clusters(struct swap_info_struct *si, > > + unsigned long start_idx, unsigned long nr) > > +{ > > + unsigned long start_addr = (unsigned long)si->cluster_info + > > + (size_t)start_idx * sizeof(struct swap_cluster_info); > > + unsigned long end_addr = start_addr + (size_t)nr * sizeof(struct swap_cluster_info); > > + /* > > + * vm_area_map_pages() requires that start and end be page-aligned. > > + * If start_addr falls within a page that was already mapped by a > > + * previous batch (grow path), round it up to skip the already-mapped > > + * partial page. Always round end_addr up so the vmap page table walk > > + * terminates correctly (the walk loop exits when addr == end, and addr > > + * advances by PAGE_SIZE each iteration). > > + */ > > + unsigned long vm_start = PAGE_ALIGN(start_addr); > > + unsigned long vm_end = PAGE_ALIGN(end_addr); > > + unsigned long npages; > > + struct page **pages; > > + unsigned long i; > > + > > + if (vm_start >= vm_end) { > > + /* All requested clusters fall within already-mapped pages. */ > > + for (i = start_idx; i < start_idx + nr; i++) > > + spin_lock_init(&si->cluster_info[i].lock); > > + WRITE_ONCE(si->nr_clusters_mapped, start_idx + nr); > > + return 0; > > + } > > + > > + npages = (vm_end - vm_start) >> PAGE_SHIFT; > > + > > + pages = kmalloc_array(npages, sizeof(*pages), GFP_KERNEL); > > + if (!pages) > > + return -ENOMEM; > > + > > + for (i = 0; i < npages; i++) { > > + /* > > + * __GFP_ZERO is critical: cluster_info structs contain pointer > > + * fields (extend_table, zero_bitmap, memcg_table, table) that > > + * must start as NULL. Without zeroing, stale data from a > > + * previous user of the page would look like valid pointers. > > + */ > > + pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO); > > + if (!pages[i]) > > + goto fail; > > + } > > + > > + if (vm_area_map_pages(si->cluster_vm, vm_start, vm_end, pages)) { > > + i = npages; /* free all pages on failure */ > > + goto fail; > > I was evaluating whether your vmalloc array can be extended to support > kernel-driven dynamic growth at least (either as a slot-in replacement > for xarray, or as a follow-up optimization if it's too complicated), > and I stumble this mapping action. > > Seems like it does not take any GFP flag argument, and under the hood > it calls GFP_KERNEL. Would this be safe in the swap allocation path > (where you slotted it in in patch 4)? Kairui used a more precise set > of flags in this path, for e.g for swap table allocation: > > ret = swap_cluster_alloc_table(ci, __GFP_HIGH | __GFP_NOMEMALLOC | > GFP_KERNEL); > > > I'm guessing this has to do with the fact that we often entered > swapping out paths with PF_MEMALLOC... Is there any risk of deadlock > etc.? Thanks for catching this. You're right — there are actually three layers of hardcoded GFP_KERNEL in the VM_SPARSE path: 1. alloc_page(GFP_KERNEL | __GFP_ZERO) in xswap_map_clusters() for the backing pages 2. vmap_pages_range() hardcodes GFP_KERNEL (passed only to KMSAN, but still semantically wrong) 3. The page-table allocations inside vmap_pages_range_noflush_walk() (pte_alloc_kernel_track etc.) use GFP_PGTABLE_KERNEL, which is GFP_KERNEL | __GFP_ZERO — also lacking __GFP_NOMEMALLOC, and completely invisible to the caller When PF_MEMALLOC is set (as it is during reclaim), __gfp_pfmemalloc_flags() returns ALLOC_NO_WATERMARKS for any allocation without __GFP_NOMEMALLOC, meaning those allocations can consume all emergency reserves — including the reserves that the reclaim machinery itself needs to make forward progress. That is a real deadlock risk. One way is to fix them one by one. Another way is to drop the VM_SPARSE approach entirely and implemented Chris's suggestion instead: cluster_info is now a struct swap_cluster_info ** pointer array, with each cluster individually kmem_cache_zalloc()'d. The grow path (xswap_grow_clusters) now uses: kmem_cache_zalloc(swap_cluster_cachep, __GFP_HIGH | __GFP_NOMEMALLOC | GFP_KERNEL) matching the pattern Kairui established in swap_cluster_alloc_table(). All allocations are fully GFP-controllable — no hidden vmap page-table allocations, no hardcoded GFP_PGTABLE_KERNEL. With kmem_cache, shrink is just kmem_cache_free() — each cluster is independently allocated and freed. Seems the logic is simpler. If no objection, I will give it a shot soon. Thanks Baoquan