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 B1C99C4332F for ; Thu, 14 Dec 2023 18:25:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 333B96B0604; Thu, 14 Dec 2023 13:25:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2E37F6B0605; Thu, 14 Dec 2023 13:25:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 15D3B6B0606; Thu, 14 Dec 2023 13:25:18 -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 F410A6B0604 for ; Thu, 14 Dec 2023 13:25:17 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id BBC2B80D1E for ; Thu, 14 Dec 2023 18:25:17 +0000 (UTC) X-FDA: 81566251074.03.C48F574 Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by imf24.hostedemail.com (Postfix) with ESMTP id EF6A618000D for ; Thu, 14 Dec 2023 18:25:15 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="Y6FLF/vI"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702578316; a=rsa-sha256; cv=none; b=KxT7v9eo1/TTorNWEQFwrZU45wp+RlJEdlWNZPqgywr7KVJfBkdbxndkjPKefdNrxuCB53 b1yj33OcJNU/dTEWbb4cA1F1oSMt0HxIq5kGc6ZgdDg1hniDuicGWyqM54tLiDzAvNf1Mg EO59Cy8T8L5wD92qkkzpbnvX/p2WeoI= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="Y6FLF/vI"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702578316; 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=KWNO9gKTX+H6PtTXgZaijRLRW5/UDeehQa2JwBF5FxI=; b=u+UxNPSc5HDEtq1XYwHcQDataJ0q0mXNB4V/8Ff35Wm12sVzUso+UTo2NSZVICa8wCmawW 6piEamnbaU8Q4ZFq5JLx2h28RZyiFMGOdiGxWcC/rOY8s4MDA1g0CW02ANb1yPkHfcghLQ WApmo3rx6uSiqwuY0gVxpkIXLAqvXVk= Received: by mail-ej1-f43.google.com with SMTP id a640c23a62f3a-a1ef2f5ed01so1067757766b.0 for ; Thu, 14 Dec 2023 10:25:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702578314; x=1703183114; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=KWNO9gKTX+H6PtTXgZaijRLRW5/UDeehQa2JwBF5FxI=; b=Y6FLF/vIhQdDG0/8a97qt2Z6Ma78s2uufq5xrCSQyPrwCujCqFtKcB668HsJjYUt4Z OtoZrqptLoV1Y7HxnyZ9dV/sq9Xf3P96k6tIry7vKIfLv3KEqbqEKDibs+fbm6UNtgEf SJlOIhF0XuCmfEPeS4VcERjglVh0PrFPhQjS6r0dNqkcscGqGOSPLn8EV3yJeyz2kCV3 jy4U42B9+HqC2zgfX8G+zhT1n6qqRd2tSCDTgSKBcekUU8Cfs+XwYb3svXmqIp5lICRX vQFhC0Chn8hb7BDo3HjULWzEOzT/E1MQb5Cj5l0feHCh6cGwTOF/5J3iFNkAO28+NcaA K2IA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702578314; x=1703183114; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KWNO9gKTX+H6PtTXgZaijRLRW5/UDeehQa2JwBF5FxI=; b=Tf9/YK1RX53cMlvWEFnvH+P4VK0O2/Y7iXtLjhnODGcNgn4HQdDOTZ8VcnC2soLlz2 nCpZb2yqcybEP/zEH627p5ERP7Fq8fmG6H03BhjIA0g+difj2BF//KxD/MY2jTLnXvus lq/XaosC0rw2NKopRtOilQblk1nfyQv1Ur486QCfSdJGw/U9gxv1By5F8HPFC5fsyzPO EI9m9FE6z2sdwk4O7807WWNB+r6+AXAj0YZiJbpyAeE0f4WJ8kyZPrr7cK6ExUrnPlxV oWZUnB8iVn5LOI/vygCHT+MjUW7oDxgQ2egaMOcsAKzRpssYpCrR5Y6fMsbAbDQtgHfC TjTg== X-Gm-Message-State: AOJu0YzY+Vs1ZXrQ8MWOKjolzYB1V01VjcciLsZ/tq+FonoBrzHwe2TW BTdZslMwJuAJhNsXfQAL35wsQoMAidsFI8bL3PVFZA== X-Google-Smtp-Source: AGHT+IF5aVTry2FC8fPpvHagersJX+u6ON5srtl/dhTppR5fbUmyPtKjz52t7C4jg+PxzIo0GY0Gp29Ro3eP/IqhN4k= X-Received: by 2002:a17:906:53d9:b0:a1d:2205:a484 with SMTP id p25-20020a17090653d900b00a1d2205a484mr2669130ejo.113.1702578314287; Thu, 14 Dec 2023 10:25:14 -0800 (PST) MIME-Version: 1.0 References: <20231213-zswap-dstmem-v1-0-896763369d04@bytedance.com> <20231213-zswap-dstmem-v1-1-896763369d04@bytedance.com> <8f80fe3e-a8c7-463d-896b-99575c362839@bytedance.com> In-Reply-To: From: Yosry Ahmed Date: Thu, 14 Dec 2023 10:24:36 -0800 Message-ID: Subject: Re: [PATCH 1/5] mm/zswap: reuse dstmem when decompress To: Chengming Zhou Cc: Andrew Morton , Nhat Pham , Chris Li , Johannes Weiner , Seth Jennings , Dan Streetman , Vitaly Wool , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: EF6A618000D X-Stat-Signature: 9i7u8xetrdphpc3398gbfg55dsioxokg X-HE-Tag: 1702578315-526361 X-HE-Meta: U2FsdGVkX1+vJgeEA7SgSq2MjzfSQLjAUCsw/wwUzIDdA+pYgm72JdszqCNnxvyk9SpCkQHHCwbXx+06SGQUcJLQKNpS0Vrb5GFmrxAqpy49l9FENHdNOR0Y3JK/vm+Ate9NdXUhiHyNKEqn1J2qlX7UeKyYxqZxfTd4fgZCukennqQuRMZUaw5hZd1QwMV6Ml4fh/KlWstF96GPHo+OyQ/8E2gkXBkG/YMhQJZrUsBGprVRzNZl/a/JjpiuNeT5EapQtJYyGlZZfhS6u62kvOGsX5Pts2/SiYcCHGZlhONy2olg5gVBgJ0ONdetrKAIZceKe/K/xJiAPVNCC9LvR9KGgz7q8shODHA45R1YHQRZiHp4kpiOPTIFvu+pKuzyFYMKMEixlRellOdMaMFWC1g5WPKFV3vX/RnaiU3YOmoLlPp+wRxxIh2IgjMXwNH9rHq3LuOexH+PW9gIeQj5RDnTDX+MO1LDMzRzAfwmJ6PDPl5RpM+Xs8TBBu9/K9AjqG029MBriw6XzS4fl8M9X1CdgQFt9j5mCQVQUmZAR6ili4Lda2s1auRCt+sq0r6kZaH+dRcgeALWAvN+CDSK36uIsH2msi5G4XTEhDu4ZJWZ294KsTB7s629q/Lzw97Wcm73GACmTo2wjHXWf6o0FZeuR6XicDB8ekF1BZ71VrZk/mXaUD0qlff/30H8kuCU/TbigtR1RMIzwE4XKSVPcoxIKxNUGkUNlLOmPJGHiUuSFw0QHopT3SR3iHlVqKvKl3kMkXfaz3s/gBiDIqSlnoPc0Wjb3dsuHu42HdF8wdMzdgPNMzNNRh1jzE1mQ18k9K1j3l7IuoaOe7R1a83hKN01kgaZfK/P9d9tBNx+ew3qqgJ0MWnISQsyhaMefqo7pjKN2k7jUXvGOOwmiE7U+URmqg3I0wdiZEi2xlj5fw5nduoiJls0eJB0mXrRciTrrj41M0ZM28V5Dnv/DxY juwj4Ghv ZKoA5ZUFTpnG4JjTbYgtyfsB1yRmffSCTdMPAlgvp1mCkytShOG54MKtb9xM3wrCm4Qa1OZsNm9EfcaZY2qRWz48LlmuhJL+ZmdVDkgNeQx4a67mtxEKrwsSfNVXzGlVt8g5WvrvHszoo5f496hJ2J83+v3erUAwsuswRvZFOHsqzGIeuNlG3Qj9dMwM0SBLIBn9W4DOCpVXYy4xo0ZqtjC8tVPt8GTYVOnVyoNq6U65yUfUE9zZkak/sQx5mjyx0X5yviZQpfMRPgQ/sHu4OSB1rXGXP1HTz+M4N 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 Thu, Dec 14, 2023 at 6:42=E2=80=AFAM Chengming Zhou wrote: [..] > >>>> - > >>>> /* decompress */ > >>>> - dlen =3D PAGE_SIZE; > >>>> - src =3D zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO); > >>>> + acomp_ctx =3D raw_cpu_ptr(entry->pool->acomp_ctx); > >>>> + mutex_lock(acomp_ctx->mutex); > >>>> > >>>> + zpool =3D zswap_find_zpool(entry); > >>>> + src =3D zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO); > >>>> if (!zpool_can_sleep_mapped(zpool)) { > >>>> - memcpy(tmp, src, entry->length); > >>>> - src =3D tmp; > >>>> + memcpy(acomp_ctx->dstmem, src, entry->length); > >>>> + src =3D acomp_ctx->dstmem; > >>> > >>> I don't like that we are now using acomp_ctx->dstmem and > >>> acomp_ctx->mutex now for purposes other than what the naming suggests= . > >> > >> The "mutex" name is coherent, "dstmem" depends on how we use IMHO. > >> Change to just "mem"? Or do you have a better name to replace? > >> > >>> > >>> How about removing these two fields from acomp_ctx, and directly usin= g > >>> zswap_dstmem and zswap_mutex in both the load and store paths, rename > >>> them, and add proper comments above their definitions that they are > >>> for generic percpu buffering on the load and store paths? > >> > >> Yes, they are percpu memory and lock, but they are used by per acomp_c= tx, > >> and the cpu maybe changing in the middle, so maybe better to keep them= . > > > > I don't mean to remove completely. Keep them as (for example) > > zswap_mem and zswap_mutex global percpu variables, and not have > > pointers in acomp_ctx to them. Instead of using acomp_ctx->dstmem > > today, we directly use the global zswap_mem (same for the mutex). > > > > This makes it clear that the buffers are not owned or exclusively used > > by the acomp_ctx. WDYT? > > Does this look good to you? > > ``` > int cpu =3D raw_smp_processor_id(); > > mutex =3D per_cpu(zswap_mutex, cpu); > mutex_lock(mutex); > > dstmem =3D per_cpu(zswap_dstmem, cpu); Renaming to zswap_buffer or zswap_mem would be better I think, but yeah what I had in mind is having zswap_mutex and zswap_[dstmem/mem/buffer] be generic percpu buffers that are used by store and load paths for different purposes, not directly linked to acomp_ctx. > acomp_ctx =3D per_cpu_ptr(pool->acomp_ctx, cpu); > > /* compress or decompress */ > ``` > > Another way I just think of is to make acomp_ctx own its lock and buffer, > and we could delete these percpu zswap_mutex and zswap_dstmem instead. You mean have two separate set of percpu buffers for zswap load & stores paths? This is probably unnecessary.