From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75526C43219 for ; Tue, 8 Nov 2022 13:57:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AD9556B0072; Tue, 8 Nov 2022 08:57:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A89C08E0001; Tue, 8 Nov 2022 08:57:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9781D6B0074; Tue, 8 Nov 2022 08:57:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 8542D6B0072 for ; Tue, 8 Nov 2022 08:57:10 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 339B1160217 for ; Tue, 8 Nov 2022 13:57:10 +0000 (UTC) X-FDA: 80110426620.25.33983DB Received: from outbound-smtp42.blacknight.com (outbound-smtp42.blacknight.com [46.22.139.226]) by imf23.hostedemail.com (Postfix) with ESMTP id 9843A140003 for ; Tue, 8 Nov 2022 13:57:08 +0000 (UTC) Received: from mail.blacknight.com (pemlinmail06.blacknight.ie [81.17.255.152]) by outbound-smtp42.blacknight.com (Postfix) with ESMTPS id CF1461B8A for ; Tue, 8 Nov 2022 13:57:06 +0000 (GMT) Received: (qmail 24642 invoked from network); 8 Nov 2022 13:57:06 -0000 Received: from unknown (HELO techsingularity.net) (mgorman@techsingularity.net@[84.203.198.246]) by 81.17.254.9 with ESMTPSA (AES256-SHA encrypted, authenticated); 8 Nov 2022 13:57:06 -0000 Date: Tue, 8 Nov 2022 13:57:03 +0000 From: Mel Gorman To: Hugh Dickins Cc: Andrew Morton , Yu Zhao , Vlastimil Babka , Nicolas Saenz Julienne , Marcelo Tosatti , Michal Hocko , Marek Szyprowski , LKML , Linux-MM Subject: Re: [PATCH v2] mm/page_alloc: Leave IRQs enabled for per-cpu page allocations Message-ID: <20221108135703.cthdmjl2lfpnairr@techsingularity.net> References: <20221104142259.5hohev5hzvwanbi2@techsingularity.net> <97b7ae87-797c-4ebb-d2d3-9415975188@google.com> <20221107111649.rzfgqk3ebvicsuyw@techsingularity.net> <486a93d7-4140-60c9-1aa8-8dafc8c66210@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <486a93d7-4140-60c9-1aa8-8dafc8c66210@google.com> ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf23.hostedemail.com: domain of mgorman@techsingularity.net designates 46.22.139.226 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667915829; a=rsa-sha256; cv=none; b=nDU2yTZHogFmO1CvZXXC++ywo2ZPsUtWUwNBTlRh2h9tKrSr0WxLYNAeAcdx6AsI18TtU8 kYw3BKJrQeKE5CzhTn699us8dP5UMQ17M+U1+9x/pi5X1HOOfB8zr+eOAgRRf1/N4QLF5N 2M5mbyPCvvgNjtPuBHLT9GRL7NZIlyM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667915829; h=from:from:sender: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: in-reply-to:in-reply-to:references:references; bh=dz0mkNciTu2jBAM5RioD1xVxD4+Rr/EgrFmqsQ2ZWEI=; b=eYj1OEmL6lNCQLVkmEWTY9sXs5QsPjpZtY9Q3S1Ldp90SDJ7rVTkO3sp9khNOJBd9dJHPo Ucp8K6RdNt1qlAX3GA1oj4XKrHwfHXspCAxEBQToAdHrNqCrc/RiXiRgYgp1yHYgwAk1q0 U7Ub2+tt7qr7MqNVqor+AFrOxmo5hRI= X-Rspam-User: Authentication-Results: imf23.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf23.hostedemail.com: domain of mgorman@techsingularity.net designates 46.22.139.226 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 9843A140003 X-Stat-Signature: k3raqxfg8qfx5u15e8om71nwgeei9r96 X-HE-Tag: 1667915828-306406 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Nov 08, 2022 at 01:40:48AM -0800, Hugh Dickins wrote: > On Mon, 7 Nov 2022, Mel Gorman wrote: > > On Sun, Nov 06, 2022 at 08:42:32AM -0800, Hugh Dickins wrote: > > > On Fri, 4 Nov 2022, Mel Gorman wrote: > > > What I'm really trying to do is fix > > > the bug in Linus's rmap/TLB series, and its interaction with my > > > rmap series, and report back on his series (asking for temporary > > > drop), before next-20221107 goes down in flames. > > > > > > I'd advocate for dropping this patch of yours too; but if it's giving > > > nobody else any trouble, I can easily continue to patch it out. > > > > > > > Given that you tested the patch against v6.1-rc3, it's clear that the > > patch on its own causes problems. Having a reproduction case will help > > me figure out why. > > Sorry for appearing to ignore your requests all day, Mel, but I just > had slightly more confidence in debugging it here, than in conveying > all the details of my load (some other time), and my config, and > actually enabling you to reproduce it. Had to focus. > Ok, understood. If you ever get the chance to give me even a rough description, I'd appreciate it but I understand that it's a distraction at the moment. Thanks for taking the time to debug this in your test environment. > Got it at last: free_unref_page_list() has been surviving on the > "next" in its list_for_each_entry_safe() for years(?), without doing > a proper list_del() in that block: only with your list_del() before > free_one_page() did it start to go so very wrong. (Or was there any > way in which it might already have been wrong, and needs backport?) > I think it happened to work by coincidence since forever because it was always adding to the same list. Even though the temporary list was thrashed, it is always either ignored or reinitialised. I've made this a standalone patch which is at the end of the mail. I can change the Reported-by to a Signed-off-by if you agree. While it doesn't fix anything today, it may still be worth documenting in git history why that list_del exists. > Here's a few things to fold into your patch: I've moved your > list_del() up to cover both cases, that's the important fix; > but prior to finding that, I did notice a "locked_zone = NULL" > needed, and was very disappointed when that didn't fix the issues; This is a real fix but it also should happen to work properly which is less than ideal because it's fragile. > zone instead of page_zone(page), batch_count = 0, lock hold times > were just improvements I noticed along the way. > The first is a small optimisation, the second addresses a corner case where the lock may be released/reacquired too soon after switching from one zone to another and the comment fix is valid. I've simply folded these in directly. The standalone patch is below, I'm rerunning tests before posting a short v3 series. Thanks! --8<-- mm/page_alloc: Always remove pages from temporary list free_unref_page_list() has neglected to remove pages properly from the list of pages to free since forever. It works by coincidence because list_add happened to do the right thing adding the pages to just the PCP lists. However, a later patch added pages to either the PCP list or the zone list but only properly deleted the page from the list in one path leading to list corruption and a subsequent failure. As a preparation patch, always delete the pages from one list properly before adding to another. On its own, this fixes nothing although it adds a fractional amount of overhead but is critical to the next patch. Reported-by: Hugh Dickins Signed-off-by: Mel Gorman --- mm/page_alloc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 218b28ee49ed..1ec54173b8d4 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3546,6 +3546,8 @@ void free_unref_page_list(struct list_head *list) list_for_each_entry_safe(page, next, list, lru) { struct zone *zone = page_zone(page); + list_del(&page->lru); + /* Different zone, different pcp lock. */ if (zone != locked_zone) { if (pcp) -- Mel Gorman SUSE Labs