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 6DF91C76188 for ; Wed, 5 Apr 2023 10:31:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C03DA6B007E; Wed, 5 Apr 2023 06:31:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BB45D6B0080; Wed, 5 Apr 2023 06:31:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A7AD0900002; Wed, 5 Apr 2023 06:31:48 -0400 (EDT) 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 99E0C6B007E for ; Wed, 5 Apr 2023 06:31:48 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 5396781024 for ; Wed, 5 Apr 2023 10:31:48 +0000 (UTC) X-FDA: 80646971496.14.0867488 Received: from outbound-smtp01.blacknight.com (outbound-smtp01.blacknight.com [81.17.249.7]) by imf11.hostedemail.com (Postfix) with ESMTP id 65F2D40019 for ; Wed, 5 Apr 2023 10:31:45 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf11.hostedemail.com: domain of mgorman@techsingularity.net designates 81.17.249.7 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=1680690706; 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=ESxI/W/G4zxz7BD/VyJHpW5wlwDH6jsIwbHRWD3qr2c=; b=WdSmpOrtrcf8Bvzd7migMKo1n2EAEmoilljarLEebXtv+zpl3mJWwpU+BqpmB+hbhhveCm 5R4dPtl42NLULSN6KM+tpis+QLoNWP5V3G7GbkRn/EsaDbStQH2csKrgyxdhb5ud/wI8w+ 2zY56jukWfMSQH+xmkOnT+fEorIMkRU= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf11.hostedemail.com: domain of mgorman@techsingularity.net designates 81.17.249.7 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680690706; a=rsa-sha256; cv=none; b=Cjpx/Py11dAlGSIveG3bQ1nPnknurG7WoyYalQrZVCvjXGiZ6H2VTLlmPyQ1pVjvFWwrSv tlLPgscRLoRJ4okXpnnxAhBmYTXLRbVcg80USQan35S45EVRD6K7lROgFFlF9veo7+W3za u/eq+YftQcQ0PM2UZO55wix4caBmdNo= Received: from mail.blacknight.com (pemlinmail06.blacknight.ie [81.17.255.152]) by outbound-smtp01.blacknight.com (Postfix) with ESMTPS id 89229C4ACE for ; Wed, 5 Apr 2023 11:31:43 +0100 (IST) Received: (qmail 7474 invoked from network); 5 Apr 2023 10:31:43 -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); 5 Apr 2023 10:31:43 -0000 Date: Wed, 5 Apr 2023 11:31:41 +0100 From: Mel Gorman To: Baolin Wang Cc: akpm@linux-foundation.org, osalvador@suse.de, vbabka@suse.cz, william.lam@bytedance.com, mike.kravetz@oracle.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] mm: compaction: consider the number of scanning compound pages in isolate fail path Message-ID: <20230405103141.yu7p53psbvstv6kg@techsingularity.net> References: <73d6250a90707649cc010731aedc27f946d722ed.1678962352.git.baolin.wang@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <73d6250a90707649cc010731aedc27f946d722ed.1678962352.git.baolin.wang@linux.alibaba.com> X-Rspamd-Queue-Id: 65F2D40019 X-Stat-Signature: ducrbt66ack9gfkuqy11gaeee6buyitk X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1680690705-972938 X-HE-Meta: U2FsdGVkX18Nf7pDDOxO54Bm3GkVy2Ho10XOAicpH+5ibpPmvxfAesxsXeginGFKy6kfZN/zMc0XQYSxjWdsQ4/DZHim9G++8mJf2sVxauk8dTRp60vvWbeyIVXI+EQabqS0OZthA5e3UiVGJCmcpUK3Oz5QTW2hS2Vb0pCtMr0U1aNCumSej1/ym4qtWZyo/HsnycMRpufLjaJwBn+Rm5uDSSvFO0r+AP2pqKH7uUIRw78/79AAfUe1sbTXaBeYDF68dA7oD/Vzoolds+8ytBHSL21HacnsCFh9j2HpyqI9adYJ4GUFH1kOqTS1dv8I/k56AopopHEuhguOABm5SBG8s+1O6drtHabmXRuAzK+OINO2RC97uJbweSDm+bdeimnJRvidSuvJXIAnZTyL+uJV5jgjX8Ejl8rPVFuu6Xraey52AQ013IOL8DmbCn3FdiCsJ92gt92VBemdBf5l1ISazHK9LnXXGBJuUPidyAm5fEvo3o0S4OvlWHQAZK6Y7tatUxnN4WZcpqAWDqAw/piulegNyjYNDM9NRU2+YPPCV+LPaGpD5/QzqGl0H4N/aQ+DWzxCrePvObnK4BEyJBbat1EVQG5EgNebXOeyIyZYws/m74jRLDIY3peECLiJiIKvZtEtOrqjwfW7KqVP/6+JS2g6CoMQxvecsaRfFzoavHv1mSWpp3CU2/y0fvZBU6GM4IHp6M6rZMa0Y4YbFq9sCnVSpDyrsjmnUwh00xrTKh1J7/ABrLxHrbobyj+zOWfMu0DdDoOBYVeBgiAdruv4JdoC6M6INQk69GXOJLmNk3eQxy0REamZImkz3TR1qj0OHSfgQnd8/R6zu/NE7SMv5HL7ab6ArVQCTutvETxXeW84WiKL2dxIyPksr7r5zU33mW2dGSVGBiFvq8sMam/WLb0L/qwrX8MQ7jPUaMt7BK2gRWaglsEq1DsSEMbitL6QajdqAuzRBv3LqMW tlVSypJ5 f2CwN7hviidlSA2uiJWnwbhVzr5wfeQYqcp5lQbcrq6i/ikCxjqKRvLhVNgrno80PJhLhr6A7zcAez1vc9dNlMYgYxol9xjQudykwA54g6/de1+FpbMEwYyjFIVbB1yHJMxRU5UFlWzqdtJcO2VlTHKDF41bcWYHItsemPwwD6V2s9sNYTgWYSr3CW5+ZvLHejmApR0OSI0gMecDZMMcx121VdQj7A9CIAh+2UusSjKYau4S4kbaJJf2EwEVGlTJ9C3tsH4oXP5mobps= 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, Mar 16, 2023 at 07:06:46PM +0800, Baolin Wang wrote: > The commit b717d6b93b54 ("mm: compaction: include compound page count > for scanning in pageblock isolation") had added compound page statistics > for scanning in pageblock isolation, to make sure the number of scanned > pages are always larger than the number of isolated pages when isolating > mirgratable or free pageblock. > > However, when failed to isolate the pages when scanning the mirgratable or > free pageblock, the isolation failure path did not consider the scanning > statistics of the compound pages, which can show the incorrect number of > scanned pages in tracepoints or the vmstats to make people confusing about > the page scanning pressure in memory compaction. > > Thus we should take into account the number of scanning pages when failed > to isolate the compound pages to make the statistics accurate. > > Signed-off-by: Baolin Wang Acked-by: Mel Gorman However, the patch highlights weakeness in the tracepoints and how useful they are. Minimally, I think that the change might be misleading when comparing tracepoints across kernel versions as it'll be necessary to check the exact meaning of nr_scanned for a given kernel version. That's not a killer problem as such, just a hazard if using an analysis tool comparing kernel versions. As an example, consider this if (PageCompound(page)) { const unsigned int order = compound_order(page); if (likely(order < MAX_ORDER)) { blockpfn += (1UL << order) - 1; cursor += (1UL << order) - 1; nr_scanned += compound_nr(page) - 1; <<< patch adds } goto isolate_fail; } Only the head page is "scanned", the tail pages are not scanned so accounting for them as "scanned" is not an accurate reflection of the amount of work done. Isolation is different because the compound pages isolated is a prediction of how much work is necessary to migrate that page as it's obviously more work to copy 2M of data than 4K. The migrated pages combined with isolation then can measure efficiency of isolation vs migration although imperfectly as isolation is a span while migration probably fails at the head page. The same applies when skipping buddies, the tail pages are not scanned so skipping them is not additional work. Everything depends on what the tracepoint is being used for. If it's a measure of work done, then accounting for skipped tail pages over-estimates the amount of work. However, if the intent is to measure efficiency of isolation vs migration then the "span" scanned is more useful. None of this kills the patch, it only notes that the tracepoints as-is probably cannot answer all relevant questions, most of which are only relevant when making a modification to compaction in general. The patch means that an unspecified pressure metric can be derived (maybe interesting to sysadmins) but loses a metric about time spent on scanning (maybe interesting to developers writing a patch). Of those concerns, sysadmins are probably more common so the patch is acceptable but some care will be need if modifying the tracepoints further if it enables one type of analysis at the cost of another. -- Mel Gorman SUSE Labs