From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-178.mta0.migadu.com (out-178.mta0.migadu.com [91.218.175.178]) (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 686E03BED70 for ; Fri, 22 May 2026 09:35:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779442560; cv=none; b=uvwd5wm/9lBNkBhByHmpaRdaEO+QfrnxKJhs0pReryE2UHF/W9eYgKfFiqHuMmpKhgCwc8P6W5YCYT7RbF5HWxXqeuPI63wtXq5mxIoz6XrN+MO3IxwCbOUuJq6Ww7r1DVg1CoM5bfP/0LA1b3w5Re77X+JA1lSPVv6OmeZumwM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779442560; c=relaxed/simple; bh=OndYXV0TFgOqgWln09I635MUUvXI3P2X6633/58SO9A=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=NAXf6TyWLvgGF2pjHdEHz6Rooet/VRlEq44qjXY8ryIapBO9JZta2apiqDFSgC+kvXlmsyQQYxfMHN8FGx8I2/zNxlYynSTDiMl9MeOCW3Tb4R501l75JZgYm2O4BLIqDFjgzlxykVis8iDjlAmVmnB5TS3sz4N7o0clSHM0+2U= 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=nQ1Sn4Wc; arc=none smtp.client-ip=91.218.175.178 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="nQ1Sn4Wc" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1779442555; 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=7fFfkymitLlVPUZI42Fgss83DIkP0MeCyuDpYJPsqwA=; b=nQ1Sn4Wcnu7UqMqH8rketLhQ6FzBpBJVRRPWzPmy6S57Hx7c42xGev43Mo8ER9w3bE36r8 WM+Bcxonkou2FCHZVy3XXOmKBozrucYdsh+fgnq5MitUjXbgS8BP+i/0MSI/Z+FFZAwYWw ncyuWVT7JzdmZ1wO5K+lg7zeotPNewM= Date: Fri, 22 May 2026 17:35:01 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH] mm: page_isolation: Avoid hugepage scan step underflow To: Andrew Morton , david@kernel.org Cc: vbabka@kernel.org, surenb@google.com, mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org, ziy@nvidia.com, liushixin2@huawei.com, osalvador@suse.de, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Kaitao Cheng , Muchun Song References: <20260519121646.40833-1-kaitao.cheng@linux.dev> <20260519105424.e422131a0ffb49e19ed1aebe@linux-foundation.org> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Kaitao Cheng In-Reply-To: <20260519105424.e422131a0ffb49e19ed1aebe@linux-foundation.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 在 2026/5/20 01:54, Andrew Morton 写道: > On Tue, 19 May 2026 20:16:46 +0800 Kaitao Cheng wrote: > >> page_is_unmovable() checks HugeTLB pages without holding hugetlb_lock and >> without pinning the folio. This is intentional for the pageblock scanning >> paths, but it means the HugeTLB folio can be freed concurrently after >> PageHuge() or folio_test_hugetlb() succeeds. > > Thanks. What are the userspace-visible runtime effects of this? The direct user-visible effect is not memory corruption, but degraded forward progress in the page isolation / contiguous allocation path. If the race makes folio_nr_pages() return 1 while the current page is still treated as a tail page of the old HugeTLB folio, the computed step can underflow: step = folio_nr_pages(folio) - folio_page_idx(folio, page); The caller then does: start_pfn += step; With unsigned arithmetic this can wrap and move start_pfn backwards, typically near the beginning of the old hugepage range rather than advancing past it. In many cases this only means rescanning part of the same hugepage range, so the effect may be limited to extra scanning work. However, it still violates the scanner's forward-progress assumption: step is expected to advance start_pfn. If the same transient state is observed repeatedly, the scanner can keep revisiting the same PFNs, causing excessive latency and, in the worst case, an apparent stall in operations that rely on page isolation or contiguous allocation. Here is another point raised by AI, the old code also used folio_test_lru() on a folio pointer obtained without holding a reference. If the folio is freed and the old head page is reused or observed as a tail page of another compound page, folio_test_lru() can reach const_folio_flags(), which asserts that the passed folio is not a tail page. On DEBUG_VM kernels, that can trigger a VM_BUG_ON_PGFLAGS() crash. >> The existing code avoids folio_hstate() and uses size_to_hstate() because >> the HugeTLB flag may already have been cleared. However, if >> size_to_hstate() returns NULL, the code still falls through and computes >> the scan step from folio_nr_pages(). If the folio has been freed and the >> head/large state has been cleared, folio_nr_pages() can return 1. When the >> current page is a tail page, subtracting folio_page_idx() from 1 can >> underflow and make the scanner skip too far. >> >> Treat a NULL hstate as unmovable so the scanner does not try to skip over >> an unstable HugeTLB folio. Once a valid hstate is found, derive the number >> of pages from the hstate instead of reading the folio size again. Also >> validate the page index before computing the step to avoid underflow if the >> page/folio relationship changed concurrently. > > This code sounds rather sketchy, and it sounds like it will remain > sketchy after the patch. And AI review says "hey, this code is > sketchy": > https://sashiko.dev/#/patchset/20260519121646.40833-1-kaitao.cheng@linux.dev Following David Hildenbrand's suggestion, I made some changes as shown below. I'm not sure whether there are still any other issues. --- a/mm/page_isolation.c +++ b/mm/page_isolation.c @@ -41,8 +41,14 @@ bool page_is_unmovable(struct zone *zone, struct page *page, * We need not scan over tail pages because we don't * handle each tail page individually in migration. */ - if (PageHuge(page) || PageCompound(page)) { + if (PageCompound(page)) { struct folio *folio = page_folio(page); + unsigned long nr_pages, pfn; + unsigned int order; + + order = compound_order(&folio->page); + if (order > MAX_FOLIO_ORDER) + return true; if (folio_test_hugetlb(folio)) { struct hstate *h; @@ -54,15 +60,16 @@ bool page_is_unmovable(struct zone *zone, struct page *page, * The huge page may be freed so can not * use folio_hstate() directly. */ - h = size_to_hstate(folio_size(folio)); - if (h && !hugepage_migration_supported(h)) + h = size_to_hstate(PAGE_SIZE << order); + if (!h || !hugepage_migration_supported(h)) return true; - - } else if (!folio_test_lru(folio)) { + } else if (!PageLRU(page)) { return true; } - *step = folio_nr_pages(folio) - folio_page_idx(folio, page); + nr_pages = 1UL << order; + pfn = page_to_pfn(page); + *step = (pfn | (nr_pages - 1)) + 1 - pfn; return false; } -- Thanks Kaitao Cheng