From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (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 741D6824BD for ; Sun, 23 Nov 2025 03:49:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763869747; cv=none; b=PowFlSEPAIQAUStjPpflPcsvoHI/gw9AzHB07rt8PtA+cjbiDzLhwi20Lp6zH/u//yQKg6Y1t6u/g0frkfSVuVf51v87k38CdwFwwzAm5fDk0xpIE/SvYeMPhw6+dgupWXNyHZRioy18ej5OjkpTMETwav5PFPgu968mJTMuHCw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763869747; c=relaxed/simple; bh=RODesnIVjSlWdC1aaE+qFy8sdzkBg/yoX1Ct3Ksfl3M=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uULQKrEO0JmXXOpypo5L7QRTqlulQG1r0cqREhi4XYX3xk2fGap4sDqXJG+FzaS36d5kzQnMaThcXmOe1dQMSIwFay8hMfoADadUp0vLpgSYahsNpHBONXj45V+yyajOVSifEoYonQF0Buqg5AYKIDt5rxftZUHGiy571B9sYbA= 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=mjJv0t4m; arc=none smtp.client-ip=209.85.128.43 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="mjJv0t4m" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-4777771ed1aso21032755e9.2 for ; Sat, 22 Nov 2025 19:49:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763869743; x=1764474543; darn=vger.kernel.org; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:reply-to:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ZXsBhl22c1IQvTlRBqL+lM/+0Tdw6EnkyTO43xiSs+M=; b=mjJv0t4mVa0XIoSSMofSCO2NCFzqcWWek9Gys4tDW0mRMFEzZWbsQNcbSQ+06M8UE7 ptJUcc2t6XRoyrGNZQnSHhUgVPxGrK44pa59/ryCecIG2d1MxvJ9gFS7Sg2ecx/pR3qO ztbw7gWj5Z/koxkh6G2kYTISlsZ9hrXWWy4FXxNxylQZ3pVe/uMUK48HY4PF/9AHn5+W vUkynDH5S946s6WrYuqOTTOIWHGT9xMiZRRc06Q6LOnZVtJOfVukcpWEfNlW3F6eJyy6 nKeKwH/K60vDn9jH9URod8pwvf1hRuOhCeYMi0QkPJhgtfeQXS+JozUB/AdcqpRiGtej fcWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763869743; x=1764474543; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:reply-to:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ZXsBhl22c1IQvTlRBqL+lM/+0Tdw6EnkyTO43xiSs+M=; b=U5cDVHh2Vw9o+T4geZcOEpnenC0SuuO508WKgv5ce6SFhiZ69r636IudY9PQ/k7wZS +twvFkaToGFVjZROSJMYDSYxyx+a3E5DUQ7EsSTekUYnXL/DnXbpeKZM0NcKDnv2klCD jR10Sy9mfhLQ/DUUFy7CznTsduNmMQnuMP3L18SaIoBtJ6tHWHYsAUVK1bfp0xvSvC9N jP2BwKi5IN48UbWw4fMJBUN9WEFfXXV2gOd0vr+NyO3JJxCDgBpjVZJKU0FiF36OgTpj cRJkopZUsuz4bpiUojI9p0Z2iiXAG1s5bKyt117pwd5orAibGtevAb88UChepzSmkZXv bkvA== X-Gm-Message-State: AOJu0YxDFJdiLEmjATe9wDdXqQzcCLEMCbzj2w0fgexjCL5GBHv0EbUL 4N7gFrDCFU5CzjM4XFWO0AZizWQZwQIWGUUJMQorX30yYt9cxPl1iGug X-Gm-Gg: ASbGncvPLK23b9ZQwxkAto5+nBG43M99mSG77cEJmH3aEsKFCF+izumOtOXsuEA97xh bTFDCSrMlcfY+Ts1+CIWacERamynzbemZDLZ2x+RSGTe73C+nk992VcbXHpD3M5DvV7dOXcwuxn iZFTPe7n/fnyntuy1npLEpBGKqlpKxFELmA6yb/H0c7s7x5Q2mNTagzqcfXEM/9WvCI925P22NP xKs4r+02xrU6osjuJJetGP+P4XSvkfFatoWNrxHvhou5HO7CNDwhke/IHb4kSvZkMcg2TP2ZteS q08J4mHa6MdE3zR5ncV6s8ClfQVL5Rkgjnycet1YiCWiJQL/eCAXCr2nDzZsgnBM0c33O28njhH SfQXyo/uIa75CzG3MuUJ2yoiDGM9kwbCCEjOHPw60MsGjYPLBp2pxEtkY/rCpFF7jOiq+cxAdnh oHhgxYNvNKIXk9QQ== X-Google-Smtp-Source: AGHT+IEbvlAKnsPBP96si9Xg/bJce/BPX2dyx8MBR559zWC8AHAjifETllRpS9g3zzpJlP7SUwQaJg== X-Received: by 2002:a05:6000:22c9:b0:42b:2eb3:c910 with SMTP id ffacd0b85a97d-42cc1cd9466mr7001715f8f.3.1763869742340; Sat, 22 Nov 2025 19:49:02 -0800 (PST) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42cb7fa35c2sm19232083f8f.25.2025.11.22.19.49.01 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Sat, 22 Nov 2025 19:49:01 -0800 (PST) Date: Sun, 23 Nov 2025 03:49:01 +0000 From: Wei Yang To: Balbir Singh Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, dri-devel@lists.freedesktop.org, Andrew Morton , David Hildenbrand , Zi Yan , Joshua Hahn , Rakie Kim , Byungchul Park , Gregory Price , Ying Huang , Alistair Popple , Oscar Salvador , Lorenzo Stoakes , Baolin Wang , "Liam R. Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lyude Paul , Danilo Krummrich , David Airlie , Simona Vetter , Ralph Campbell , Mika =?iso-8859-1?Q?Penttil=E4?= , Matthew Brost , Francois Dugast Subject: Re: [PATCH v2] fixup: mm/huge_memory.c: introduce folio_split_unmapped Message-ID: <20251123034901.nqza7nlg57ivobzu@master> Reply-To: Wei Yang References: <20251120134232.3588203-1-balbirs@nvidia.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=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20251120134232.3588203-1-balbirs@nvidia.com> User-Agent: NeoMutt/20170113 (1.7.2) On Fri, Nov 21, 2025 at 12:42:32AM +1100, Balbir Singh wrote: >Code refactoring of __folio_split() via helper >__folio_freeze_and_split_unmapped() caused a regression with clang-20 >with CONFIG_SHMEM=n, the compiler was not able to optimize away the >call to shmem_uncharge() due to changes in nr_shmem_dropped. >Fix this by adding a stub function for shmem_uncharge when >CONFIG_SHMEM is not defined. > >smatch also complained about parameter end being used without >initialization, which is a false positive, but keep the tool happy >by sending in initialized parameters. end is initialized to 0. >smatch still complains about mapping being NULL and nr_shmem_dropped >may not be 0, but that is not true prior to or after the changes. > >Add detailed documentation comments for folio_split_unmapped() > >Cc: Andrew Morton >Cc: David Hildenbrand >Cc: Zi Yan >Cc: Joshua Hahn >Cc: Rakie Kim >Cc: Byungchul Park >Cc: Gregory Price >Cc: Ying Huang >Cc: Alistair Popple >Cc: Oscar Salvador >Cc: Lorenzo Stoakes >Cc: Baolin Wang >Cc: "Liam R. Howlett" >Cc: Nico Pache >Cc: Ryan Roberts >Cc: Dev Jain >Cc: Barry Song >Cc: Lyude Paul >Cc: Danilo Krummrich >Cc: David Airlie >Cc: Simona Vetter >Cc: Ralph Campbell >Cc: Mika Penttilä >Cc: Matthew Brost >Cc: Francois Dugast > >Suggested-by: David Hildenbrand >Signed-off-by: Balbir Singh >--- >This fixup should be squashed into the patch "mm/huge_memory.c: >introduce folio_split_unmapped" in mm/mm-unstable > > include/linux/shmem_fs.h | 6 +++++- > mm/huge_memory.c | 30 +++++++++++++++++++++--------- > 2 files changed, 26 insertions(+), 10 deletions(-) > >diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h >index 5b368f9549d6..7a412dd6eb4f 100644 >--- a/include/linux/shmem_fs.h >+++ b/include/linux/shmem_fs.h >@@ -136,11 +136,16 @@ static inline bool shmem_hpage_pmd_enabled(void) > > #ifdef CONFIG_SHMEM > extern unsigned long shmem_swap_usage(struct vm_area_struct *vma); >+extern void shmem_uncharge(struct inode *inode, long pages); > #else > static inline unsigned long shmem_swap_usage(struct vm_area_struct *vma) > { > return 0; > } >+ >+static void shmem_uncharge(struct inode *inode, long pages) >+{ >+} > #endif > extern unsigned long shmem_partial_swap_usage(struct address_space *mapping, > pgoff_t start, pgoff_t end); >@@ -194,7 +199,6 @@ static inline pgoff_t shmem_fallocend(struct inode *inode, pgoff_t eof) > } > > extern bool shmem_charge(struct inode *inode, long pages); >-extern void shmem_uncharge(struct inode *inode, long pages); > > #ifdef CONFIG_USERFAULTFD > #ifdef CONFIG_SHMEM >diff --git a/mm/huge_memory.c b/mm/huge_memory.c >index 78a31a476ad3..18c12876f5e8 100644 >--- a/mm/huge_memory.c >+++ b/mm/huge_memory.c >@@ -3751,6 +3751,7 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n > int ret = 0; > struct deferred_split *ds_queue; > >+ VM_WARN_ON_ONCE(!mapping && end); > /* Prevent deferred_split_scan() touching ->_refcount */ > ds_queue = folio_split_queue_lock(folio); > if (folio_ref_freeze(folio, 1 + extra_pins)) { >@@ -3919,7 +3920,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order, > int nr_shmem_dropped = 0; > int remap_flags = 0; > int extra_pins, ret; >- pgoff_t end; >+ pgoff_t end = 0; > bool is_hzp; > > VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio); >@@ -4092,16 +4093,27 @@ static int __folio_split(struct folio *folio, unsigned int new_order, > return ret; > } > >-/* >- * This function is a helper for splitting folios that have already been unmapped. >- * The use case is that the device or the CPU can refuse to migrate THP pages in >- * the middle of migration, due to allocation issues on either side >+/** >+ * folio_split_unmapped() - split a large anon folio that is already unmapped >+ * @folio: folio to split >+ * @new_order: the order of folios after split >+ * >+ * This function is a helper for splitting folios that have already been >+ * unmapped. The use case is that the device or the CPU can refuse to migrate >+ * THP pages in the middle of migration, due to allocation issues on either >+ * side. >+ * >+ * anon_vma_lock is not required to be held, mmap_read_lock() or >+ * mmap_write_lock() should be held. @folio is expected to be locked by the Took a look into its caller: __migrate_device_pages() migrate_vma_split_unmapped_folio() folio_split_unmapped() I don't see where get the folio lock. Would you mind giving me a hint where we toke the lock? Seems I missed that. >+ * caller. device-private and non device-private folios are supported along >+ * with folios that are in the swapcache. @folio should also be unmapped and >+ * isolated from LRU (if applicable) > * >- * The high level code is copied from __folio_split, since the pages are anonymous >- * and are already isolated from the LRU, the code has been simplified to not >- * burden __folio_split with unmapped sprinkled into the code. >+ * Upon return, the folio is not remapped, split folios are not added to LRU, >+ * free_folio_and_swap_cache() is not called, and new folios remain locked. > * >- * None of the split folios are unlocked >+ * Return: 0 on success, -EAGAIN if the folio cannot be split (e.g., due to >+ * insufficient reference count or extra pins). > */ > int folio_split_unmapped(struct folio *folio, unsigned int new_order) > { >-- >2.51.1 > -- Wei Yang Help you, Help me