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 571ABC369A2 for ; Thu, 10 Apr 2025 02:09:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2E4F92800C2; Wed, 9 Apr 2025 22:09:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 294D82800C0; Wed, 9 Apr 2025 22:09:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1361F2800C2; Wed, 9 Apr 2025 22:09:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id E561F2800C0 for ; Wed, 9 Apr 2025 22:09:40 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 4BC35BBC24 for ; Thu, 10 Apr 2025 02:09:42 +0000 (UTC) X-FDA: 83316503004.22.8DC4110 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf28.hostedemail.com (Postfix) with ESMTP id AAE91C0003 for ; Thu, 10 Apr 2025 02:09:40 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b="f/bxq3xo"; dmarc=none; spf=pass (imf28.hostedemail.com: domain of akpm@linux-foundation.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744250980; 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=um5mre/mIaRTmuRSoCrM5ITMPdk6fTkltt1TLrdd8D0=; b=B3pg2r0y5Irfpet5tRJuPpX2hknDyH4xXxVGsT1naNk1n2jj2JadxrCDILP77qxqKPby5P If71uPesalmXvZQVpfa46t+Jc699f3U3PTYg2uS7MtQCLgMjdvY6KpXHmOZh3Q3q1/ln5c 0n6xUyGC5Rp5LSxno5m04qq9nbrICsI= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b="f/bxq3xo"; dmarc=none; spf=pass (imf28.hostedemail.com: domain of akpm@linux-foundation.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744250980; a=rsa-sha256; cv=none; b=R7supXRNTDL3eoAxCzxTNKj50oLlCL6F3/RDDYmFTuCHjstTrnTCOZ+xAq1etmMCMuHl2v /CSA68WieIaGA/mgf1roZp4HPNrEpqal63KDi+8kR2CjszEX4Dd0uvGezNUW4zxG2/yUcl VkhJaz042Ou/f8qu9tsr9Urc4EdJD6s= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 68FDBA4801C; Thu, 10 Apr 2025 02:04:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 75E0BC4CEE2; Thu, 10 Apr 2025 02:09:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1744250979; bh=5WAZIPN/EyEP4Yvj8qfVcq+vQ3y8dvMh/Wpf8md2JD8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=f/bxq3xooKH2KD17rgbjqplyz2lCOlegYIHe9RY2N2BGCV7XFuLEzpe3+FfCcX3OQ xRlzKOGOFCozENPuywXyNF2Ra+MXFezgzsV43XoYUD9+N2mZVRfJQPs4D+7gOEMcC+ 9LQbbsX98fjRqRNus5xL5k3zNkrvMqYifdr7WT64= Date: Wed, 9 Apr 2025 19:09:38 -0700 From: Andrew Morton To: Zhongkun He Cc: hannes@cmpxchg.org, mhocko@suse.com, yosry.ahmed@linux.dev, muchun.song@linux.dev, yuzhao@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH V3 2/3] mm: add max swappiness arg to lru_gen for anonymous memory only Message-Id: <20250409190938.f6befeeb9e86ad72f46503a5@linux-foundation.org> In-Reply-To: References: X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: AAE91C0003 X-Stat-Signature: nb5pg654su4oekyussi6bymt8ytt49zh X-Rspam-User: X-HE-Tag: 1744250980-840713 X-HE-Meta: U2FsdGVkX18TvCYp5SLWCUucrFX0+lMvj7KCyO8yJNseQFdQ6Hl6n2Ut4Obg23/7tS1luzyk59RwmVHtmsraIYwUt7p0atVFe+U2FgFKiGv0OmizhGntvHNu44DV1qq9WqYFLoPC7yVtV7C0kT7KoyqbhJpu2445NLPLeWS49e2YyyBElXVRN3id0jk0vJkZyvWubDpXfveQSLCvwgvNbowMrbp1Exj1wy0DfUCSYOfnmR1Tl7dC+ffejwEzheJubK2gAMf2p8nEID5cQ+109vVBcF4374p5cIvtvgwMDJwQNx4bM9gCENJNMREwI8ZNEtbErnm4IP9sAuI0A7/hVBRGnwJ7nXR9O6O+M4nUCxjxoik8UAtNZiDLf6eUFFaAauLs71LJVj+Fz26tcEcl1sfwmDZ7x+LDYC9PPSAVa/HyYognDmr6kQftppnz0zZ8o8QKfLJ0BPN3RQ7/ie+Wd+1c4iZTFHEwGdkq8A5wIcvYXd+PUoYHYwwgdd1SSr3BiwLXebl2gPQPY3HBjJhHtgLUBFB4T0kE9N312DbVZ7JWAZ6CGNYlNDwUpciTMOkqHm3euYkkRBXy+GMh6quYPwDrJC67yeACBIG1BN6u1eGR31sC+YRgaUwuKAtSNGF3F7odYd7zBL9wDUqa5KjwarpTeqIj2WrPHnxxsr9KgX0mYzDSTcFw9eJYYZOk2SEIAiTsDP71BpThpozZkUcl/j1zI3BrdOl9hFj2nUr6dEQLPm6TcgHrXBf8AstMpC4ElQhrnFCUkS3h+YFd9C9cTWl31D+7pjszvzpB0LxrqtpRaFOg1q7/X9ldH1A+/io+M3D3p36bxOqoeCamThzu6Ac8saEqlA3bv2SFHosM9DWXX/uNO7uWaxCC56wAT0Uzj3/QY+4QtJ1XnjRJ8y53zBu7eD21Dmh2BhB/NTekT+CLSy5fL3jIHmwb7CUehYuN43iyvSsrXyNQK3Ups4/ 2xLoA2PU kUE7cicCrI0QDfqTNfHVmgBtJS2D86NaFoNMpNLibGXatwEhAKy+/QibSDwf7LYBClrVqTkIV7pNtfRnS2SoPcDvAvNtge6FVUXES1orR+5Qi2WhxVgNyTh7v+2nR81UpQben11L+n3NsvcF10aQDsO2aC+wG5/DztKP1xkzV5QeR4ywuCyNNSUOS8XlRiwRPhWAcJX52DLO0YV6XaQ0fpTe6z0A1KTrteSkw3rNipsAa03SRyQ6geAihZxyoDIPmgeHzhyoNoO0Qo2Veq+oUc+pawpV5PXfZp8WRINVsrtugOBG1HNzenUAaHjQ6T3nm499/we/3gvb6VR73BfNLr+u1HSxHZqg8hogO 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 Wed, 9 Apr 2025 15:06:19 +0800 Zhongkun He wrote: > The MGLRU paging yuzhao? > already supports reclaiming only from anonymous memory > via the /sys/kernel/debug/lru_gen interface. Now, memory.reclaim > also supports the swappiness=max parameter to enable reclaiming > solely from anonymous memory. To unify the semantics of proactive > reclaiming from anonymous folios, the max parameter is introduced. > > Additionally, the use of SWAPPINESS_ANON_ONLY in place of > 'MAX_SWAPPINESS + 1' improves code clarity and makes the intention > more explicit. > > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -2697,8 +2697,11 @@ static bool should_clear_pmd_young(void) > READ_ONCE((lruvec)->lrugen.min_seq[LRU_GEN_FILE]), \ > } > > +#define max_evictable_type(swappiness) \ > + ((swappiness) != SWAPPINESS_ANON_ONLY) > + > #define evictable_min_seq(min_seq, swappiness) \ > - min((min_seq)[!(swappiness)], (min_seq)[(swappiness) <= MAX_SWAPPINESS]) > + min((min_seq)[!(swappiness)], (min_seq)[max_evictable_type(swappiness)]) Why oh why did we implement these in cpp? > > @@ -3857,7 +3860,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness) > int hist = lru_hist_from_seq(lrugen->min_seq[type]); > int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]); > > - if (type ? swappiness > MAX_SWAPPINESS : !swappiness) > + if (type ? (swappiness == SWAPPINESS_ANON_ONLY) : !swappiness) This expression makes my brain bleed. if (type) { if (swappiness == SWAPPINESS_ANON_ONLY) { /* * Nice comment explaining why we're doing this */ goto done;; } } else { if (!swappiness) { /* * Nice comment explaining why we're doing this */ goto done; } } or if (type && (swappiness == SWAPPINESS_ANON_ONLY)) { /* * Nice comment explaining why we're doing this */ goto done; } if (!type && !swappiness) { /* * Nice comment explaining why we're doing this */ goto done; } It's much more verbose, but it has the huge advantage that it creates locations where we can add comments which tell readers what's going on. Which is pretty important, no? > goto done; > > /* prevent cold/hot inversion if the type is evictable */ > @@ -5523,7 +5526,7 @@ static int run_cmd(char cmd, int memcg_id, int nid, unsigned long seq, > > if (swappiness < MIN_SWAPPINESS) > swappiness = get_swappiness(lruvec, sc); > - else if (swappiness > MAX_SWAPPINESS + 1) > + else if (swappiness > SWAPPINESS_ANON_ONLY) > goto done; > > switch (cmd) { > @@ -5580,7 +5583,7 @@ static ssize_t lru_gen_seq_write(struct file *file, const char __user *src, > while ((cur = strsep(&next, ",;\n"))) { > int n; > int end; > - char cmd; > + char cmd, swap_string[5]; > unsigned int memcg_id; > unsigned int nid; > unsigned long seq; > @@ -5591,13 +5594,22 @@ static ssize_t lru_gen_seq_write(struct file *file, const char __user *src, > if (!*cur) > continue; > > - n = sscanf(cur, "%c %u %u %lu %n %u %n %lu %n", &cmd, &memcg_id, &nid, > - &seq, &end, &swappiness, &end, &opt, &end); > + n = sscanf(cur, "%c %u %u %lu %n %4s %n %lu %n", &cmd, &memcg_id, &nid, > + &seq, &end, swap_string, &end, &opt, &end); Permits userspace to easily overrun swap_string[]. OK, it's root-only, but still, why permit this? > if (n < 4 || cur[end]) { > err = -EINVAL; > break; > } > > + /* set by userspace for anonymous memory only */ > + if (!strncmp("max", swap_string, sizeof("max"))) { Can sscanf() give us a non null-terminated string? > + swappiness = SWAPPINESS_ANON_ONLY; > + } else { > + err = kstrtouint(swap_string, 0, &swappiness); > + if (err) > + break; > + } > + > err = run_cmd(cmd, memcg_id, nid, seq, &sc, swappiness, opt); > if (err) > break;