From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (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 6F5C83FFFA8 for ; Fri, 21 Aug 2026 19:48:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787341701; cv=none; b=NUGBwSy2JNlZM0SnlmVoPTvxqUIZkYRYE526fSyPUFP52/m0+zMTy7pSMcdLgsLOnCFpEOKdB8u809E6/AmGjWDgECFnj23fvqa+VQGfBPEJ921YAzn2DGwVHksId8BFO2BRESsqp9/rENjGYq6jeNe1e0PQyIQuBUGiPYQgk8g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787341701; c=relaxed/simple; bh=Lf7b5athM/MNWm9oSBElcINnUxyk1EdJSWasN0HVF0U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=NEI/nEkqfefgzT+YNEs0yID4inw7cljNAHRDfLYkaG/LSXO6iUMv/6H76wERDOOOx6JXZZa7utdj6cK7CSLesyZy8w2yqGg3w/mauP9F4W7Fh9mHi2Yq2hhtMqmmQ21YH6/FPYftIt9oHCgqrvTI/o2IuzVwn+vg3/dI67f7tcY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=pass smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=n036ESkE; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="n036ESkE" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=J2plf99ZSi4RvgrNhakNW0Ew4wlGptknswaCYlfxyes=; b=n036ESkE0ogiUqeh4hgpiPsDTO gbhSAkhvHkoztzLBvWgIvd3arQqI7rOwIRfjGFWUicARO45wot1BLEdrRDIteEDhXLVr3PYzEnpK7 J/9/CVJ5DAF8ump0ILfZcEfNPz3JbRK2tunSdH1+kCgUonPvTPMJe2Aum5KHi+VXAsB7Ws7gC2RrH WgvqzN1A7FuMcg+buq84J9O9QzMCepCnLsxg4/3WoQg1n7Gq/KTWL9uSLpNwpZVx7Z6B9hs7b2x2T GIWwHOteG5H1ZSDpWdZYY3+KsR/hzQjnUb6n4SpBFS1e/iFVHZFG/FoPMspZtkxvYFeNK32383HG8 6OpCf/uQ==; Received: from willy by casper.infradead.org with local (Exim 4.99.1 #2 (Red Hat Linux)) id 1wxVDp-00000000LTE-0lyj; Fri, 21 Aug 2026 19:48:13 +0000 Date: Fri, 21 Aug 2026 20:48:12 +0100 From: Matthew Wilcox To: "Lorenzo Stoakes (ARM)" Cc: Gregory Price , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com, akpm@linux-foundation.org, liam@infradead.org, david@kernel.org, vbabka@kernel.org, jannh@google.com, sashiko-bot Subject: Re: [PATCH] mm/madvise: use folio_trylock() in the cold/pageout PMD split Message-ID: References: <20260821150912.183976-1-gourry@gourry.net> 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: On Fri, Aug 21, 2026 at 07:23:34PM +0100, Lorenzo Stoakes (ARM) wrote: > On Fri, Aug 21, 2026 at 11:09:12AM -0400, Gregory Price wrote: > > MADV_COLD or MADV_PAGEOUT over part of a PMD splits the THP in > > madvise_cold_or_pageout_pte_range(). Two threads doing that to > > the same THP create spurious failures. > > > > CPU0 CPU1 > > ---- ---- > > folio_get() > > spin_unlock(ptl) > > folio_lock() > > folio_get() > > spin_unlock(ptl) > > folio_lock() <- blocks, keeps its ref > > split_folio() > > folio_expected_ref_count(folio) != folio_ref_count(folio) - 1 > > -EAGAIN > > Hmm, but doesn't converting to a folio_trylock() introduce entirely new spurious > failures due to folio lock contention? For the task running on CPU 1, yes. But the folio does get split rather than probably both failing. > > +++ b/mm/madvise.c > > @@ -405,9 +405,10 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > if (next - addr != HPAGE_PMD_SIZE) { > > int err; > > > > + if (!folio_trylock(folio)) > > + goto huge_unlock; > > Doesn't this violate lock ordering? > > >From rmap.c: > > folio_lock > ... > mm->page_table_lock or pte_lock > > So now you hold the ptl lock _before_ you obtain the folio lock? > > I'm not sure if it being a trylock gets us out of that particular situation? And > I'd be reticent for us to violate it... unless I'm missing something :) It's a common way of getting out of a lock ordering problem. Surprised you've not encountered it as a solution to the Dining Philosophers problem. We have even weirder solutions to "I want to sleep on the folio lock but not with a reference held", and such might be appropriate here if we want to prevent the spurious failure on CPU 1. See the DROP behavior in mm/filemap.c. See folio_put_wait_locked() in mm/filemap.c, not that it's exported. We couldn't quite make that work here since the whole point is to _never_ get the refcount on the folio if somebody else has the lock, and once we've dropped the PTL, the folio might have been split and thus not be the folio we want any more (indeed it may have been freed, reallocated and now be a pointer to a tail page instead of a folio).