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 F4079C4332F for ; Tue, 8 Nov 2022 15:47:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C0EB16B0072; Tue, 8 Nov 2022 10:47:28 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BBF046B0073; Tue, 8 Nov 2022 10:47:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A877F6B0074; Tue, 8 Nov 2022 10:47:28 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 995256B0072 for ; Tue, 8 Nov 2022 10:47:28 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 6398A1210A1 for ; Tue, 8 Nov 2022 15:47:28 +0000 (UTC) X-FDA: 80110704576.28.44567E0 Received: from mail-qt1-f179.google.com (mail-qt1-f179.google.com [209.85.160.179]) by imf29.hostedemail.com (Postfix) with ESMTP id 1930012000C for ; Tue, 8 Nov 2022 15:47:26 +0000 (UTC) Received: by mail-qt1-f179.google.com with SMTP id fz10so8874325qtb.3 for ; Tue, 08 Nov 2022 07:47:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=zrmtWHF5svuELzO7ajcbneYYKL4MlzN5FNv548nxCrA=; b=Zgst8G0oRQ5ICrfogThsRX0A6nMRThBhTvSH3pMKfZSnjRFGCwPIhf02v9ntE044oP 52AEgmD7hB5F5x64O7q38Q43rpEoKlMyYp98gpzJBVT584JtoQnSzRNzj9ckbTmhtfMr fbS5lGj2J7g/bU+t3Ls4RXAgoExcZ0NnzqP/cuxWxBurQEkKJxgZXe7BpQG3Hl8ZsfVu 2L6spjh+RpXddGhJ/U3fkvcdsnP82/Fj2QP84AxHqqCV7LJxFkehnJm7ImOur3bCFqbJ Wv/DHdeTOTWO+ngssQFFhUg8ONiSOeD4SQ2b9r6SguZq5GbG8M1kF8kCtc/btpQzSWCu kyxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zrmtWHF5svuELzO7ajcbneYYKL4MlzN5FNv548nxCrA=; b=ehQ54Wof6sg7pPn7bjXnDvmbDno3ZzU1Jvgq/ej4/CNxUQXwdHLcfQN0Dy9DTaoU5H MUVyNH/qxtu5CTRbcmRyOszQmn54KKMnkJ8EFSh1p95Vm9PC4mBO+5w35Nso3vW/S76t a28Qn7pR7xBF85l3BHoE8hHEqlDi9qYmFEoHLla4suUGNnvX8picjlxzuDHhTJKvcfxj YbIGjfzle1IR540887fqB7jqB3ywJzN4qBQrvjabwT5MA3FmNzYfXYmGxDc14J4sbT75 jkAnaEfncHG7XFEJO7qJRQHafeCKsfN98wE0EorMWlCcRC5WLLB6uSIsemZoJaPzjMeC hQtw== X-Gm-Message-State: ACrzQf1BAOiii3SkYeff0x3xYA/acfiXojVuSngcICcTKyhCJc9Xd5N7 GQqwCwl47rcZdwuSMecfh8RAhA== X-Google-Smtp-Source: AMsMyM75P96/HEXYrQlJIlj7Qzfy6nt1fsvtexwWn6NmpjoTeB5ppWCs2d7xZys/FZUJiDTFKg+cow== X-Received: by 2002:a05:622a:13c8:b0:3a5:4cee:4ecc with SMTP id p8-20020a05622a13c800b003a54cee4eccmr24123780qtk.351.1667922446237; Tue, 08 Nov 2022 07:47:26 -0800 (PST) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id g10-20020a05620a40ca00b006f3e6933bacsm9660665qko.113.2022.11.08.07.47.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Nov 2022 07:47:24 -0800 (PST) Date: Tue, 8 Nov 2022 07:47:12 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@ripple.attlocal.net To: Mel Gorman cc: Hugh Dickins , 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 In-Reply-To: <20221108135703.cthdmjl2lfpnairr@techsingularity.net> Message-ID: References: <20221104142259.5hohev5hzvwanbi2@techsingularity.net> <97b7ae87-797c-4ebb-d2d3-9415975188@google.com> <20221107111649.rzfgqk3ebvicsuyw@techsingularity.net> <486a93d7-4140-60c9-1aa8-8dafc8c66210@google.com> <20221108135703.cthdmjl2lfpnairr@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667922447; a=rsa-sha256; cv=none; b=QViJMr+s7DKhHLNVx4nuopH/lMByE/Tne5sbiOjMAM4uD3gmRIBbIUJz1mz3ktZKWnWOfF fcU97waNsfScrM+GqFu9apmH5Ptgf+inzI7u/Ewe0yNosaqDhW1gQx6uUo/keU9OaIvjlt H3b04mRMMevG6SOHlZVWJFNeQJKrpqQ= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Zgst8G0o; spf=pass (imf29.hostedemail.com: domain of hughd@google.com designates 209.85.160.179 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667922447; 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:dkim-signature; bh=zrmtWHF5svuELzO7ajcbneYYKL4MlzN5FNv548nxCrA=; b=MRc1rmpwpVvHmrCH3MWMnO9aU+/CLzjupHEy0OhcuMoFJdrHM84GPW1coojOUJg+gq+mkg zYzVjIOJdmA1wvahVqJmr+1HnIDoG5qWiW7fOH2mi1tb8HcgtVaa02cPoixU5Ikn/vZmB9 jo4HWK8I6XE16hS5Y38/21piYW/T94k= X-Stat-Signature: daxzngooo77q3xpk46we579t69w4ponh X-Rspamd-Queue-Id: 1930012000C Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Zgst8G0o; spf=pass (imf29.hostedemail.com: domain of hughd@google.com designates 209.85.160.179 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1667922446-213520 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, 8 Nov 2022, Mel Gorman wrote: > 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. I have sent it out two or three times before (I prefer privately, so my limited shell scripting skills are not on public view!): later in the day I'll look out the last time I sent it, and just forward that to you. No doubt I'll have tweaked it here and there in my own usage since then, but that will be good enough to show what I try for. Wonderful if it could get into something like mmtests, but I should warn that attempts to incorporate it into other frameworks have not been successful in the past. Maybe it just needs too much handholding: getting the balance right, so that it's stressing without quite OOMing, is difficult in any new environment. Perhaps it should restart after OOM, I just never tried to go that way. > > > 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. Yes, worth separating out to its own git commit. Just continue with what you already have, Reported-by me, Signed-off-by you, thanks for asking. > > > 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. I thought that if the next page's zone matched the stale locked_zone, then it would head into free_unref_page_commit() with NULL pcp, and oops there? But I've certainly never seen that happen (despite first assuming it was responsible for my crashes), so maybe I read it wrong. > > > 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. Great, thanks Mel. Hugh > > 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