From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 41363282F3B for ; Thu, 9 Apr 2026 18:44:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775760242; cv=none; b=bUaLZ+1xYZPT9Go2F0wczIacxb2hpY2TwK4nqpjwvp/l8EtpwEy0+VddxmSD+aPvefIbx4dyxY1bakVQ6nQgCwvPX266klVnwyQ52yxy3V7vsNyn4d5I0Vrl1ZRcrV0GqMjGIJVRWBSGoUKOCQD4aWd/myf4Nztq8FTUNocdwSk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775760242; c=relaxed/simple; bh=uIg82L4DjC6SQMflVbk9FHtb5Uti2at/qU8a4k6XjA0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QVG/xfOMQulpOvNVeV94rKbZklkb6nBb4dNnGCIs7GwCLjK/lfciXdMBIbnJjqNNzkqKzQ7sxK+EvNo9TeA3VMrcyYElSrY8++CxRXOswQ1F1c5tsZI1jteiteZ1sDqiYgmNUL/ZEEpFx0elWadDXyN2kuH+GACWR3fSRtxiX2U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=IAL0yO0g; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="IAL0yO0g" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 537ADC19424; Thu, 9 Apr 2026 18:43:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775760241; bh=uIg82L4DjC6SQMflVbk9FHtb5Uti2at/qU8a4k6XjA0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IAL0yO0grcurODGd5NI/NYJiE6E0PMYwjOcSMTrGSvPHBUOttOKPTnUesNGyeQToB MM4nYrBdTzGIw6opFv0NrLmjJvxE0/KYe3Yt/ioYsSlXHjvD3RMjNBB3wkS4m8OJMX MX5387cmvjSkWAq3O3rp7qH/hiMUTI4PfLiP1e98RAvU8LOBHjzqvJTHPYFKqZVD89 F3Fu9+DEKjXpVFLIAchYvkGJ6QgWE2iIKX5ax/q+7R//u/SVY6HqGzW2khM112y8Za YKS8stNZ8FfMvQPHhKylPOHR8+Al+2WNG1Nk6L4CQ+44Zoj3hMwM1NxcBC1P8kU5ak z0+nLzEmcC4uQ== Date: Thu, 9 Apr 2026 19:43:55 +0100 From: Lorenzo Stoakes To: Haakon Bugge Cc: Hugh Dickins , John Hubbard , Joseph Salisbury , Andrew Morton , David Hildenbrand , Chris Li , Kairui Song , Jason Gunthorpe , Peter Xu , Kemeng Shi , Nhat Pham , Baoquan He , Barry Song , "linux-mm@kvack.org" , LKML Subject: Re: [RFC] mm: stress-ng --mremap triggers severe lruvec lock contention in populate/unmap paths Message-ID: References: <4a4f5b48-8a1d-48f8-8760-0f5d43b5d483@nvidia.com> <982e5964-5ea6-eaf7-a11a-0692f14a6943@google.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: On Thu, Apr 09, 2026 at 06:15:50PM +0000, Haakon Bugge wrote: > > > > On 9 Apr 2026, at 20:03, Lorenzo Stoakes wrote: > > > > On Tue, Apr 07, 2026 at 05:35:18PM -0700, Hugh Dickins wrote: > >> On Tue, 7 Apr 2026, John Hubbard wrote: > >>> On 4/7/26 1:09 PM, Joseph Salisbury wrote: > >>>> Hello, > >>>> > >>>> I would like to ask for feedback on an MM performance issue triggered by > >>>> stress-ng's mremap stressor: > >>>> > >>>> stress-ng --mremap 8192 --mremap-bytes 4K --timeout 30 --metrics-brief > >>>> > >>>> This was first investigated as a possible regression from 0ca0c24e3211 > >>>> ("mm: store zero pages to be swapped out in a bitmap"), but the current > >>>> evidence suggests that commit is mostly exposing an older problem for > >>>> this workload rather than directly causing it. > >>>> > >>> > >>> Can you try this out? (Adding Hugh to Cc.) > >>> > >>> From: John Hubbard > >>> Date: Tue, 7 Apr 2026 15:33:47 -0700 > >>> Subject: [PATCH] mm/gup: skip lru_add_drain() for non-locked populate > >>> X-NVConfidentiality: public > >>> Cc: John Hubbard > >>> > >>> populate_vma_page_range() calls lru_add_drain() unconditionally after > >>> __get_user_pages(). With high-frequency single-page MAP_POPULATE/munmap > >>> cycles at high thread counts, this forces a lruvec->lru_lock acquire > >>> per page, defeating per-CPU folio_batch batching. > >>> > >>> The drain was added by commit ece369c7e104 ("mm/munlock: add > >>> lru_add_drain() to fix memcg_stat_test") for VM_LOCKED populate, where > >>> unevictable page stats must be accurate after faulting. Non-locked VMAs > >>> have no such requirement. Skip the drain for them. > >>> > >>> Cc: Hugh Dickins > >>> Signed-off-by: John Hubbard > >> > >> Thanks for the Cc. I'm not convinced that we should be making such a > >> change, just to avoid the stress that an avowed stresstest is showing; > >> but can let others debate that - and, need it be said, I have no > >> problem with Joseph trying your patch. > > > > Yeah, the test case (as said by others also) is rather synthetic, and it's a > > test designed to saturate, if not I/O throttled by swap then we hammer the > > populate path. It feels like a micro-optimisation for something that is not (at > > least not yet demonstrated to be) an actual problem. > > > > stress-ng is not a benchmarking tool per se, it's designed to eek out bugs. > > > > So really we need to see a real-world case I think. > > > >> > >> I tend to stand by my comment in that commit, that it's not just for > >> VM_LOCKED: I believe it's in everyone's interest that a bulk faulting > >> interface like populate_vma_page_range() or faultin_vma_page_range() > >> should drain its local pagevecs at the end, to save others sometimes > >> needing the much more expensive lru_add_drain_all(). > > > > I mean yeah, but I guess anywhere that _really_ needs to be sure of the drain > > has to do an lru_add_drain_all(), because it'd be fragile to rely on > > lru_add_drain()'s being done at the right time? > > > >> > >> But lru_add_drain() and lru_add_drain_all(): there's so much to be > >> said and agonized over there They've distressed me for years, and > >> are a hot topic for us at present. But I won't be able to contribute > >> more on that subject, not this week. > > > > Yeah they do feel rather delicate... :) sometimes you _really do_ need to know > > everything's drained. But other times it feels a bit whack-a-mole. > > > > I also do agree it makes sense to drain locally after a batch operation. > > > > It all comes down to whether this manifests in a real-world case, at which point > > maybe this is a more useful change? > > > >> > >> Hugh > >> > >>> --- > >>> mm/gup.c | 13 ++++++++++++- > >>> 1 file changed, 12 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/mm/gup.c b/mm/gup.c > >>> index 8e7dc2c6ee73..2dd5de1cb5b9 100644 > >>> --- a/mm/gup.c > >>> +++ b/mm/gup.c > >>> @@ -1816,6 +1816,7 @@ long populate_vma_page_range(struct vm_area_struct *vma, > >>> struct mm_struct *mm = vma->vm_mm; > >>> unsigned long nr_pages = (end - start) / PAGE_SIZE; > >>> int local_locked = 1; > >>> + bool need_drain; > >>> int gup_flags; > >>> long ret; > >>> > >>> @@ -1857,9 +1858,19 @@ long populate_vma_page_range(struct vm_area_struct *vma, > >>> * We made sure addr is within a VMA, so the following will > >>> * not result in a stack expansion that recurses back here. > >>> */ > >>> + /* > >>> + * Read VM_LOCKED before __get_user_pages(), which may drop > >>> + * mmap_lock when FOLL_UNLOCKABLE is set, after which the vma > >>> + * must not be accessed. The read is stable: mmap_lock is held > >>> + * for read here, so mlock() (which needs the write lock) > >>> + * cannot change VM_LOCKED concurrently. > >>> + */ > > > > BTW, not to nitpick (OK, maybe to nitpick :) this comments feels a bit > > redundant. Maybe useful to note that the lock might be dropped (but you don't > > indicate why it's OK to still assume state about the VMA), and it's a known > > thing that you need a VMA write lock to alter flags, if we had to comment this > > each time mm would be mostly comments :) > > > > So if you want a comment here I'd say something like 'the lock might be dropped > > due to FOLL_UNLOCKABLE, but that's ok, we would simply end up doing a redundant > > drain in this case'. > > > > But I'm not sure it's needed? > > > >>> + need_drain = vma->vm_flags & VM_LOCKED; > > > > Please use the new VMA flag interface :) > > > > need_drain = vma_test(VMA_LOCKED_BIT); > > > > I think we all agree that the stress-ng test case is synthetic. I evaluated John's patch as I understood that was requested, and the outcome was, merely, as expected. (Please wrap lines :) Ack re: synthetic. Thanks for evaluating it! I don't think John's patch is incorrect per se, but as I said on reply further up thread, I fear this all might be rather a distraction from a real world perspective, because you'd expect similar results due to reasons other than the lruvec being a bit *ahem* sub-optimal shall we say. > > The fio case is more interesting, as, if my runs make sense, it improves IOPS by ~20% and avoid threads being stuck at termination. But, I am not intimate with fio, so take that part as a grain of salt. That is interesting, but again I wonder what it's actually measuring, because if things are getting stuck because of saturation from stress-ng doing insane things (hammering the hell out of madvise(..., MAP_PAGEOUT), mremap(), munmap() in the hot path, all while not caring about NUMA node locality), then that's sort of what you'd expect I guess? I guess the only way to avoid possibly measuring the wrong thing is to examine a real-world case, and if there is something lurking there with lruvec scalability (very possible) then we can definitely look at that! Thanks for digging into this! > > > Thxs, Håkon > > Cheers, Lorenzo