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 4DECDC46CD4 for ; Wed, 27 Dec 2023 06:11:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 39A0B6B006E; Wed, 27 Dec 2023 01:11:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 349166B0071; Wed, 27 Dec 2023 01:11:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 210A36B0072; Wed, 27 Dec 2023 01:11:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 119256B006E for ; Wed, 27 Dec 2023 01:11:19 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id D19E5160155 for ; Wed, 27 Dec 2023 06:11:18 +0000 (UTC) X-FDA: 81611575836.07.533919C Received: from mail-pg1-f181.google.com (mail-pg1-f181.google.com [209.85.215.181]) by imf26.hostedemail.com (Postfix) with ESMTP id 05EC0140017 for ; Wed, 27 Dec 2023 06:11:14 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=gqLxNSAi; spf=pass (imf26.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.215.181 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1703657476; 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=hk2kRuZJzsTncYz8diW0amRVuQLlRcJy88VPv7kReGA=; b=8rqshllp0Pwrh7OEIzPXvxyiMdRT203FxHE7wLvrGxBt15g0uSETjv+U594UrtxhBZSe6y HnkwVlV33GpwX7xxh1tzEWhgYOaR0LruX/W5KWZLKYNtBp3F/KqGXhq+jUCzz7f5M/BbPd nGdUes3/vQoOHHJN/SqFQVpjs+nwaUM= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=gqLxNSAi; spf=pass (imf26.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.215.181 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703657476; a=rsa-sha256; cv=none; b=DeL6cIbeXR2TKR5YL7xnCe0uvEGXyzyZuAQUvQi5CfSw47GpaABy/JB3S9qmS6/PdZaZur glv5R3FdcLjFF9Z21EdppyalmXJnEvEWWF5JWEqKFFrBUmmwDwqS+JbXNVzKnVkBBcnFzl Yk/xe0PmwDTt/L6+wequ6YusiV+5Y18= Received: by mail-pg1-f181.google.com with SMTP id 41be03b00d2f7-5cdbc7bebecso1372157a12.1 for ; Tue, 26 Dec 2023 22:11:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1703657473; x=1704262273; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=hk2kRuZJzsTncYz8diW0amRVuQLlRcJy88VPv7kReGA=; b=gqLxNSAidzddQ/Gdt9ZGrlcnSReRCmw8Ql+HKQD0gyr4l6CEJqpnWFFUAjtu4ufUe9 sd9qWWDi+8OjIYYJVOnJ9MmuxEUuOUHDTYE1DzjI4LBg9DG1jhgrPc0DGOnPYcPNiM1V noMDuMQcQUaQuMhHrwGibVWspicyNc5bWbCaVfBLJ5MzNfSt7jtRn6lELKPvir2ExtPZ E9yiAWF4WEuqAr3mkRqgf/QGUaCQc6N3Irhp0lfEcJOYjtTnYndnoTj36LG+NqCpINyg RjHB25lfY8R6DHRo4mYxqLtPmtUxgqmfQUpsJ0o9M72UigLu1+bkCVvVzDIb6Xna3wmA shTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703657473; x=1704262273; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=hk2kRuZJzsTncYz8diW0amRVuQLlRcJy88VPv7kReGA=; b=n9xAHmVssiPA+EiL897PmFJECj0nDomn+mMKYjV66yAij60hWVmXNXXzw8cBrNRINg e63Dl2xN+zwViQItyqpqqrfGTtC4IO85klvQURC6oql23bRsAjeGjhmQqPrS3qoj23L/ rvA4xf/p75WuvRqBVWC01JSeU3lh7voFsFU0l9iAURNepofeoBOAA8Y/m4VZ8ETzk90U vfST+HiNOSAg2YX/FuO8jVST0n79hyUP6UuhvBAMy6Siy4ignm0/HXp7VSSdNySr9ZhS roc9K1eNFErRIypvq6Yc+hMBy3kNSHnBP3GWC+i4ILzYh4aPNhUPPFfBkN3QAgVy2grV N8tA== X-Gm-Message-State: AOJu0YznNxqPWoBy0i/JVyko+knLXDWel1/J6bW1xNam32HSeok1gR8A grIl7UvscfveDSaZg9diDenE2QewKv1bwQ== X-Google-Smtp-Source: AGHT+IHZXpcj4F9r6j1E5ik6jZqZGkQ5M2E2bBGhP0Kx7ayOTSqytnoB9YneLu1KPzzmM4XwYwyf/w== X-Received: by 2002:a05:6a21:a5a8:b0:195:e20b:f02c with SMTP id gd40-20020a056a21a5a800b00195e20bf02cmr835139pzc.98.1703657473489; Tue, 26 Dec 2023 22:11:13 -0800 (PST) Received: from [10.254.10.159] ([139.177.225.237]) by smtp.gmail.com with ESMTPSA id jb2-20020a170903258200b001d0c5037ed3sm11155478plb.46.2023.12.26.22.11.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 Dec 2023 22:11:13 -0800 (PST) Message-ID: Date: Wed, 27 Dec 2023 14:11:06 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 1/6] mm/zswap: change dstmem size to one page Content-Language: en-US To: Barry Song <21cnbao@gmail.com> Cc: Andrew Morton , Seth Jennings , Johannes Weiner , Vitaly Wool , Nhat Pham , Chris Li , Yosry Ahmed , Dan Streetman , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Chris Li References: <20231213-zswap-dstmem-v4-0-f228b059dd89@bytedance.com> <20231213-zswap-dstmem-v4-1-f228b059dd89@bytedance.com> From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 05EC0140017 X-Rspam-User: X-Stat-Signature: qnsmbnc1x6xdt7enm3kesc4rk5318kb8 X-Rspamd-Server: rspam01 X-HE-Tag: 1703657474-144270 X-HE-Meta: U2FsdGVkX19AMvrSwkonStBDR0qmFM5f4uWZsROU+3ZEJxb6o5fmsgwi0EWnt0COvK+Bvpivl8DKoruQ2bE/f0P3N3wM7Knk4Mg18QJVj2XjAzQQ04pHQX6jzB3nB/Ftpixoubs14nDQexPS3yIgMCO7Pj236pMmhKs42HdZAIKo1zSlVTMTKETU3uz5pYF64m5V33rpUmuXbkfTboZaRdO67fiJv/vyrOYhAAjvkQAFcPI8+SdTe5sQVxKWrfMUx/apl7WzFg3WqIIMyxAbHfxgLs4prTONaK7MrSAGKmbiU45ppD3tj94Gc6fDHsjEESHXbZLD5tz3KQGCeVv3UekEJ0/NQlkoaHfS9ubf6Vbtj0ovhnCbijjhLNHbviNQ296T91KgONhvSBeAPLlUl2S1c18/R8L2oFvRIh3E3/M9IMJt74rv1wdljAJGwG3bipGVLMEPd3MrD2oPutjquhqd4radJUScg4kO07Lfe6a8/+ckD7gQZofljFh5TVS1lvI6S6L2peQl5fAcVU0KHJFtvT7AsBPtgqILz06gLNjAvMG8vghdmg/gM/uz7gJZtW6bRbsimZopdNSQ/RGRg+++quxC7D5IZeLksVdF8N9lJhxRrJobO9Ff1XCBPAPgXc/iFXbnGiYcfGPtn6kUkzyj76REVF7EJEHOAws8pLA2toXM7iTjpcuYytgoMtT6wTQU8HjJmQkF3CjymnHVY5e3yXcuiYLdD3TNplJJc6GhLU+2p4XvlZ9YhLIIIxV+oed44pxjB2nccq0HUu1wmPq5Q3VDuc/10/qbsXNyu7uo+oIv29EWQuZ/HO4+SPB4WA1yaXo6LLtd6Nfv5fJ+c4eywbFZ8/gllqtR0ocn3HvWQZXRgT1TJuOOjjfDc3E20RwGuqUlsTQuPmz3+n3g80w/dkiqMMvBSEuW4PM2lgAxyBz2ad8NAHLGhPa2Tm6Qk5ZciXL40pqXslkSJys IEHaPVm8 TZHJI74jSvVDrD2w67EFnSQJc3TPjnPHNszmHaHwe5aGmA5CqL/pk61j2gLpZ9rRKgy2LzGHhIkQ5cAGO+2e1JAPukvzM4YfX2FRB/WkcYjjyYgzcRI0BalNFy2QV3ufeC+Y4OGzXhFHdx5ASkhV7MTEm6b22v1kpRRclDM8T93FkWZoQi2LMMo/VWA0JLsBxPBbqaOK188wx3TmoKTPGP6A11v1RRJJ6r0GZJnMrrL0B9TxVDIh0VNhFDhMQGq8SgPFxg4G4FhdjlXxoyRUGTVYGbzezxqm6Thd0KzHIdydckm1aeZSCFRD6+Tc5J8Jr/bFAjOK14QOAShUMSKzQAcjG99UDQYE+z2dedxMsvUgFPJPU4ciIhj2f6v1+E++dnTq/T88QUD/OImrdZwiccM++d31kuGRPRPDtK3mlbSc6GamzXqs3zY9EswUTRtCvWhSLCs/FnfCvZSEW7Ya3id9rCxnhaiHCXec3dZilOkUf54zQF4erAI5JbF9b8yU6z3oxrMeAYn1Xshv1k1/rfr5s4DXTqzC0VOXaeeWHY1Ayn9l5Grr0dHm5S0F8vKdbLKwJ 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 2023/12/27 09:07, Barry Song wrote: > On Wed, Dec 27, 2023 at 4:55 AM Chengming Zhou > wrote: >> >> Change the dstmem size from 2 * PAGE_SIZE to only one page since >> we only need at most one page when compress, and the "dlen" is also >> PAGE_SIZE in acomp_request_set_params(). If the output size > PAGE_SIZE >> we don't wanna store the output in zswap anyway. >> >> So change it to one page, and delete the stale comment. >> >> There is no any history about the reason why we needed 2 pages, it has >> been 2 * PAGE_SIZE since the time zswap was first merged. > > i remember there was an over-compression case, that means the compressed > data can be bigger than the source data. the similar thing is also done in zram > drivers/block/zram/zcomp.c Right, there is a buffer overflow report[1] that I just +to you. I think over-compression is all right, but buffer overflow is not acceptable, so we should fix any buffer overflow problem IMHO. Anyway, 2 pages maybe overflowed too, just with smaller probability, right? Thanks. > > int zcomp_compress(struct zcomp_strm *zstrm, > const void *src, unsigned int *dst_len) > { > /* > * Our dst memory (zstrm->buffer) is always `2 * PAGE_SIZE' sized > * because sometimes we can endup having a bigger compressed data > * due to various reasons: for example compression algorithms tend > * to add some padding to the compressed buffer. Speaking of padding, > * comp algorithm `842' pads the compressed length to multiple of 8 > * and returns -ENOSP when the dst memory is not big enough, which > * is not something that ZRAM wants to see. We can handle the > * `compressed_size > PAGE_SIZE' case easily in ZRAM, but when we > * receive -ERRNO from the compressing backend we can't help it > * anymore. To make `842' happy we need to tell the exact size of > * the dst buffer, zram_drv will take care of the fact that > * compressed buffer is too big. > */ > *dst_len = PAGE_SIZE * 2; > > return crypto_comp_compress(zstrm->tfm, > src, PAGE_SIZE, > zstrm->buffer, dst_len); > } > > >> >> According to Yosry and Nhat, one potential reason is that we used to >> store a zswap header containing the swap entry in the compressed page >> for writeback purposes, but we don't do that anymore. >> >> This patch works good in kernel build testing even when the input data >> doesn't compress at all (i.e. dlen == PAGE_SIZE), which we can see >> from the bpftrace tool: >> >> bpftrace -e 'k:zpool_malloc {@[(uint32)arg1==4096]=count()}' >> @[1]: 2 >> @[0]: 12011430 >> >> Reviewed-by: Yosry Ahmed >> Reviewed-by: Nhat Pham >> Acked-by: Chris Li (Google) >> Signed-off-by: Chengming Zhou >> --- >> mm/zswap.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/mm/zswap.c b/mm/zswap.c >> index 7ee54a3d8281..976f278aa507 100644 >> --- a/mm/zswap.c >> +++ b/mm/zswap.c >> @@ -707,7 +707,7 @@ static int zswap_dstmem_prepare(unsigned int cpu) >> struct mutex *mutex; >> u8 *dst; >> >> - dst = kmalloc_node(PAGE_SIZE * 2, GFP_KERNEL, cpu_to_node(cpu)); >> + dst = kmalloc_node(PAGE_SIZE, GFP_KERNEL, cpu_to_node(cpu)); >> if (!dst) >> return -ENOMEM; >> >> @@ -1662,8 +1662,7 @@ bool zswap_store(struct folio *folio) >> sg_init_table(&input, 1); >> sg_set_page(&input, page, PAGE_SIZE, 0); >> >> - /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */ >> - sg_init_one(&output, dst, PAGE_SIZE * 2); >> + sg_init_one(&output, dst, PAGE_SIZE); >> acomp_request_set_params(acomp_ctx->req, &input, &output, PAGE_SIZE, dlen); >> /* >> * it maybe looks a little bit silly that we send an asynchronous request, >> >> -- >> b4 0.10.1 >> > > Thanks > Barry