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 2026CC7EE2F for ; Mon, 29 May 2023 12:43:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 785E0900003; Mon, 29 May 2023 08:43:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 73659900002; Mon, 29 May 2023 08:43:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5FE23900003; Mon, 29 May 2023 08:43:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 526A8900002 for ; Mon, 29 May 2023 08:43:54 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 1FC2012011D for ; Mon, 29 May 2023 12:43:54 +0000 (UTC) X-FDA: 80843259588.29.D56C1E0 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf12.hostedemail.com (Postfix) with ESMTP id C949940005 for ; Mon, 29 May 2023 12:43:50 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=euZA2upp; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=KEt7R7TP; dmarc=none; spf=pass (imf12.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1685364231; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=GzCtx5Td+eoDNVY0dXN9m8+YNDjh3ADQgtk2GxyYC5g=; b=AxNA0hE2vpXg/Xqjdb+TsXt0oVH4w3TEq6CIw8CLJbCTNjOmQmUBXveI2C5HwLc9+Erzg4 smrhG37KfeMRkPnSmpnwhYH3ZeBz7pFM6FQS4wcpgvffzVuEEhP2Fsy5aNYwxY03mYMQBs nV+81RSBC1F0LQUW6sJ7IjWOQTfYZVc= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=euZA2upp; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=KEt7R7TP; dmarc=none; spf=pass (imf12.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685364231; a=rsa-sha256; cv=none; b=8jxIed/G1pkj0zbWqNnVYeTD31os8AHukJCAtJR+6FFBeO05sUrIjLb3dIYC59uCzyg58y tICaYdWxxDegk/Kl4RMCG7glSTrS/5m/Vv/GLPC4Khjfb+IKPGdC3FYf2qRLhQ8DR11vwg 0lGmvBCdZoukJBQvuazH4Y+yknup+gU= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 551431F88E; Mon, 29 May 2023 12:43:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1685364229; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GzCtx5Td+eoDNVY0dXN9m8+YNDjh3ADQgtk2GxyYC5g=; b=euZA2uppJ9p3Gv6P0DaFZ2l2OvM3BhioXsCROMC8vzMsI1ZRqHoGajYwlkedZb4z3sx6mi N7ntNcJtyVU3e/sPg2SUYxMLiks3ByU2pYd4BihU0sVCPcTbImgFeqSbzDALeMrkV+lo/D 9K/5p2rM83QT05xTNVOVSCi+L+TOhv8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1685364229; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GzCtx5Td+eoDNVY0dXN9m8+YNDjh3ADQgtk2GxyYC5g=; b=KEt7R7TPRgqrGjgZDCTbEwvGsi3WHoikP/8JKHT67Bm3YqIyUaozS38aWDuKNtYakfwz1y i2I9beTY2u7P3pAg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 2FB171332D; Mon, 29 May 2023 12:43:49 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id NROFCgWedGRgHAAAMHmgww (envelope-from ); Mon, 29 May 2023 12:43:49 +0000 Message-ID: <6695b7e5-9fa5-fae8-8a66-cc5985b0baaf@suse.cz> Date: Mon, 29 May 2023 14:43:48 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.2 Subject: Re: [PATCH 3/4] mm: compaction: Update pageblock skip when first migration candidate is not at the start Content-Language: en-US To: Mel Gorman Cc: Andrew Morton , Jiri Slaby , Maxim Levitsky , Michal Hocko , Pedro Falcato , Paolo Bonzini , Chuyi Zhou , Linux-MM , LKML References: <20230515113344.6869-1-mgorman@techsingularity.net> <20230515113344.6869-4-mgorman@techsingularity.net> <20230529103342.esek6r5fvmft2nky@techsingularity.net> From: Vlastimil Babka In-Reply-To: <20230529103342.esek6r5fvmft2nky@techsingularity.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: C949940005 X-Stat-Signature: ydsgmiab9s9mz3yj4yaarwge3rkdqkgg X-HE-Tag: 1685364230-515082 X-HE-Meta: U2FsdGVkX1+yX5WPYjKUpGMU324fNaZFZ9XkgYM1eDrQq9OeJwzSyuG2wmO/z5+nH6OVVN0RV+o7ba1Z2QmwX5rt7dqK4+QLLYUkku+M8ZtaYkqJ53nXfJDIQLS9kS9tQ/xeG34LNR4b/yAffFzyUP4BgSS39QLufypYD+SeCPvbMdk0gtlpDktFK9HFD/7AnBpIwyHz1TKk2qYkIPJ5mk42GPOrXeIg8UHREBHNMv0aL41DbVCdZnrvErYiZrQSinwZ+qS62CcuPcg6CrpgzlEBL7B1xAm3enEHP3dDtr2CYre3EM2wE+CwJucw8JhR5sTm8VmXsCbVk/CYSS2tz0np+7BArAHnWHrEkIeq+CmUT8eGbFzmW0HAT44OiHSRUcJlGVfDlu+FTXWTfqvnUFDK3n7Y5vC50+Wk9nYdgmetzpXRFjsKhoGYE3S3z1TWLZqCG6R/+8bNYLBt6fU+TebyPVpMU0p7FsCff1heJDsFAeH3fkBZMi3HeCrXXuxhr04+MPWTepDbYC12FDtwaTyEcmsPCcWSW+9gN1X8jw+Zhp8D4kqOeAP01L/DdaAmkZHEMsmRIXqb96ldDGnYni+TUHCMG282GqmWU08hDwtBQXkJbFjB81896YiJKnKtqB6VqmEQNLrwSBMexcfr0/lZAF1aFzbCZdTod3IMDtj5Klg9Z07Tf6c2b/o/m9eouWrw20OWnxFKXGFM8hhjvHCpOxkLTfKlV2O1Sx68CsEFDIz6yj/AVFh3LGafMRvHLVhl1yGVsXkvZrbf4B+1b7A4w9VcFP1+EdAPw9TAk90sSm+930nd8d/H2qt212I6jMGMy0SsJXW5haww1BwqEAUxoPwJSq0xHDDx+H/xf24dgVWR9blsOQiPn+ZyIyGpnBV+3VihLtK48T1C/9N1nzpZjAlKVLF4piLyskwhJ/kD04LnzQZgesViZJsilK2ugCZwK+hM1JNsKKRT/E+ XbVaKtP8 LHWEWK13fPZ0aOkspv3UBvP/1GJn0mT+Cp8N9t2VSp7WcknaevrdjLrY5kXG3ElaiEPykiqqGVz9+cqKjhPNDOQDf7ODz2kV7IrSiMJQITQQUHOiMA1vmQJAy7WsCBN2rap11zRPDnz8FFx85r5CNWVI92MXV7tGwKMMowsIt6DN8xPLcOy3w3WrysLeF6YKXeHHSiPcVbpyhDEE0GY6SJ+Ufrnb3+YZI0Ed0BJYS0mza6tJ9GbcD8RvqWaBgfH9TKFWQIkdfuC9PrypW8MhF/WxbGUdw1sTuxYJZNdcW96A1iPIHRvzDRtzGwKrzKFdFzKyzSWJdCWQCX7c= 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 5/29/23 12:33, Mel Gorman wrote: > 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 :( Yeah that should work! I think it should be even folded to 3/4 but if you want separate, fine too. > 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; > } >