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 B3E78C47073 for ; Thu, 4 Jan 2024 08:48:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3A3478D0070; Thu, 4 Jan 2024 03:48:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 354448D006F; Thu, 4 Jan 2024 03:48:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1F32B8D0070; Thu, 4 Jan 2024 03:48:41 -0500 (EST) 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 0E1968D006F for ; Thu, 4 Jan 2024 03:48:41 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id C7E1840830 for ; Thu, 4 Jan 2024 08:48:40 +0000 (UTC) X-FDA: 81641002800.22.C41E7DC Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf20.hostedemail.com (Postfix) with ESMTP id 855FD1C0016 for ; Thu, 4 Jan 2024 08:48:38 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=aDlWzz6A; dkim=pass header.d=suse.com header.s=susede1 header.b=aDlWzz6A; spf=pass (imf20.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704358119; 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=wm5QlyvcznWtplNRx9gonr2CCF8FxZMqNW82GS8OYto=; b=lrn5Wk/pMMKybICrmBpZbPHSEEbgdNJX7qapmsb2OTXqdyTuYsx0AVyJq6f3hDwvtP4yKv WLHChJTj8hLVQhghQBdUXFzrjWanCsYQEwluWB5zqgv548kpKjmna4DMGfXoaTH6zrs36N QnF2kfg72rxrX7U8TChg4HYpkNWas/I= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704358119; a=rsa-sha256; cv=none; b=Ly/nMXHNbj5iBq7XLSL7zx7n8WL+NQrzd4+SOo7bFpDWxYLVASi23z2pE+SiP2mLH565XU bcKgfl8UBRfiEjDfVp7oNpXuX7zTN6+80qygRu5JqwoBDUkQokURAdUDDDmd7vyaC+Du2A yjRjzNgk24SnrUMUfK8xzAH9nx/2FP0= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=aDlWzz6A; dkim=pass header.d=suse.com header.s=susede1 header.b=aDlWzz6A; spf=pass (imf20.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com 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 D6DD921F8C; Thu, 4 Jan 2024 08:48:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704358116; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=wm5QlyvcznWtplNRx9gonr2CCF8FxZMqNW82GS8OYto=; b=aDlWzz6AYUd//0CioCBvgVBGSt/KNDNfv0oz/0keYnwTq6sq/56CVspGPYopmhtFVYNAIW S0ImbtpkXuoi5Na/YS7SZh2HsXoTwS1cwS/yFJkFmwGhFDakbGuKAaB5ZQ4fK50rtnu9xR zwgDUk3KEp1dpVTkVVw+a3Y0TqPMrVM= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1704358116; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=wm5QlyvcznWtplNRx9gonr2CCF8FxZMqNW82GS8OYto=; b=aDlWzz6AYUd//0CioCBvgVBGSt/KNDNfv0oz/0keYnwTq6sq/56CVspGPYopmhtFVYNAIW S0ImbtpkXuoi5Na/YS7SZh2HsXoTwS1cwS/yFJkFmwGhFDakbGuKAaB5ZQ4fK50rtnu9xR zwgDUk3KEp1dpVTkVVw+a3Y0TqPMrVM= 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 AC7E213722; Thu, 4 Jan 2024 08:48:36 +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 5m3gJ+RwlmXoVAAAD6G6ig (envelope-from ); Thu, 04 Jan 2024 08:48:36 +0000 Date: Thu, 4 Jan 2024 09:48:36 +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=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 855FD1C0016 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: 61zz1pkdqgwoffejt7jz71gxdtibaqh1 X-HE-Tag: 1704358118-222186 X-HE-Meta: U2FsdGVkX1+LN3zoQaJ0GvM3yDhBoaARH3EAh0t6H/+kG5KpsplJsDQTiwbWlzRkij1EUhXVofQZ9igbytp7ZmxO8Kn64Cjt8rD2LEdx8ab2UJ6NLkmUYMsrRLmCBvPD3kvnNj7q6htV8mrYZ5FLcTlns001Dbwyhy3oa44j+MUaek/Y4heCCVYIJKqKBbISc1/VaIZL9kcf1mVn7exrcOIQuNJ+LV7ZEum62Ee2+JCImqsbYzEmeFBgVrDQMLz9rQwY+fLdQEcEe+VMHSXZLqOtlTdxfiD6PjJ9ldCEsz3QyNt44m8ofOsw4RTTU7jrWndX8t2o41Hmb+BBLgcgJUAE4L7rEwO1Mh1Y13x4RteXGBLzwinCAx+f6ukemv42O/AvWwnm1h3PbKQn0Ys2N5pNfihLFGrXmWSD6Ube6jJ2S8GJCEA1MDYwMq56+GRtkbHoOdvrlFX9daEuKMPzGWsuyyPcdM28H9+FebfoMhZfsiO0+LKhzPFc4a67tL6LQED4TTJhyfozo3gTdmAuoYCxq+TT76IhX9eGzMZKNLaikRd7MbRpr94z71GrOwnT1dIaoKftcnpW9rwsrvH2zGwdH3mZlBi6Ox5UIzkmcIKOZmFIWcpOQw1KQfcfGffcnCZo098L05Y2Rrt83RggfeU5T+qPmA4uyGX6gJ6ezsZU1l0BCEL6E6WMZnmiPWkzz/jcss76fD3wJqhTzI/xwOGidwR/X9M9nr1crj2YQSlTA3UVHUSBw7pqZ1MOffRw2sp2wL3Q72cZr9YmMFbENkk3sbqIgNAn1ZZ2wKT44zvtxJcRzH2QWX5WuI+KHcCB+0Uj0fzjjjmKaWYgb4F0jLz9fIR+5zCWTVq1muvPyDd7buUjrw1sKHYUCH86/OfqzSfOB7+GQdRZhexzDu7Ue5kqpMCsoxAX/w+XBEb+yDak6dJFtSjr6FyorHoMiLLm3806DVRvhx+0I45hhTU q1g39bCe 4g3xjXEuYW88S3A1P4FM43xozHUQrp8l9mXgp5OEJKuDWLV4lAOuzYtiTc9VgFHy+bSnn0oxLR+laOo8bqV0r9H091CJP0HPzFMm2HCcNkQZRjwNcM4AYL1AsOx0ujcbJZA+NCx8kHnMVuJUg1zvl0UGiX6Cs7A6rGSkRX17uqEJxT56+nNZaQzmqaNXfUrLNzQ9uhIK6RozrAxM9UJO5UtwYghNYtyTtkAv9nF1/nAU2aY55W6MbMWQ82PZZNm2IlQGSFiiMYT6zJKSIJA3rTBdVkMfLyMqCb9D8FW7gwLtAP/eLNbJbTzdjJOvqKxFxSiwwn9NVh3/PIRo6VlU51XP/xw== 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 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. The main argument for NULL check based approach is that it is less error prone from an incorrect ussage because any bug becomes obvious. If we use any other special constant a missing initialization would be much harder to spot because they would be subtle behavior change. Are there really any strong arguments to go against this "default initialization is safe" policy? -- Michal Hocko SUSE Labs