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 C57A9C6FA8E for ; Sun, 26 Feb 2023 04:38:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5E2D66B0072; Sat, 25 Feb 2023 23:38:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 593F66B0073; Sat, 25 Feb 2023 23:38:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 45A786B0074; Sat, 25 Feb 2023 23:38:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 376AC6B0072 for ; Sat, 25 Feb 2023 23:38:30 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 044751A0552 for ; Sun, 26 Feb 2023 04:38:29 +0000 (UTC) X-FDA: 80508186780.23.27C73D5 Received: from mail-pl1-f169.google.com (mail-pl1-f169.google.com [209.85.214.169]) by imf22.hostedemail.com (Postfix) with ESMTP id 0766EC000A for ; Sun, 26 Feb 2023 04:38:27 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=Tx1cHnfV; spf=pass (imf22.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.169 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677386308; 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=4qp/JVKTUEDkrRkz2e6HuPmWd8JQCtHI7nSlAH64P2U=; b=4rLycsMsVr8F9Z/IOj4ohazJc6mNweyAWX9gEZW23cgQkHDVTpI9kXxaIHMwR21RfosouW LyvI6GD1gTRa6nuzLnrER0LDihT+9fuOSQtwj9a2Vry5DPamc8ZhxezTDYNylFv5WfHAqN I7qHAUm2/QxAOiXihL3Y3ER5xN+jP1Y= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=Tx1cHnfV; spf=pass (imf22.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.169 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677386308; a=rsa-sha256; cv=none; b=JRBfWkz+KlUcupZV67F6A7CNFmyROwNx2Z3lTTj0YYIchtJ5tjJNVJ1TCmqW+F3Z54UkkG LWyfwE3BKagZHS3pxw4iqXozNczUv0fGiw4AUutc22MsPHjL6xZK+qTRFYc12y9Vx19HE1 l+LgN8ktptQ4L8yXc5Ue76R39Lh5PRs= Received: by mail-pl1-f169.google.com with SMTP id i3so3555047plg.6 for ; Sat, 25 Feb 2023 20:38:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=4qp/JVKTUEDkrRkz2e6HuPmWd8JQCtHI7nSlAH64P2U=; b=Tx1cHnfVu4O2docewlcqcozrPd8Rksf7DfjTqU+8z87kjaGnCK3/RhR2w8Ao4kzHj0 3sMD5Rcq/WkZ5ssDbTF/WclFb4QVMJDy9FnjAFcKb0mF6Ein1qN91x7Z/ZO8ky9KUPz5 bTFYQ8jH55sHXhSFKAQQvZVRujfQO+MeR/oFA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=4qp/JVKTUEDkrRkz2e6HuPmWd8JQCtHI7nSlAH64P2U=; b=PkqV0xhutADLM/wVU+OkHbkcMX/HFLTtPFpwCIj7dL7vFanA/7ziWbMdCo9ClqTtUh J0B6DFi78Op8JgmEgbhdVcbYDLzW/Doeci0St6a0JLJuEbikJr/U72Z43WQ4zSj9FDaC Qr3W33ey8EJh0PZQuJR1ugyl5v1SUbjIRLA71EFnIOWSoPmWpSqC1rKYgZ87H8cO2NIq BL/JJwsBXKgymx+wUa++Fuezxs6jDHJxA8wm0z/PBJecqkOC8jSg4fxSf7+K/uQPxwbD BYQZYdpCes50zLQpL1SlIOwHPkBz4VL01ZtNRDucaGueqhhNRGWytsjNdpj1PF5WGOca 8XdQ== X-Gm-Message-State: AO0yUKUPSj50yFxsTCtsEkjcrYiQQ1wzUYJI4UENJLKqBOrBnBqtwVLS ngMyAtzxGE/mYOYLtRiz+CaPpg== X-Google-Smtp-Source: AK7set/5ihRAeyeA3EcM7EfrNK8nILQK3d5w4fmIBzV6ILpXzEurTSdM7F4qBwcWXw/D50neO/j/qA== X-Received: by 2002:a17:902:f690:b0:19a:80e6:2a1d with SMTP id l16-20020a170902f69000b0019a80e62a1dmr24743532plg.41.1677386306801; Sat, 25 Feb 2023 20:38:26 -0800 (PST) Received: from google.com (KD124209188001.ppp-bb.dion.ne.jp. [124.209.188.1]) by smtp.gmail.com with ESMTPSA id k3-20020a170902ba8300b0019a6f32e6c1sm1988140pls.148.2023.02.25.20.38.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Feb 2023 20:38:26 -0800 (PST) Date: Sun, 26 Feb 2023 13:38:22 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Sergey Senozhatsky , Andrew Morton , Yosry Ahmed , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCHv2 3/6] zsmalloc: fine-grained inuse ratio based fullness grouping Message-ID: References: <20230223030451.543162-1-senozhatsky@chromium.org> <20230223030451.543162-4-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 0766EC000A X-Stat-Signature: aura4ncgr5kjcznw3o7qw4kk4yo1wpw3 X-HE-Tag: 1677386307-862614 X-HE-Meta: U2FsdGVkX19CxdOLjGH1lj0BoDdhzUHTJjjhJfwQpcU08CclwD5gELbjZwcnydNpDVyu0uSj+O894KMQI3/JDcyw8yvT5/uqq/oSvuEKUa7zvTdiTjrH+rh5RtlNE+gPWqBg6rTcKMG9x/2H4luqBkDktKppjd/7cTlHv1PlIa0LZiUbTLZASxMCxchjdvIEZyJtritHgsNWu+faLQvXLcUxWcCWbo8RL7PsZe7O+xa+ROEyL4hI0KjSe1ZJenVng7vQkMhxu0K4zvmP6IZjMRQd92tv7Nkd9ty8EBDb3WBHwZLljFnEOZcOJxL26Vf7mQLwgw2Rq01nxNhJb5MR/kKZl67VrzXuXFV1tCKGgL9RETZYBUdt198PbyHLWWGIcato/FR6YDYGiXRce11aSGVsX8ZL2SeSaDGg/b50w7YiFIs62gnQzaZjlO37mcJqC8q7SCALjYj2Ur3Ee1+cmjsWEVoULSkTyImk33E1VyAHqsFHndxcr/dGqRifapmIuDTf16erR5TQSY7Sw97syPA/lKRBcftq8KmhZ6QRON8CSekaPNefRkn5SwmoDABLnRate7YRph7f2SMdIZ0jnlGj743HEzq8sJ6SFVT3AQOEaRHLxxeQiiBdRj2h742NdYokXzO8GCm1IZC8mTLYC4KMjE3vru837PPNh2LGgY4tArwwQt4bnfmZowfBVBcePuDbXdAiAvOIBpDxir7rpVcML/aM3bbm/Nh30FYgLF1oVKoj8gt4cAUHTkALQjssGi/aqZjjyMgnrEdrzhR2VGKNHrscy3TtSGAoyPMGGbQZoHvdvqm7ICTNnURedoofLekXMkM2dQ7JqULiWrhHk+DoH+SVjhh9ODquU/Kh4IvmcqAeO+VMPmCHzKnnRsSNAZb+ULhIGtWnC7VSC25t+7tsXD2Rv/8VDrZ3uXPM+XCm2F0wcDSBZlHw994/IcOr1oDuLqd+/1GCLfSzZnJ hQJQZbe7 Bn+y5DzUgzD808Xr26BEAElUvbf+rbAyrZP1+Y7JJCRvcl3T7dYEIiFi9MFGIdj3z/1T2Zhc2aQqMS+P5ybLkbu8jS0OWY64V/U5YoiimB1gbfE8uhnt3dJTkOX9acXrJ4yWf3bktqFDCuzQ7N7b+ESU8B3+anDsSJkQ+WGqhD4HlZje8OnWawHxi63d4nkCdRKw5KWfUWhmARrmH1IzCcNslyu65MUULoVYapLMjJoR7EJ/qCfUzIdQnZoIq0vHGnMYa4bfXDIvyxVcUss2xgx1DVa9Q4W4//U0EZ3M+rgqlDh/MISWuBocVSTS4YKCRJ5q3bI1B7F/7gGM= X-Bogosity: Ham, tests=bogofilter, spamicity=0.009774, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On (23/02/23 15:27), Minchan Kim wrote: > > + * Pages are distinguished by the ratio of used memory (that is the ratio > > + * of ->inuse objects to all objects that page can store). For example, > > + * INUSE_RATIO_30 means that the ratio of used objects is > 20% and <= 30%. > > + * > > + * The number of fullness groups is not random. It allows us to keep > > + * diffeence between the least busy page in the group (minimum permitted > > + * number of ->inuse objects) and the most busy page (maximum permitted > > + * number of ->inuse objects) at a reasonable value. > > + */ > > +#define ZS_INUSE_RATIO_0 0 > > How about keeping ZS_EMPTY and ZS_FULL since they are used > multiple places in source code? It would have less churning. I have to admit that I sort of like the unified naming "zspage inuse ratio goes from 0 to 100" but I can keep ZS_EMPTY / ZS_FULL as two "special" inuse values. > > +#define ZS_INUSE_RATIO_10 1 > > +#define ZS_INUSE_RATIO_20 2 > > +#define ZS_INUSE_RATIO_30 3 > > +#define ZS_INUSE_RATIO_40 4 > > +#define ZS_INUSE_RATIO_50 5 > > +#define ZS_INUSE_RATIO_60 6 > > +#define ZS_INUSE_RATIO_70 7 > > +#define ZS_INUSE_RATIO_80 8 > > +#define ZS_INUSE_RATIO_90 9 > > +#define ZS_INUSE_RATIO_99 10 > > Do we really need all the define macro for the range from 10 to 99? > Can't we do this? > > enum class_stat_type { > ZS_EMPTY, > /* > * There are fullness buckets between 10% - 99%. > */ > ZS_FULL = 11 > NR_ZS_FULLNESS, > ZS_OBJ_ALLOCATED = NR_ZS_FULLNESS, > ZS_OBJ_USED, > NR_ZS_STAT, > } This creates undocumented secret constats, which are being heavily used (zspage fullness values, indeces in fullness lists arrays, stats array offsets, etc.) but have no trace in the code. And this also forces us to use magic number in the code. So should fullness grouping change, things like, for instance, zs_stat_get(7), will compile just fine yet will do something very different and we will have someone to spot the regression. So yes, it's 10 lines of defines, it's not even 10 lines of code, but 1) it is documentation, we keep constats documented 2) more importantly, it protects us from regressions and bugs >From maintinability point of view, having everything excpliticly documented / spelled out is a win. As of why I decided to go with defines, this is because zspage fullness values and class stats are two conceptually different things, they don't really fit in one single enum, unless enum's name is "zs_constants". What do you think? [..] > > * Size of objects stored in this class. Must be multiple > > * of ZS_ALIGN. > > @@ -641,8 +644,23 @@ static int zs_stats_size_show(struct seq_file *s, void *v) > > continue; > > > > spin_lock(&pool->lock); > > - class_almost_full = zs_stat_get(class, ZS_ALMOST_FULL); > > - class_almost_empty = zs_stat_get(class, ZS_ALMOST_EMPTY); > > + > > + /* > > + * Replecate old behaviour for almost_full and almost_empty > > + * stats. > > + */ > > + class_almost_full = zs_stat_get(class, ZS_INUSE_RATIO_99); > > + class_almost_full += zs_stat_get(class, ZS_INUSE_RATIO_90); > > + class_almost_full += zs_stat_get(class, ZS_INUSE_RATIO_80); > > + class_almost_full += zs_stat_get(class, ZS_INUSE_RATIO_70); > > > + > > + class_almost_empty = zs_stat_get(class, ZS_INUSE_RATIO_60); > > + class_almost_empty += zs_stat_get(class, ZS_INUSE_RATIO_50); > > + class_almost_empty += zs_stat_get(class, ZS_INUSE_RATIO_40); > > + class_almost_empty += zs_stat_get(class, ZS_INUSE_RATIO_30); > > + class_almost_empty += zs_stat_get(class, ZS_INUSE_RATIO_20); > > + class_almost_empty += zs_stat_get(class, ZS_INUSE_RATIO_10); > > I guess you can use just loop here from 1 to 6 > > And then from 7 to 10 for class_almost_full. I can change it to for (r = ZS_INUSE_RATIO_10; r <= ZS_INUSE_RATIO_70; r++) and for (r = ZS_INUSE_RATIO_80; r <= ZS_INUSE_RATIO_99; r++) which would be safer than using hard-coded numbers. Shall we actually instead report per inuse ratio stats instead? I sort of don't see too many reasons to keep that below/above 3/4 thing.