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 E5C83C64EC7 for ; Tue, 28 Feb 2023 22:53:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C07B6B0074; Tue, 28 Feb 2023 17:53:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 871A36B0075; Tue, 28 Feb 2023 17:53:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7135A6B0078; Tue, 28 Feb 2023 17:53:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5C7896B0074 for ; Tue, 28 Feb 2023 17:53:59 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 1A55E1602AF for ; Tue, 28 Feb 2023 22:53:59 +0000 (UTC) X-FDA: 80518204998.27.6A85C8F Received: from mail-pg1-f169.google.com (mail-pg1-f169.google.com [209.85.215.169]) by imf07.hostedemail.com (Postfix) with ESMTP id 3D9D940005 for ; Tue, 28 Feb 2023 22:53:57 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=lVY47DTv; spf=pass (imf07.hostedemail.com: domain of minchan.kim@gmail.com designates 209.85.215.169 as permitted sender) smtp.mailfrom=minchan.kim@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677624837; h=from:from:sender: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=FmJBk+SbzeGsb6WDHWELtLn5ezNrmb5a5dd/nz1DZN8=; b=o3vI/NbHKDMDxddFmq/S79ejIgIEOhyzxUp4X3771R3wDVAq3Y1mFB/tcGsGjhexz7xVid wslDXUN0942dgPHBtvqFGQi79es8fTad7E9yy6v6EmYYjxyqicBc0SiojPXuBvg+YSprN8 XjIsm5KQi5Hx4lZNrnnGlSEN3ebHlC4= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=lVY47DTv; spf=pass (imf07.hostedemail.com: domain of minchan.kim@gmail.com designates 209.85.215.169 as permitted sender) smtp.mailfrom=minchan.kim@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677624837; a=rsa-sha256; cv=none; b=q9wFzTuYERgC5BSll0iiOP9BJNSnAk4mW4nTWvR4JprAJhF5iVTJzoUzCQ942LZm5S+5Ul 7z5GipRXPGAov41dXG1Hk5WVnw2/lhEFyhXHhVv5OK+0+v5x7j7Duplgu/gbwCumAn4BEk Qy1q6hCK8iOaSzfujwT7gt7sbfgQ6y4= Received: by mail-pg1-f169.google.com with SMTP id q189so6639029pga.9 for ; Tue, 28 Feb 2023 14:53:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1677624836; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=FmJBk+SbzeGsb6WDHWELtLn5ezNrmb5a5dd/nz1DZN8=; b=lVY47DTv23Y0ZhtqrSvSGHFUwL9XUFn/6RhnyXQvK02Zauv+OZuVLTbZrpB6MTzbCW QiCDcEoizVdXP4ltBDgZm7nlSdjHhc5sr2J8LK6qWTiznx1aGcw+3J2vyMbKGy0gssce ZTpcSgI+oCr1w/nAZE3l7J7bV6zZbZaCpxUzr62DArrBlNGBRr0iSRJOQxKLrlP50ftz v+x6TYw+RIZH4fbysoFhF96H7+YVN1teZwjOE+QMTIzj6gJU2EYjTCYHx7kenUu9F6Nl ue3toi9HBpL4XO1/0XG3B65FkAuvwIWZTV4+hFeyGX3IvqYnpjT44Ocwmvs3K/Z9dDih 5QWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677624836; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=FmJBk+SbzeGsb6WDHWELtLn5ezNrmb5a5dd/nz1DZN8=; b=DkcR0tOO3InH4gHmnx1TMyktVgsBdiC89phzVTas/g5pPgTintkY8jaEVl20WqsEYG TDZwzjEM2cq1tXTsRHdHPuIRGfMcSSGCJH1XiNLfIYtIwQYiDRVqHvkNRu1rxgmZeW/S VCQpf/b1kqzgpWirygn+tR1T2npU3Ks+fuFfA7XJB+GkRsiZ4G12p/FDzR86/guqOXS/ YGwa6XZxQB1HyGgre/fNmxHiXM2/z82yERc5gtqdusm4lzPrrH+/sZ2XFT9Ldka6FmPQ 2CSpOY48ywcsXTRqcULKgbhVR6ldjMoTeoBChUhWeRgkldU9+hpJ00TDWETzszk29Gxs P4RA== X-Gm-Message-State: AO0yUKXJ4kuCqjrtXEJfzneJZak75zShiAW29ZSuB3oVFWs//Ok+kkRL xriBRM47CFojr2zjOmgwKH1uJzwJvhU= X-Google-Smtp-Source: AK7set/jEROoXr9AYq0co/9oRqaBiZryOKRUFIeuUhLG9S5SuZlHGgdaODylKVxuZJ8qHKeNUJr7cg== X-Received: by 2002:a62:1989:0:b0:5aa:2d65:4733 with SMTP id 131-20020a621989000000b005aa2d654733mr3252721pfz.10.1677624835978; Tue, 28 Feb 2023 14:53:55 -0800 (PST) Received: from google.com ([2620:15c:211:201:639:82f5:b510:3494]) by smtp.gmail.com with ESMTPSA id e3-20020aa78243000000b005e5861932c9sm6528850pfn.129.2023.02.28.14.53.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 14:53:55 -0800 (PST) Date: Tue, 28 Feb 2023 14:53:53 -0800 From: Minchan Kim To: Sergey Senozhatsky Cc: 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-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 3D9D940005 X-Stat-Signature: 7y8hkrzsah49sfkp7i7jarkngqqxpcya X-Rspam-User: X-HE-Tag: 1677624837-567842 X-HE-Meta: U2FsdGVkX18/8iM0dmnz4wy+H1bvL/Ag97u58nXwjG8CcyVwjlHrsAbFOgVzRpto1c1Ie7xr99XPtb7GITZ2a2C+Ec98vvi7yKOPnjDSycHtldVNS542eGyENJf9xxVp9IAi7MtnDsv+ttZK7T4KMOrv5TdlbBQJkdyL65qXTfJSDSVggve1+oxYRTOKjVf7U5Ztvthu9CIraJXgHpWizq5wgbLrP5bUXiIZFxEaMAlxXODKbqzuomTsbkmzYMuNw3HNvUYL1AoV1Q1b8A7vSsRtVKDC7r5Zhl0/2zTsmpvlyhvLBS05fpvY6kIAVOsEKeBE2vhnfCTvqBqNkhElTVSsGcq07fldJmGoPNOyjk2I7u6Pa4unYHyPukUB/xzLQ//eKYdc4ZlTwDrIlSzXxJfrC/81LyFPgQvYcY2o1LoH0YQHNQmT6IJn9Mlg0p4zXXbzniKDEmFmbrp86wjCD3ubuzin2xHA8byRYi/R0pyxEzhlInDQKv1hRdqXbY13FbQmMC47S28Ah+j83JkLr6OfkT6PMT9RtocrGApIXgvhClP5CQLCFKKnpnCEbgdNAREoyEDNwyCGTk7VfKbOrOQGUZKiw1XoUsNZsB2+kt4bstUGEZiWzs7zkc0kVYd3swBPeT57YE4Ion9UMngS2mDw1QmQvfPtw/q8QKxGFWYSG2Z2oQU0EHyIQVnYppALdxi+61XDAlIZhPzQQ+8gSnaW/Clz/JT6S83xgc/dQcoE6LViTGJAjx3DzZCozMw7XXkNDjBqvy6HKcobho0Mp1c64WVHu3QAsIijDUtlxjtgXEfBo5UuMZoYC3OIlBoN5Tc2G+3H95p3p3k+Sf7LUq2tif8TbhdGbyjyFPqhluJBt0vZzkPrulW4vbaJWvZEjV3NHArkd/hlwTVa9B13WpvkxmLbxwRnoC+gK0yI4X3sUcwu6Ge9rTswoTi2nxPOuWgvNtlqGj9N5+p4Q4D Eim6NuJz CArPb27ZjD8u0A1o7KGh3XgoDiCbEUdWvHO6wTS2inXe4YX7WL/AeRs6dGqWPg6kMmuHwTxy30ezDFcLaoetDuJn/MkQxNlaR43YpCbBdWIzYf0krvf+e6bUUw0hzL9kJlHljB5RcDV16b2M1TUpt7j6DSsnBTI0j8nj8db7nNEJ/tZBghudPmSRd2xQvblW3X+mtg16k1hnk33Gkhol2Q6v16lsmR1GjuK9HF2bzW+TuyetCdKBb928QUT2KAsds098b4xW9cvUBvhkT7YEpXxCzSATprco3C6Px8uJVZU4UfD5I3S97DPQEqo9B7cBFGK6GSfootuv0dlCSz+TETDcig3bXrbg4QfGis2MaFcLXegkVnnR2q1bphD+K4mtYds2MSjf3WBB65lWUf7bC12wKnIEA0zHZ2gBr8YQS3B7yrwfoalGTyDLhkdRSVTWlzO08GQwPKXIHaEiozkNWNSLlcIE9f+JOQwVOnFf7vL/IXXhs/3jl59D/+V4coRzQCbDx1ElJts4vjwOnQvbSJI6/q3le8g0hurpuNFBKNyWgrjg= 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: On Sun, Feb 26, 2023 at 01:38:22PM +0900, Sergey Senozhatsky wrote: > 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? Agree. We don't need to combine them, then. BTW, I still prefer the enum instead of 10 define. enum fullness_group { ZS_EMPTY, ZS_INUSE_RATIO_MIN, ZS_INUSE_RATIO_ALMOST_FULL = 7, ZS_INUSE_RATIO_MAX = 10, ZS_FULL, NR_ZS_FULLNESS, } > > [..] > > > * 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. I didn't mean to have hard code either but just wanted to show the intention to use the loop. > > 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. Oh, yeah. Since it's debugfs, we would get excuse to break.