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 B6D71C4332F for ; Tue, 12 Dec 2023 18:33:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 508796B0327; Tue, 12 Dec 2023 13:33:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4B9246B0329; Tue, 12 Dec 2023 13:33:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 35A3B6B032A; Tue, 12 Dec 2023 13:33:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 25BB76B0327 for ; Tue, 12 Dec 2023 13:33:03 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B7576806C7 for ; Tue, 12 Dec 2023 18:33:02 +0000 (UTC) X-FDA: 81559013004.23.07C8231 Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by imf22.hostedemail.com (Postfix) with ESMTP id BA215C001B for ; Tue, 12 Dec 2023 18:33:00 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=devkernel.io header.s=fm2 header.b=OjznXwIl; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=3h22ptbb; dmarc=none; spf=pass (imf22.hostedemail.com: domain of shr@devkernel.io designates 66.111.4.29 as permitted sender) smtp.mailfrom=shr@devkernel.io ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702405980; 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=tuYDtz0rZ+euf9UCS0pvvRidqHgRQ+VyODOTAYv7ja8=; b=PptEC2qHJobckVegaqvRvcnYoCujSAR6zwXNjiwAMW87FYYyedhMv7WfbmBw/m11CkVeg5 Od6kM0fmlWB5wLu8QCxw7AStjDKL3LWq7NKRRtWIZsjvrt3SjtXVMFu5Z+/Y9B9Zc/DEaf t69ap1UWenEFHAL3ZXk1Xy3+0xUyfs0= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=devkernel.io header.s=fm2 header.b=OjznXwIl; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=3h22ptbb; dmarc=none; spf=pass (imf22.hostedemail.com: domain of shr@devkernel.io designates 66.111.4.29 as permitted sender) smtp.mailfrom=shr@devkernel.io ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702405980; a=rsa-sha256; cv=none; b=xGa1d/GbXNzcj9kxrt3Sfh566ZfQHGI0/XCTnKoe4vAMwPsfy41d78cl+1Stw0VVEtbD10 oRY3tuQAHOU42Dt71fplZf/zZqg8xgQyE3IsHldLz1lle79FEq0sjiXIx87LxgWBQ7W1JL E/1XTGyBAnRBVtc+sIWpL+vnj4aHaPg= Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 0A1DF5C01A5; Tue, 12 Dec 2023 13:33:00 -0500 (EST) Received: from imap51 ([10.202.2.101]) by compute6.internal (MEProxy); Tue, 12 Dec 2023 13:33:00 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=devkernel.io; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1702405980; x=1702492380; bh=tuYDtz0rZ+ euf9UCS0pvvRidqHgRQ+VyODOTAYv7ja8=; b=OjznXwIljkD5vbMezWdOwRssOg x9/W71pB9uK1jpoBQnl3JKbr73M2t+4eMFrYPnFYBXvivVED7aMTJ55uf6Qw1lRY XVF/7fomrwZKGvLgN7RQ0RMeil8znUBwYCUhpuOtux7W3fRs0xIM9Jh9515g1zFV 9P4lgW3uFeJ+fIEqaj08BRnJtfUsD8iZcTl6YdYEsY/KGMGGQXGDiyaSzJS+gZ/V KmsSmWdoIQCMsBHSFCtn3fMMCjdxrcwQ3JAiKBe6X/+Rhd4vCj0oyYPIHivLYD+/ hWxMc6nVjQ46Subx65Q29fGa6QYiFstY6HC756WJll/WbysD+08jEyEjFgsA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1702405980; x=1702492380; bh=tuYDtz0rZ+euf9UCS0pvvRidqHgR Q+VyODOTAYv7ja8=; b=3h22ptbbEq75fuK0Z6nfn+NXsIIF0aDChRlorH8dYI2H Dx1K9MN7JBD/PRVQRHofVyf3Q+DFDL0exrddaCiv8nrM0tdSPJaZiHOEMhpQhl6y J1SM3yMoxI67AduB2a1h0oLld8OAsCGuujMDoSQhsXVPR70AwdEoEkcf4dGdWjjM 2nqH/ZfGLLGRQfBXUrdfPTXFF1NVu8BT3Q9j0JT/x0/cayb4Oun0LKAq8OOk7ozT aaaqQC8h7yLdwyNBPoybeque5/qvjxZPKSKwjq6J3ASJvY5YasgCa8HDr1cc2bTM 9RZvAlYXdCbsMD8eoijTmjbfewsMuN+PNq/RxIvOBA== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudelgedgudduudcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefofgggkfgjfhffhffvvefutgesthdtredtreertdenucfhrhhomhepfdfu thgvfhgrnhcutfhovghstghhfdcuoehshhhrseguvghvkhgvrhhnvghlrdhioheqnecugg ftrfgrthhtvghrnheplefgteffgfejtdelfeekgfetjefftdejgfdvudffiedtueevvdej gfevvdfgleetnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepshhhrhesuggvvhhkvghrnhgvlhdrihho X-ME-Proxy: Feedback-ID: i84614614:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id BD680B6008D; Tue, 12 Dec 2023 13:32:59 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-1283-g327e3ec917-fm-20231207.002-g327e3ec9 MIME-Version: 1.0 Message-Id: In-Reply-To: <9f81f89f-c3ad-4cef-a619-ad36348c8ef5@redhat.com> References: <20231204234906.1237478-1-shr@devkernel.io> <20231204234906.1237478-2-shr@devkernel.io> <9f81f89f-c3ad-4cef-a619-ad36348c8ef5@redhat.com> Date: Tue, 12 Dec 2023 10:32:39 -0800 From: "Stefan Roesch" To: "David Hildenbrand" , kernel-team@fb.com Cc: "Andrew Morton" , hannes@cmpxchg.org, riel@surriel.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v3 1/4] mm/ksm: add ksm advisor Content-Type: text/plain X-Rspamd-Queue-Id: BA215C001B X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: z56nkmpgprpn9cewzhy7tywk1pzz17iy X-HE-Tag: 1702405980-969502 X-HE-Meta: U2FsdGVkX19BsLlaVViZgMAi9+RA4blRMcIZPyJ7KXwxM65oDcD6EnVuVeLWyjkDhlHXLIVrF2XAAjlEnKxYB9gixk/epebCaL9dDc3SwotfpRzlzZVSszB2vAkrVkwfEdWHkbSm0xKZWNoxSreOYQ2RmfaLwQM6areyzAOBktFoQkYC6alGFTp5V0XZ7FDtzThJmV9h5QOXsxqLnMTC6/26vj3e8Xe7ndfmDR22vg9lwf5PDQ+ad6tOZ53FHggFCKNEOgz4SVHtMCOp7vhrYBRbX6FwO1oFDQ2DwLcBiQguSm3EDqD3yz/xQHC4E8shojmlNZfgwO7lursjVeegRSSCfaYXJM0fnSmxCDVI1u4zR2P1ZdesBB0HbNPxaPNpCYcdCPu5fJ60bqKazoQGYCQ0WPGlx1R6qYF0JqUwQ5fsNxGUf8eqic1hWgAQEKWhDEtmzBLVeKSXwEzkqb+ouI5DY67zoQSMpR0rtJTf9/cQvFT4+eVIe+/vf7Hctc+6q/S8a0cROjeW3Wgv5PUgNkedJLtRaGrisD94Qh25Jdsqt/oXJUtcibHih637bezVE3SM5Nx0+DmFljT6CvfAgfVw+WyV5NF+lM2JZqYIZHXb1EK/JyYtin/BOatYBV0CwzXGv2tPKpnPNXa6Xx68pl3YGb/6B4qOspH9aCYNtKzHDCyg+xYRRDvGl9cGa70XUzWP1JqstjEkQqw6akoA1/7x2Q5npXQPKFzk9yqZ1Kzrqp5WISQNvzr84qjEOEYHb91iSuWtMyvtcOcPyArYHTkJmmmDsnzVJ1/KoUOBIoBGRwBZ1jqlShl+LN92QzOjJVSdzIO0IXm63hgvUUkbUTM1Xcfe5W8YksK9GYMoeJA9y7SYbO1cnIfLzmkQs2hU0P9Ykmgm8S7kyt5bDdR01+qtUTLSgufBIJhuvVLJ3AK2bY8tkr0+Rw+7YKuA1OCf1bYYtonjLYXhe5nLyda uIcUHbDI WvthFAtlXZZqWytVJpPxv7aeKr6Dw+ahbHcg0oAzU6q3CkaRDlyGawbhwLyYddOvCjYieRgi+f5F+EZk7Rth9yudMXfotp4KEJ73ni8URcb2oIncjbnjj5NMWbt6NFRgfa3pUGZi6X1R5ufM246V6ZFDj/3RBbJYPtrrbLObJNi5jCGAHBINVpQDOQw== 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: List-Subscribe: List-Unsubscribe: On Tue, Dec 12, 2023, at 5:44 AM, David Hildenbrand wrote: > [...] > >> + >> +/** >> + * struct advisor_ctx - metadata for KSM advisor >> + * @start_scan: start time of the current scan >> + * @scan_time: scan time of previous scan >> + * @change: change in percent to pages_to_scan parameter >> + * @cpu_time: cpu time consumed by the ksmd thread in the previous scan >> + */ >> +struct advisor_ctx { >> + ktime_t start_scan; >> + unsigned long scan_time; >> + unsigned long change; >> + unsigned long long cpu_time; >> +}; >> +static struct advisor_ctx advisor_ctx; >> + >> +/* Define different advisor's */ >> +enum ksm_advisor_type { >> + KSM_ADVISOR_NONE, >> + KSM_ADVISOR_SCAN_TIME, >> +}; >> +static enum ksm_advisor_type ksm_advisor; >> + >> +static void init_advisor(void) >> +{ >> + advisor_ctx = (const struct advisor_ctx){ 0 }; >> +} > > Again, you can drop this completely. The static values are already > initialized to 0. > It is needed for patch 2, I folded it into set_advisor_defaults > Or is there any reason to initialize to 0 explicitly? > >> + >> +static void set_advisor_defaults(void) >> +{ >> + if (ksm_advisor == KSM_ADVISOR_NONE) >> + ksm_thread_pages_to_scan = DEFAULT_PAGES_TO_SCAN; >> + else if (ksm_advisor == KSM_ADVISOR_SCAN_TIME) >> + ksm_thread_pages_to_scan = ksm_advisor_min_pages; >> +} > > That function is unused? > I think you already saw it, it is used in patch 2, moving the function to patch 2. >> + >> +static inline void advisor_start_scan(void) >> +{ >> + if (ksm_advisor == KSM_ADVISOR_SCAN_TIME) >> + advisor_ctx.start_scan = ktime_get(); >> +} >> + >> +static inline s64 advisor_stop_scan(void) >> +{ >> + return ktime_ms_delta(ktime_get(), advisor_ctx.start_scan); >> +} > > Just inline that into the caller. Then rename run_advisor() into > advisor_stop_scan(). So in scan_get_next_rmap_item)( you have paired > start+stop hooks. > The next version has this change. >> + >> +/* >> + * Use previous scan time if available, otherwise use current scan time as an >> + * approximation for the previous scan time. >> + */ >> +static inline unsigned long prev_scan_time(struct advisor_ctx *ctx, >> + unsigned long scan_time) >> +{ >> + return ctx->scan_time ? ctx->scan_time : scan_time; >> +} >> + >> +/* Calculate exponential weighted moving average */ >> +static unsigned long ewma(unsigned long prev, unsigned long curr) >> +{ >> + return ((100 - EWMA_WEIGHT) * prev + EWMA_WEIGHT * curr) / 100; >> +} >> + >> +/* >> + * The scan time advisor is based on the current scan rate and the target >> + * scan rate. >> + * >> + * new_pages_to_scan = pages_to_scan * (scan_time / target_scan_time) >> + * >> + * To avoid pertubations it calculates a change factor of previous changes. > > s/pertubations/perturbations/ Fixed. > > Do you also want to describe how min/max CPU comes into play? > I added additional documentation for it in the next version of the patch. >> + * A new change factor is calculated for each iteration and it uses an >> + * exponentially weighted moving average. The new pages_to_scan value is >> + * multiplied with that change factor: >> + * >> + * new_pages_to_scan *= change facor >> + * >> + * In addition the new pages_to_scan value is capped by the max and min >> + * limits. >> + */ > > > > With that, LGTM > > Acked-by: David Hildenbrand > > -- > Cheers, > > David / dhildenb