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 8DC85C77B7A for ; Mon, 29 May 2023 10:33:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B391E900003; Mon, 29 May 2023 06:33:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AE953900002; Mon, 29 May 2023 06:33:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 98955900003; Mon, 29 May 2023 06:33:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 853B6900002 for ; Mon, 29 May 2023 06:33:51 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 419CF140288 for ; Mon, 29 May 2023 10:33:51 +0000 (UTC) X-FDA: 80842931862.03.BABA5F1 Received: from outbound-smtp40.blacknight.com (outbound-smtp40.blacknight.com [46.22.139.223]) by imf08.hostedemail.com (Postfix) with ESMTP id 47EE616000A for ; Mon, 29 May 2023 10:33:48 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf08.hostedemail.com: domain of mgorman@techsingularity.net designates 46.22.139.223 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1685356429; 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=TuwUzMa2aLsH9o9Vi4NMsFoF3Buu9Ds5LjNx2LcZ5O4=; b=B2t/UMcD+kYtzJwphqqKMEMMP+3mqUng78/JYPtl4gjwA9yITjjq0c3RHljXSAtWdkedBN S3xIncNaVwUticWQq8TbXZubJAiGJA+miXmP8eFGN9iyl5KBW/PKJLLjbaH07e+2tMRGmx z0ycfbdZbnQn2N1vvUrZWtygjJs3EVk= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf08.hostedemail.com: domain of mgorman@techsingularity.net designates 46.22.139.223 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685356429; a=rsa-sha256; cv=none; b=tbXusD0Mx2s4oJvptA2yHou8xKweprk1T3lbzAxf/ZMvsmWbJATb3L2Ex7sui20oSwZoqj 1vqSx1xo4bxqEwwVH7S7KiNvVmk+PlCjO0T/DKvmb/HDR1fTe1Kl55gTLpj+9apPiLCRox 94G7UqqCzCobES3WL7FCZjyRl5R3ARg= Received: from mail.blacknight.com (pemlinmail05.blacknight.ie [81.17.254.26]) by outbound-smtp40.blacknight.com (Postfix) with ESMTPS id 350A11C3BA5 for ; Mon, 29 May 2023 11:33:47 +0100 (IST) Received: (qmail 10972 invoked from network); 29 May 2023 10:33:46 -0000 Received: from unknown (HELO techsingularity.net) (mgorman@techsingularity.net@[84.203.21.103]) by 81.17.254.9 with ESMTPSA (AES256-SHA encrypted, authenticated); 29 May 2023 10:33:46 -0000 Date: Mon, 29 May 2023 11:33:42 +0100 From: Mel Gorman To: Vlastimil Babka Cc: Andrew Morton , Jiri Slaby , Maxim Levitsky , Michal Hocko , Pedro Falcato , Paolo Bonzini , Chuyi Zhou , Linux-MM , LKML Subject: Re: [PATCH 3/4] mm: compaction: Update pageblock skip when first migration candidate is not at the start Message-ID: <20230529103342.esek6r5fvmft2nky@techsingularity.net> References: <20230515113344.6869-1-mgorman@techsingularity.net> <20230515113344.6869-4-mgorman@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 47EE616000A X-Stat-Signature: 9znnt7i47zdqgqd88nr76d5d35fbtrmd X-Rspam-User: X-HE-Tag: 1685356428-125657 X-HE-Meta: U2FsdGVkX1/3IIFSwJoS7e4+aVgwuzVUy6HaC2RDRt5ZiaFeLIsiXMEfKyxYfHI3gTBwNtEBaL6Q4uDfbUFsu8/XVLdS/+EPAvjUjo/mo++3E6+4Aq9MAFs5XBDpqjcMsFKFVhFz+NoMBEEUHxZWmuj7QnX6Mn9vVN+W85U8VZySGZGX94IB/E4E3SBD9nMU/oDjJvr5/oNJ2AcBWN3E5t8qfJMRrmXUfs4ayhCKXISZbQagZdvB/6Ny4KhLS4bQkBpo2LmbnZeU74OEq4nB+DpRixYxpOAXeck/S1hPLQe+TuxfZfjaotc1rJAVnX/OcIRZueW/kHaebUiWn3eQm3QB5ln3lJHsWU8eOkXkd8/WLZWcGP0IYusRuJygbH2iQi+NXTsQlMgstrKsji3qN8Qs3H5Je8KLFUav/l52vJUaqpUfwGn/B5HRjCVin47bjG7xFZel6KjvmsUZ0n3POtlJH85XrjvSKzII60g9NH1yp7fP0NEMJU/TdVtFpR5HyFspLpSNbAngCwCKO13iSeJyITl/JNW3/DgdMtimHdYlyhX+XYh+c8bJd3d8ch1Ix3NZ7qOSA3apXdTKvORLEKoqamPsX3okDjZ8eT0QlKjpe0mUOMh9PaDrqwogvz9IIRwNT/635noDH2eHJxiMdfmwCoXmXqQZJ3pwLuKuuogLxXJ59KA/jZhcqfEOyGld8yGsPckzgsMRX+OwCpAdMToEFJ/0QbldMnC7cORhSKTMNYxz1vLlNEvqOiI2v+2Qbe7Oc983QbuMYvtd3DcV6T1b5rZLPJV4F3udMpeJUJrHgOyOhOiuAOa7aeYN11e5gEuH1Wod8VPmlaBbj6ceYcnIibfgIGVuIze4Bo+srqiHY2p8jKMSaOo58Nu2dSjaYX4IAYBSZX+9v8z1TzCeylWs/dss0QXzRfX1oVUu+SrNuq3DPDOYeV426o0fGw04NAgL9aJvulEs2pvpDRd +l1HcEGq zvTYgwcBdFVzMeDQZTLW5w6BIPNyOxo5LoU8k2dZ24+uZ0Qdy9hhpUjlhFyRBdloD6XW3LN5Y/t7xhaKq0yDEN68jiPqlDSlbk+Cuz++tYZH5Oc7e6ZBqRnwp/IEF3mg+nx1CEITS8X4vpZ6BPlwoRluk81kuBlGo4p4ZoECkOKIOXwq6SSseLaRvDJ1rxyHyXM9bD1+RZxXSes6bQKIxMRSqh0IaERdqS1FpT1WzvuFJAwlQegzvMUBDmKBoKyl1vniJAMKXNIm68LC0zg8zfQHzClE3B4Po/qZjhhfeoeUAwmw= 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 Thu, May 25, 2023 at 03:37:43PM +0200, Vlastimil Babka wrote: > On 5/15/23 13:33, Mel Gorman wrote: > > isolate_migratepages_block should mark a pageblock as skip if scanning > > started on an aligned pageblock boundary but it only updates the skip > > flag if the first migration candidate is also aligned. Tracing during > > a compaction stress load (mmtests: workload-usemem-stress-numa-compact) > > that many pageblocks are not marked skip causing excessive scanning of > > blocks that had been recently checked. Update pageblock skip based on > > "valid_page" which is set if scanning started on a pageblock boundary. > > > > Signed-off-by: Mel Gorman > > I wonder if this has an unintended side-effect that if we resume > isolate_migratepages_block() of a partially compacted pageblock to finish > it, test_and_set_skip() will now tell us to abort, because we already set > the skip bit in the previous call. This would include the > cc->finish_pageblock rescan cases. > > So unless I miss something that already prevents that, I agree we should not > tie setting the skip bit to pageblock_aligned(pfn), but maybe if we are not > pageblock aligned, we should ignore the already-set skip bit, as it was most > likely being set by us in the previous iteration and should not prevent us > from finishing the pageblock? > Hmm, I think you're right. While it should not hit the original bug, migration candidates are missed until the next compaction scan which could be tricky to detect. Something like this as a separate patch? Build tested only but the intent is for an unaligned start to set the skip bet if already unset but otherwise complete the scan. Like earlier fixes, this might overscan some pageblocks in a given context but we are probably hitting the limits on how compaction can run efficiently in the current scheme without causing other side-effects :( diff --git a/mm/compaction.c b/mm/compaction.c index 91af6a8b7a98..761a2dd7d78a 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -792,6 +792,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, bool skip_on_failure = false; unsigned long next_skip_pfn = 0; bool skip_updated = false; + bool start_aligned; int ret = 0; cc->migrate_pfn = low_pfn; @@ -824,6 +825,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, } /* Time to isolate some pages for migration */ + start_aligned = pageblock_aligned(start_pfn); for (; low_pfn < end_pfn; low_pfn++) { if (skip_on_failure && low_pfn >= next_skip_pfn) { @@ -1069,10 +1071,15 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, lruvec_memcg_debug(lruvec, page_folio(page)); - /* Try get exclusive access under lock */ + /* Try get exclusive access under lock. Isolation is + * only aborted if the start was pageblock aligned + * as this may be a partial resumed scan that set + * the bit on a recent scan but the scan must reach + * the end of the pageblock. + */ if (!skip_updated && valid_page) { skip_updated = true; - if (test_and_set_skip(cc, valid_page)) + if (test_and_set_skip(cc, valid_page) && start_aligned) goto isolate_abort; }