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 3AB4FC41535 for ; Tue, 19 Dec 2023 21:39:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A3ABA8D0007; Tue, 19 Dec 2023 16:39:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9EB3B8D0001; Tue, 19 Dec 2023 16:39:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8B39A8D0007; Tue, 19 Dec 2023 16:39:57 -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 7616E8D0001 for ; Tue, 19 Dec 2023 16:39:57 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 3BE08140397 for ; Tue, 19 Dec 2023 21:39:57 +0000 (UTC) X-FDA: 81584885634.11.8274526 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by imf07.hostedemail.com (Postfix) with ESMTP id 732B440005 for ; Tue, 19 Dec 2023 21:39:55 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=WHHibhG2; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.45 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=1703021995; 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=PqnUL3STGI/mTYPrGHX2qTH/VZgVQ1K394F/fVIBFiQ=; b=qCEq8weTjxEiNfL5aPFmD70p33uGd677ixFRmQO/UT/SC4TxziTaALU+wUQcI6x399HEtZ yci6SKX5nGBhYRpY4mr363Ek9fpIXQGNgFpqqJBTsoKirYNicMb21rsP1jpBU7giRvOBlP fbepn9XLp2EEYZyrz+xr0vHGOpo82qc= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=WHHibhG2; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.45 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703021995; a=rsa-sha256; cv=none; b=N5MT7QUMn9jRHC/evNYlbCHJv87CqGOR0Y1FI4E+L+W7gsxm0KVrDyp6+7jXWvtH3F14eJ tlg34SFfQvwJbzYWquqWm+4RLfX6hCZVlmqLNhwjnP5gAxPrLjJIaPIM/jQZ6Vywx+fWis 2EHvIjsMq4FGc0kaJ8WgZBtJ/1qUrkI= Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-3363aa2bbfbso5393719f8f.0 for ; Tue, 19 Dec 2023 13:39:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1703021994; x=1703626794; 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=PqnUL3STGI/mTYPrGHX2qTH/VZgVQ1K394F/fVIBFiQ=; b=WHHibhG2Xo/cIQf0gDrzU2nEf0C9mAE4FCdOMW8ymMrTvmxi2K1c8KY2DyXCJl+LKo +qif6OTiYOpLKSWhUWbEbkh5+qenP0Qvmwokl6yMYGcAilxemhAIZuGeGbtUqPwkaFqQ w8dm1cKj6+cBITj1iBTYqjlBm+pczBa22YmDR8MDxYMiwUF2KOl3TLBFXpYvdFkWVdAK P4N+kyGaSMaYnur/1QO1DU2NZkF0olRlZhinzjPXQjFUu0+9H6Ac5P4AragSlXJcZWT4 gPupXwvakjLuAFN5uhEcIiBpcNSQLIC9rF/g7lu7iDHO2BiaeA6ZEtUls+dz1U2OSXZN pTnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703021994; x=1703626794; 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=PqnUL3STGI/mTYPrGHX2qTH/VZgVQ1K394F/fVIBFiQ=; b=lRzRIfzD+9ctPcPxCkOpdWSdcG06E0nWWSzeDWa76NIiHTwfEWgchzJf+e4sTkq1wH rnljeXn1ASOrMrDxzcgYjR3zzQ1YroegzzKQwn8PGTBwE7Y1mYBn4PCErj7nQ/x8Qasc iZ7tdMjVXQIn8MPz/wkatueUejX6qBqgmUA3xlgcxUSOsmxHOjk7g86/F3sb+KBse7gT Z3ME4EA0U81eGzJFvgAfjW5ZuTiXfveEwOSLCWcfVfdyhxLwmKhrfn17DmOOBHfofn3+ jVJ3iiwn1IPFSqZzuv4+xiW2xip8ylBFnDx1RVSqycQn7bEViPiE1jgxuNgAgesA23pT +4jw== X-Gm-Message-State: AOJu0Yyb6oawm6kZBmp8l2UslzshvZtRmbdVugrR2DdxhvbmLmdNk9IN cPWDH7uXVFyz9SOJxUYFwlZ9Z0zmu05vt3PJCD/1Ew== X-Google-Smtp-Source: AGHT+IHDjOPMB0Rw4CsegtN+gghw2G+4WfN33bekHI42mt3gv0tuLFeWAGQoVBxh0Y/JHausWNTzYfD+Oecrm6SQwms= X-Received: by 2002:a5d:6586:0:b0:336:788b:335c with SMTP id q6-20020a5d6586000000b00336788b335cmr196486wru.106.1703021993678; Tue, 19 Dec 2023 13:39:53 -0800 (PST) MIME-Version: 1.0 References: <20231213-zswap-dstmem-v3-0-4eac09b94ece@bytedance.com> <20231213-zswap-dstmem-v3-6-4eac09b94ece@bytedance.com> In-Reply-To: From: Yosry Ahmed Date: Tue, 19 Dec 2023 13:39:15 -0800 Message-ID: Subject: Re: [PATCH v3 6/6] mm/zswap: directly use percpu mutex and buffer in load/store To: Nhat Pham Cc: Chris Li , Chengming Zhou , Seth Jennings , Vitaly Wool , Dan Streetman , Johannes Weiner , Andrew Morton , 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: rspam12 X-Rspamd-Queue-Id: 732B440005 X-Stat-Signature: 5c1sdci14mqx5cwgyad41y8kzg7ed9ex X-HE-Tag: 1703021995-145850 X-HE-Meta: U2FsdGVkX18AgMkTGC6E5xz2lF+g4QrKdTHb25emXCfdU70APDq0n5abhVrpbwO++TNf23tVWSs4zpohGhROD+RvjPIhRPNMtVsHROiFfRCZqFfbwT7vMy0IFuXUn3vOlt+akT9jJ7Mi0TadXhioF3YzN+rAjqdQuz3Is70h2qe9PrJjm12Br3DiFr9ZSeqF0zH26iOpK5xeNwyX/6sWajtJTQ2WEn2C+iEDvHKk/WWxj2vXXw7OsjSlRjU8R8mx5QTgRJT69rAtpRXJXyyPnkXvwz2eIRWkcTe3es5UrWtZWCfbEsObtAl0tBkH8ybpJC64gossOWn2xIf00w5IlB/sjcVBQyGiGOZ4MmoRpXIgBuh60r1Mb6O6c4cGNvBLtsyWZ0MDRo4LEm4bRQivIHX38cTjf4sIKPW+erLpinNcSMIJMNu1iKY9sUPcoaVXNsdxaNnnGmU3betK5ra4biYr88PtFhokNM4kdUPk3NdqME9/KeotKOHUzUsg1S8GCO6juSHH2OhtV0BR0dUvh5dBXaFir8iKtkNqa97OaNbkCtJs7vNefSO+K2FJXF1R1mCYoBPL6t0ojcLspvy9zH3cnancPAkzpZfkzghQSAAmiYIsH0O06z0RLluESeuu6ph8GXfOr6xu2gI4ENf3K5nDTju4QWGWMKJgTHMCD5C1etvQQwtj4NZZHnRd5TYG+zCaqYMix5G+wbQKsjS/LDXLwT/fNW/B7odMwD7Luo4AnXLE3gS+ZaZytBaIOl3AQfa2+gu9knoi06jjgSFXV/5YdGsR17F6G2AKi5xdf/1XNgAORQ7fOdsC++GnSkq6LfwQetwYUWBgGW7ydqigu2wqcXS8FwEKjNZ+5Soy3yKpLiYI4Nmw2ekDnZWGDsRb1WLboZ/PepHrF/yrD1pJNQQeJxxwj75ZLUrvXz2Ttna8taaK2me+ZSzyBjbDWWlPEkG9wT3KdtVwFT9gXPe nCTuRcW+ jXo2+vOhSdPQ8KaOCGsyd8JjdUwwHD9UgqRf0Whn+d0GEwDdJD3vF0PRKcmldtlEzWNINzQPTZljuFKTuPvKrLXbq8rdcKARIb8WYYsj0tcFR86ar0kWhF9o5yarU+Hyy96MMnAieDIeG2mkZs1hYd8Duag7EPeNCZJwbPDUKeK+QCkYEtCT9HhMGOgdsFYO1S2fXrP5vmE8ClpwHC86ss8gWQ7Ym8kb3MXZZ7E/NEK2HiWvy8tKiE+Vxo1W7Km+QAdk/ouZ7InHSsydmPNb/cMv/NzxHfP2l3hJpzzvR3j4YvrBRt/PtuddbeGb2rX6qUVJM//Qo0XIZY0EUq9c10XVgcaOE9dXfk3kjrFy29id7NPo= 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 Tue, Dec 19, 2023 at 10:43=E2=80=AFAM Nhat Pham wrot= e: > > On Tue, Dec 19, 2023 at 5:29=E2=80=AFAM Chris Li wrot= e: > > > > Hi Chengming and Yosry, > > > > On Mon, Dec 18, 2023 at 3:50=E2=80=AFAM Chengming Zhou > > wrote: > > > > > > Since the introduce of reusing the dstmem in the load path, it seems > > > confusing that we are now using acomp_ctx->dstmem and acomp_ctx->mute= x > > > now for purposes other than what the naming suggests. > > > > > > Yosry suggested removing these two fields from acomp_ctx, and directl= y > > > using zswap_dstmem and zswap_mutex in both the load and store paths, > > > rename them, and add proper comments above their definitions that the= y > > > are for generic percpu buffering on the load and store paths. > > > > > > So this patch remove dstmem and mutex from acomp_ctx, and rename the > > > zswap_dstmem to zswap_buffer, using the percpu mutex and buffer on > > > the load and store paths. > > > > Sorry joining this discussion late. > > > > I get the rename of "dstmem" to "buffer". Because the buffer is used > > for both load and store as well. What I don't get is that, why do we > > move it out of the acomp_ctx struct. Now we have 3 per cpu entry: > > buffer, mutex and acomp_ctx. I think we should do the reverse, fold > > this three per cpu entry into one struct the acomp_ctx. Each per_cpu > > load() has a sequence of dance around the cpu id and disable preempt > > etc, while each of the struct member load is just a plan memory load. > > It seems to me it would be more optimal to combine this three per cpu > > entry into acomp_ctx. Just do the per cpu for the acomp_ctx once. > > I agree with Chris. From a practicality POV, what Chris says here > makes sense. From a semantic POV, this buffer is only used in > (de)compression contexts - be it in store, load, or writeback - so it > belonging to the orignal struct still makes sense to me. Why separate > it out, without any benefits. Just rename the old field buffer or > zswap_buffer and call it a day? It will be a smaller patch too! > My main concern is that the struct name is specific for the crypto acomp stuff, but that buffer and mutex are not. How about we keep it in the struct, but refactor the struct as follows: struct zswap_ctx { struct { struct crypto_acomp *acomp; struct acomp_req *req; struct crypto_wait wait; } acomp_ctx; u8 *dstmem; struct mutex *mutex; }; , and then rename zswap_pool.acomp_ctx to zswap_pool.ctx?