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 D7329C3DA6E for ; Wed, 10 Jan 2024 10:33:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 56C3B6B0087; Wed, 10 Jan 2024 05:33:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 51B786B008A; Wed, 10 Jan 2024 05:33:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3BC536B008C; Wed, 10 Jan 2024 05:33:04 -0500 (EST) 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 2A74F6B0087 for ; Wed, 10 Jan 2024 05:33:04 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E14A0C03D1 for ; Wed, 10 Jan 2024 10:33:03 +0000 (UTC) X-FDA: 81663038646.08.EAF959F Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf28.hostedemail.com (Postfix) with ESMTP id A3436C0023 for ; Wed, 10 Jan 2024 10:33:01 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b="ckOO/5A0"; dkim=pass header.d=suse.com header.s=susede1 header.b="ckOO/5A0"; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704882782; 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=BXR7PBJo/0k7RMlA2liuV9sILDH8IFvToNJCRDJt55E=; b=tIJ/YeErgScJsNr9hOXG9daWT/k80Tvn5fjI5weRXGubdwUu1wFibRBGCLJw2z5My8aH7y KMK60KlaaGUpd7oYONVwLSv5jDrSn26aYLm1Kv6ccagJqQhCmpkSJWU43DdEIjM9DFYgwW oePRpn/xnEmotDs4jc7nOHyylsoGNko= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b="ckOO/5A0"; dkim=pass header.d=suse.com header.s=susede1 header.b="ckOO/5A0"; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704882782; a=rsa-sha256; cv=none; b=q2Ubia+DCVMS2iA4aiyvoRLMymO9HUvKTC+MT4SQ41I0gV5f1fkrURdseH0CwNDQAH67hu hpNn1n5b/BaiEiOkU2dREnHQTq80LMNSIuyr3m3qh4h+exi9SVlRkh9QCXaYOwsdZcpdTc Y/UGJGyJTstblsiwbet9lGRjc47ehS4= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id AC8292206D; Wed, 10 Jan 2024 10:32:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704882779; 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=BXR7PBJo/0k7RMlA2liuV9sILDH8IFvToNJCRDJt55E=; b=ckOO/5A0SO7+ciUHsiXzgEYJhcMrLorPd+ohuWjdgh6e5IE3uzah0+OYHvQkFui0OujWpM R0pDPwN/bAwf06163wv+Wa9IkgzoOddCg0ZwgoP0PMVo0r1+1ULorGEhKCqZmbXZNHsN6j 7nq1V338IJfg07YIrCOh5FbK+7/omjc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704882779; 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=BXR7PBJo/0k7RMlA2liuV9sILDH8IFvToNJCRDJt55E=; b=ckOO/5A0SO7+ciUHsiXzgEYJhcMrLorPd+ohuWjdgh6e5IE3uzah0+OYHvQkFui0OujWpM R0pDPwN/bAwf06163wv+Wa9IkgzoOddCg0ZwgoP0PMVo0r1+1ULorGEhKCqZmbXZNHsN6j 7nq1V338IJfg07YIrCOh5FbK+7/omjc= Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 7FEB113CB3; Wed, 10 Jan 2024 10:32:59 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id TxhKHFtynmUkewAAD6G6ig (envelope-from ); Wed, 10 Jan 2024 10:32:59 +0000 Date: Wed, 10 Jan 2024 11:32:58 +0100 From: Michal Hocko To: Yu Zhao Cc: Dan Schatzberg , Andrew Morton , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org, Yosry Ahmed , David Rientjes , Chris Li , Tejun Heo , Zefan Li , Johannes Weiner , Jonathan Corbet , Roman Gushchin , Shakeel Butt , Muchun Song , David Hildenbrand , Matthew Wilcox , Kefeng Wang , Yue Zhao , Hugh Dickins Subject: Re: [PATCH v6 2/2] mm: add swapiness= arg to memory.reclaim Message-ID: References: <20240103164841.2800183-1-schatzberg.dan@gmail.com> <20240103164841.2800183-3-schatzberg.dan@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: A3436C0023 X-Stat-Signature: o9e3sqxoaz5e8ak6wqsa9cmjkm87rwio X-Rspam-User: X-HE-Tag: 1704882781-239462 X-HE-Meta: U2FsdGVkX18io2mJpp7EkWpIcv9pPRywMn2Y4G0mZMcFD6sNBXeo87O9xAFZ6+GxhxPsm5kRZlz3yperl5Z6zUY4Z/Pf+Qx864pwBFPCHvmFqKXSG2Rdt510XYfwzlEtKDZ5KrRk0dHbcqP1tXD8OpcKC07OdZVT+ELynlnllFbAGuQgz/TPgRM7RbLmWX8aVly9zyeGxABd69DV92nowkmQysoyYE2Ax0/78m734RIOoZXXv2EVLh1dlUBag1uxzQP9eE4iOflX54Fo8vpAecbe+4UQZHlLg12iHSyCmARTP0k0qWPT8Xt6ldB0H3AhGEyfM4rl6TTSx/X0PafO8zrjk4EGlz/a5N9V0bu2saiYQEF6b0MfPRDnw8zrhDTWRxqzlpAQkt6ChbTcIqvnCHYdU1C1k+fVKsL/m5lxBoWat8kGFh5yxs2i0TuCupFO4LByspdzxYFXDfb/rRhxoZG5tUG5aD9hDNe9DXU/X+QKhPwm2GdHC9vnUPzAtcGCl1OfHMB1Chydc3FiTPpbmDDWKbG9m0VVS7BHtuImyjIKtGxDKWYhqIOhbwwGi1mlB7dfkcHNETdiUPN4dVVBylyjkzcCcbjQJOQ/i2BUTF6ekdk/3dHZtmiTUZuOxTpdME7Qqqw+94AHkvRo2amtnYbcj8OKTFN25wAzl+/5PumoZTnxwbWZ/sArs+NwQ0lGtpcrADHgis1UJYb3drl0McU90ABkRD4i0fvKNDzTuk3ioOLAcQsY7vbof0n9QFgxJoqGChw9Erev7MN5fCaNn5oDoroy27R6QjkwvjGeoRFjEPIpwQmT1zFgCEce8TyeluEiCV8d1PcbIFMFsSyKIxVtLOR2QnaRducEiY0HTcjsHB0vWVQg3+hFmBqx50pCfspYi5LRitcqiPJmLDJFsDlUxhzRAzD/VCGfYsZBrWyP8LfNfMd7XnMUZmVacx+cMbtio7N65YeY0wZJT3y mCi5Fgjb rWsgSuCD3gSDr4nlOf1iYdzcOH9i2m8QzubUqXL0CJJEsvjIKP2qyFFoEa7fso2CGGUhAeAUWQuNHvavcld0AU/v3mnK30RJoMdSUaoXVrG4w3LEds1S4zv+cBjB4libFnlmrgIIkWbBxQX/OeybVN6BI1wBmaqtgVnq4tVB45S9pLQOwCxOS7YCu4R+1wxHzC+iKsspFgLBk0ZVVHNzddKzhcZcLCw9tI2HNaEehQ4A0ofrb2dpveK5P9uOzlbU1tEG12dTsyvLXv6mR9epRxnIZ942XDrjYuj4elRiKVZt2C6sYAtgCWJILiDZDX2ORSGo3tEeaGIJtke+XzJxZx3vGAHjW33+eTr744CC6NiGEVLMTweJnlspyu7uObB5RYtqXnzOI4CUhnsEO/6IauQN9lQ== 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 09-01-24 16:54:15, Yu Zhao wrote: > On Thu, Jan 4, 2024 at 1:48 AM Michal Hocko wrote: > > > > On Wed 03-01-24 18:07:43, Yu Zhao wrote: > > > On Wed, Jan 03, 2024 at 01:19:59PM -0500, Dan Schatzberg wrote: > > > > On Wed, Jan 03, 2024 at 10:19:40AM -0700, Yu Zhao wrote: > > > > [...] > > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > > > > index d91963e2d47f..394e0dd46b2e 100644 > > > > > > --- a/mm/vmscan.c > > > > > > +++ b/mm/vmscan.c > > > > > > @@ -92,6 +92,11 @@ struct scan_control { > > > > > > unsigned long anon_cost; > > > > > > unsigned long file_cost; > > > > > > > > > > > > +#ifdef CONFIG_MEMCG > > > > > > + /* Swappiness value for proactive reclaim. Always use sc_swappiness()! */ > > > > > > + int *proactive_swappiness; > > > > > > +#endif > > > > > > > > > > Why is proactive_swappiness still a pointer? The whole point of the > > > > > previous conversation is that sc->proactive can tell whether > > > > > sc->swappiness is valid or not, and that's less awkward than using a > > > > > pointer. > > > > > > > > It's the same reason as before - zero initialization ensures that the > > > > pointer is NULL which tells us if it's valid or not. Proactive reclaim > > > > might not set swappiness and you need to distinguish swappiness of 0 > > > > and not-set. See this discussion with Michal: > > > > > > > > https://lore.kernel.org/linux-mm/ZZUizpTWOt3gNeqR@tiehlicka/ > > > > > > static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf, > > > size_t nbytes, loff_t off) > > > { > > > struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of)); > > > unsigned int nr_retries = MAX_RECLAIM_RETRIES; > > > unsigned long nr_to_reclaim, nr_reclaimed = 0; > > > + int swappiness = -1; > > > ... > > > reclaimed = try_to_free_mem_cgroup_pages(memcg, > > > min(nr_to_reclaim - nr_reclaimed, SWAP_CLUSTER_MAX), > > > - GFP_KERNEL, reclaim_options); > > > + GFP_KERNEL, reclaim_options, > > > + swappiness); > > > > > > ... > > > > > > +static int sc_swappiness(struct scan_control *sc, struct mem_cgroup *memcg) > > > +{ > > > + return sc->proactive && sc->proactive_swappiness > -1 ? > > > + sc->proactive_swappiness : mem_cgroup_swappiness(memcg); > > > +} > > > > Tpo be completely honest I really fail to see why this is such a hot > > discussion point. To be completely clear both approaches are feasible. > > Feasible but not equal. > > > The main argument for NULL check based approach is that it is less error > > prone from an incorrect ussage because any bug becomes obvious. > > Any bug becomes *fatal*, and fatal isn't only obvious but also hurts > in production systems. > > This was the reason for going through the trouble switching from > VM_BUG_ON() to VM_WARN_ON() and documenting it in > Documentation/process/coding-style.rst: > > 22) Do not crash the kernel > --------------------------- > > In general, the decision to crash the kernel belongs to the user, rather > than to the kernel developer. > > Isn't? I do agree with this general statement but I do not think it is applicable in this context. This is not an explicit BUG() when kernel explicitly sets to panic the system. We are talking about subtle misbehavior which might be non-trivial to debug (there are other reasons to not swap at all) vs. a potential NULL ptr which will kill the userspace in a very obvious way. Sure there are risks with that but checks for potential NULL ptr dereferncing is easier than forgot explicit initialization. There are clear pros and cons for both approaches. NULL default initialized structures members which allow for behavior override are a general kernel pattern so I do not really see this going way off the rails. -- Michal Hocko SUSE Labs