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 801E4C4332F for ; Thu, 3 Nov 2022 17:00:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ECABF6B0072; Thu, 3 Nov 2022 13:00:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E7B4C6B0073; Thu, 3 Nov 2022 13:00:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D1B026B0074; Thu, 3 Nov 2022 13:00:37 -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 B94416B0072 for ; Thu, 3 Nov 2022 13:00:37 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8C3D34151A for ; Thu, 3 Nov 2022 17:00:37 +0000 (UTC) X-FDA: 80092744914.10.D703440 Received: from mail-pg1-f180.google.com (mail-pg1-f180.google.com [209.85.215.180]) by imf19.hostedemail.com (Postfix) with ESMTP id 0385C1A000C for ; Thu, 3 Nov 2022 17:00:36 +0000 (UTC) Received: by mail-pg1-f180.google.com with SMTP id h193so2171590pgc.10 for ; Thu, 03 Nov 2022 10:00:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; 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=CYg+m5oLj8gc4kPsOwH3pQu6lmRCj0M+7nnvmSQAEtk=; b=ek0Tnf71xlL+BT9a8ZACJoSOdDzV2J9rf1Ooq0ml9b5EbDqxl048g53h5TgacjwdaP zsOwcUq2uVEFaWBGra1dV/rvd5Nw3JSSfJsezjJMKbm5TrSNVf/BUgH3/mEu8XQZ7obG pLHcgHeNhxJ7Vi6R1IhLq1FMJ9hA+HHrPEVBwYQkH1HszDmKU/7pZ5bd1UPiRxwRy8Wd +IiXtL1uUij+BveVQkFmhEzPNqu0SaAawTIkm6xcENDyNGYJJPly3TWiapjhgxldwLQs ikcsm1A96ge1R8rWBxdrGT8F0uctLpNc4yw5y5OHweeSwtAR0kQer2GS2yCVB2xzz2sQ DzBQ== 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:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=CYg+m5oLj8gc4kPsOwH3pQu6lmRCj0M+7nnvmSQAEtk=; b=dDGbasIQ0KCfglc/rzqN1wUXTaEhmiMv05MtJU5dsx2/UgivLAO9ACW1/D/TcbUjC1 zQkv9b7LYUTWXHXMuHE0H3Rrz65AXPFSZlRDoakfnXsELgdk5voFJ/37jMv3N2ZUzdGB sBxWTsepLEFMG4bmYAtPwb1eKbBrqlUGdkNBRM9wDFlJfPHaiDahCCwbeb6shx2tdfo6 djx7HhPeSSf0tGIvy3KNY6reTSv2m4WB3OkC4JPnPKS15WM1wn9mwTqsmYDHJdp7lW+1 SYJk6Wi6fiU226RTgn7YQ2M0CH1nzJCaZhhP3jtv9Ez3xlKHxj78xxQu5MsbP4LrhhiX ZJpw== X-Gm-Message-State: ACrzQf0fc5hYPLhSnqgHVqfjGHHQnWPg+stCtCCVZElr3jQchjG0cKwB JYY2h5+/HLUCb8Q/Q39Yo0k= X-Google-Smtp-Source: AMsMyM6Li/qF0VF08Jhehm/+qV40PSQ2spm6kzzjk6TZWN5wstVGu+YbrXpnEhkYxX/OI6nVD4nJlw== X-Received: by 2002:a63:90c7:0:b0:442:87:3a38 with SMTP id a190-20020a6390c7000000b0044200873a38mr27164332pge.216.1667494835812; Thu, 03 Nov 2022 10:00:35 -0700 (PDT) Received: from google.com ([2620:15c:211:201:3d65:7dc2:c62a:5d98]) by smtp.gmail.com with ESMTPSA id e11-20020a170902784b00b0016d72804664sm880085pln.205.2022.11.03.10.00.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 10:00:35 -0700 (PDT) Date: Thu, 3 Nov 2022 10:00:33 -0700 From: Minchan Kim To: Sergey Senozhatsky Cc: Andrew Morton , Nitin Gupta , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCHv4 4/9] zram: Introduce recompress sysfs knob Message-ID: References: <20221018045533.2396670-1-senozhatsky@chromium.org> <20221018045533.2396670-5-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667494837; 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=CYg+m5oLj8gc4kPsOwH3pQu6lmRCj0M+7nnvmSQAEtk=; b=fVXRcPyRYcJQYqnRra84vKDLjwi0FEsQFRPoFtemJ9EudTWMz7NzHJ/ppuvYDd9aK8SnpJ 1rx95Mht+IiCKHmTbnb6w6ylTYIso34jYMUTc9RZY+frBQVFYcfU0jGEwWu4nXPakXBthk 8ifR03SmCdDBIAlITUX3xH4zbtmGD3c= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ek0Tnf71; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (imf19.hostedemail.com: domain of minchan.kim@gmail.com designates 209.85.215.180 as permitted sender) smtp.mailfrom=minchan.kim@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667494837; a=rsa-sha256; cv=none; b=SE2mYNXQpfrsvZ6cKLsrWebcg84xta8gEn5jcerP1JIhU7S5OgSi6dOTCMEOxUdpIiacmS YoXPyUnW+7Zkzx6tkCYoY9ZoXzhtSay3YgT1NF1DVfTK0OyBtNZh6D41Arfx9reWa/a42W 4RZL/oKFkI7P5ROm0Zapt+1FdQO8nd8= Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ek0Tnf71; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (imf19.hostedemail.com: domain of minchan.kim@gmail.com designates 209.85.215.180 as permitted sender) smtp.mailfrom=minchan.kim@gmail.com X-Rspam-User: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 0385C1A000C X-Stat-Signature: r8za3xji7e7mqp1f37381xaxbeihj7ui X-HE-Tag: 1667494836-364620 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 Thu, Nov 03, 2022 at 12:25:43PM +0900, Sergey Senozhatsky wrote: > On (22/11/02 14:06), Minchan Kim wrote: > > On Tue, Oct 18, 2022 at 01:55:28PM +0900, Sergey Senozhatsky wrote: > > > Allow zram to recompress (using secondary compression streams) > > > pages. We support three modes: > > > > > > 1) IDLE pages recompression is activated by `idle` mode > > > > > > echo idle > /sys/block/zram0/recompress > > > > > > 2) Since there may be many idle pages user-space may pass a size > > > watermark value (in bytes) and we will recompress IDLE pages only > > > of equal or greater size: > > > > > > echo 888 > /sys/block/zram0/recompress > > > > Hmm, how about having seperate knob for threshold? > > Per-my understanding this threshold can change quite often, > depending on memory pressure and so on. So we may force > user-space to issues more syscalls, without any gain in > simplicity. Sorry, didn't understand your point. Let me clarify my idea. If we have separate knob for recompress thresh hold, we could work like this. # recompress any compressed pages which is greater than 888 bytes. echo 888 > /sys/block/zram0/recompress_threshold # try to compress any pages greather than threshold with following # algorithm. echo "type=lzo priority=1" > /sys/block/zram0/recompress_algo echo "type=zstd priority=2" > /sys/block/zram0/recompress_algo echo "type=deflate priority=3" > /sys/block/zram0/recompress_algo > > > recompress_threshold > > > > With that, we could make rescompress 888 and idle/huge > > as well as only 888. > > > > echo 888 > /sys/block/zram0/recompress_threshold > > echo 1 > /sys/block/zram0/recompress > > > > or > > > > echo 888 > /sys/block/zram0/recompress_threshold > > echo idle > /sys/block/zram0/recompress > > > > or we can introduce the threshold with action item. > > > > echo "idle 888" > /sys/block/zram0/recompress > > echo "huge 888" > /sys/block/zram0/recompress > > echo "normal 888" > /sys/block/zram0/recompress > > I like the latter one, when threshold is an optional argument. > I probably would even go a bit further and add keywords: > > type=STRING threshold=INT Yeah, kerwords would be better. Let's discuss we need a threshold optional argument for each algo or just have one threshold for every secondary algorithm or both. > > > > 3) HUGE pages recompression is activated by `huge` mode > > > > > > echo huge > /sys/block/zram0/recompress > > > > > > 4) HUGE_IDLE pages recompression is activated by `huge_idle` mode > > > > > > echo huge_idle > /sys/block/zram0/recompress > > > > > > Signed-off-by: Sergey Senozhatsky > > > --- > > > drivers/block/zram/Kconfig | 15 +++ > > > drivers/block/zram/zram_drv.c | 196 +++++++++++++++++++++++++++++++++- > > > drivers/block/zram/zram_drv.h | 2 + > > > 3 files changed, 210 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig > > > index d4100b0c083e..3e00656a6f8a 100644 > > > --- a/drivers/block/zram/Kconfig > > > +++ b/drivers/block/zram/Kconfig > > > @@ -78,3 +78,18 @@ config ZRAM_MEMORY_TRACKING > > > /sys/kernel/debug/zram/zramX/block_state. > > > > > > See Documentation/admin-guide/blockdev/zram.rst for more information. > > > + > > > +config ZRAM_MULTI_COMP > > > + bool "Enable multiple per-CPU compression streams" > > > > per-CPU is implementation detail. Let's do not mention it. > > OK. > > > > + depends on ZRAM > > > + help > > > + This will enable per-CPU multi-compression streams, so that ZRAM > > > > indentation > > OK. A question: does this matter? I don't see any problems in menuconfig. > > > > + can re-compress IDLE/huge pages, using a potentially slower but > > > + more effective compression algorithm. Note, that IDLE page support > > > + requires ZRAM_MEMORY_TRACKING. > > > + > > > + echo TIMEOUT > /sys/block/zramX/idle > > > + echo SIZE > /sys/block/zramX/recompress > > > + > > > + SIZE (in bytes) parameter sets the object size watermark: idle > > > + objects that are of a smaller size will not get recompressed. > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > > > index 94c62d7ea818..da11560ecf70 100644 > > > --- a/drivers/block/zram/zram_drv.c > > > +++ b/drivers/block/zram/zram_drv.c > > > @@ -1282,6 +1282,12 @@ static void zram_free_page(struct zram *zram, size_t index) > > > atomic64_dec(&zram->stats.huge_pages); > > > } > > > > > > + if (zram_test_flag(zram, index, ZRAM_RECOMP)) > > > + zram_clear_flag(zram, index, ZRAM_RECOMP); > > > + > > > + if (zram_test_flag(zram, index, ZRAM_RECOMP_SKIP)) > > > + zram_clear_flag(zram, index, ZRAM_RECOMP_SKIP); > > > > Let's squeeze the comp algo index into meta area since we have > > some rooms for the bits. Then can we could remove the specific > > recomp two flags? > > What is meta area? zram->table[index].flags If we squeeze the algorithm index, we could work like this without ZRAM_RECOMP_SKIP. read_block_state zram_algo_idx(zram, index) > 0 ? 'r' : '.'); zram_read_from_zpool if (zram_algo_idx(zram, idx) != 0) idx = 1; zram_recompress .. we don't need to set ZRAM_RECOMP since every meta will have the algo idx. zram_free_page zram_clear_algo(zram, index); recompress_store int algo_idx = zram_algo_idx(zram, index); if (algo_idx == max_algo_idx) goto next > > > I am thinking the feature to work incoming pages on the fly, > > not only for recompress manual knob so it would be good > > if we could make the interface abstract to work something > > like this(I may miss something why we couldn't. I need more > > time to look into then) > > > > zram_bvec_write: > > > > for (i = 0; i < MAX_COMP_ALGO; i++) { > > zstrm = zcomp_stream_get(i); > > zcomp_compress(src, &comp_len); > > if (comp_len > threshold) { > > zcomp_stream_put(i); > > continue; > > } > > break; > > } > > > > zram_bvec_read: > > algo_idx = zram_get_algo_idx(zram, index); > > zstrm = zcomp_stream_get(zram, algo_idx); > > zcomp_decompress(zstrm); > > zcomp_stream_put(zram, algo_idx); > > Hmm. This is something that should not be enabled by default. Exactly. I don't mean to enable by default, either. > N compressions per every stored page is very very CPU and > power intensive. We definitely want a way to have recompression > as a user-space event, which gives all sorts of flexibility and > extensibility. ZRAM doesn't (and should not) know about too many > things, so ZRAM can't make good decisions (and probably should not > try). User-space can make good decisions on the other hand. > > So recompression for us is not something that happens all the time, > unconditionally. It's something that happens sometimes, depending on > the situation on the host. Totally agree. I am not saying we should enable the feature by default but at lesat consider it for the future. I have something in mind to be useful later. > > [..] > > > +static int zram_recompress(struct zram *zram, u32 index, struct page *page, > > > + int size_watermark) > > > +{ > > > + unsigned long handle_prev; > > > + unsigned long handle_next; > > > + unsigned int comp_len_next; > > > + unsigned int comp_len_prev; > > > > How about orig_handle and new_nandle with orig_comp_len and new_comp_len? > > No opinion. Can we have prev and next? :) prev and next gives the impression position something like list. orig and new gives the impression stale and fresh. We are doing latter here.