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 71D5CCDE026 for ; Thu, 26 Sep 2024 17:20:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D6B8A6B009B; Thu, 26 Sep 2024 13:20:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D1A436B009D; Thu, 26 Sep 2024 13:20:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BE1D86B00A0; Thu, 26 Sep 2024 13:20:38 -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 99E856B009B for ; Thu, 26 Sep 2024 13:20:38 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5385EACFA9 for ; Thu, 26 Sep 2024 17:20:38 +0000 (UTC) X-FDA: 82607553756.21.844CBEE Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com [209.85.218.42]) by imf03.hostedemail.com (Postfix) with ESMTP id 58EDF20018 for ; Thu, 26 Sep 2024 17:20:36 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PDJHdoEE; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.42 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727371219; a=rsa-sha256; cv=none; b=YMdXlcUo86fJ2Svzzwkwr575mcHqTBtsjBdf595QRuaIAsfciBNSkH8onlAgw77KjKdQSC yF3SjXh4qQgTEK7GQVj705mrr1bLY05kaPNGu1MmdDydIF8NxBFNOSAqgfXP3ZfAM28Hov N1Ttd+CnCCUAj04vJUaJorWnQCaW5Cs= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PDJHdoEE; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.42 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=1727371219; 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=fpb1W6vwxoW1hWsG2h/K4P1/JC6iKH9peyNxcKy1+dU=; b=KfsHe1sS5pIQbu1Zqfrd9PXlAqEMKkchYNqwQgnTCo2694maxWuLcYE4hIiB6HR5nFyyyA V4zIT9X443aoh8T4+drtkXbb/76iavmn8MjYgu6vAhJn7RMkDY3gsPGbX9DKLpQdFiPpfK spLeKgPCWTMdnbF/YsRjdso97oxKHCc= Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-a86e9db75b9so196398266b.1 for ; Thu, 26 Sep 2024 10:20:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1727371235; x=1727976035; 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=fpb1W6vwxoW1hWsG2h/K4P1/JC6iKH9peyNxcKy1+dU=; b=PDJHdoEEk14HQq22e5Op/UUlmANDxQ9gHGtJN1eJKlkzK4fb78FI83jl9KpCUcDOQw iue+xgnd223SKiylVucHzOW4z2J8YG4MqLu/jYjO7898a9jqNvt2/CUX4tBe8NkwZ27I 0NNH/etYB5wIbT0uoSG1qnYIymHQYmW4/HVx3NRFZ+k0DZpVTVU7BHHZ2rbJF0GSkO2N 2r5l70ln6jy5I4imwtG9rNYbmNbQmqzKqOrdDIvk/+rPq1ro2jxw/z5t0Lisqqc9Ob3s fXV4drrQIbOv7pI01G9ynLYjUadSgiDMe+VhOEadX8CEW3v+omF/q8nfI2NvI3YskBsF qT9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727371235; x=1727976035; 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=fpb1W6vwxoW1hWsG2h/K4P1/JC6iKH9peyNxcKy1+dU=; b=UUKFLNPEzFpORC7U02LYslpCPJ3C802gRdBXOTzlmg5ZcblI7q4kSCBxFW37/091+o ltR200cEKVjOSG5eOND8WQe72fsbVC/TjqL1RbFxopvZHIJpMmvpSbHfTORZXPNsILmV P3NzaQ6qJjfl+bsXaxSIb4+zLCV7lTZ2GaPbgTHnYdgMWxlfiKUxiTqQ4qvhEsPS6svN OTRftWzHeK8vLp0GFQnlZYD6VYv/jce7XXav0vCZvJ3J/IYlK3JtMeeXecJxfU0orJWW EqY8CLlD2dVPdmnafzwecBcaTHzuP31Qye3dkMA7Oh/HOAg7590sy3XZKWx0wGRTdudO RK9A== X-Forwarded-Encrypted: i=1; AJvYcCWZCVC+TMP//+w/hviVQlHI/I++beazHwnZdAWgLyCAJg6hKby6MfJDLfKEdQFuBgVLzVQXo+YnVw==@kvack.org X-Gm-Message-State: AOJu0YzIk6hG2fy7Oa7CbkcLtiQba+Xne28fduwZzUG17fSv9G9oCgDu WUo64EBAAv/+5GtJRRNnF5GITyhNFgHNwEsyJY+ouL3U4RaYTUuI3AW7NMQO9tv/WqMU5TXi9sj oUT/AcSFGN7Gm+re6tbSHiRKPoF+L7H9lMNwndgs3GqUzYKGb8w== X-Google-Smtp-Source: AGHT+IGJ1h2EgjDzUQwlEw1o2SrgKLQprJ5V21GaPR7Z0lFepQ5iNyhr2ykAWOfMLdYEpmfRWp90wQbonNZq0VhBdAc= X-Received: by 2002:a17:907:7f28:b0:a8a:8de6:a605 with SMTP id a640c23a62f3a-a93c4908226mr24113166b.17.1727371234479; Thu, 26 Sep 2024 10:20:34 -0700 (PDT) MIME-Version: 1.0 References: <20240924011709.7037-1-kanchana.p.sridhar@intel.com> <20240924011709.7037-7-kanchana.p.sridhar@intel.com> <20240925134008.GA875661@cmpxchg.org> <20240925192006.GB876370@cmpxchg.org> <20240925201323.GA880690@cmpxchg.org> In-Reply-To: From: Yosry Ahmed Date: Thu, 26 Sep 2024 10:19:56 -0700 Message-ID: Subject: Re: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in zswap_store(). To: "Sridhar, Kanchana P" Cc: Johannes Weiner , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "nphamcs@gmail.com" , "chengming.zhou@linux.dev" , "usamaarif642@gmail.com" , "shakeel.butt@linux.dev" , "ryan.roberts@arm.com" , "Huang, Ying" , "21cnbao@gmail.com" <21cnbao@gmail.com>, "akpm@linux-foundation.org" , "Zou, Nanhai" , "Feghali, Wajdi K" , "Gopal, Vinodh" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: cbcuwe3nnwworr9izrz578yisdsteaq4 X-Rspamd-Queue-Id: 58EDF20018 X-Rspamd-Server: rspam02 X-HE-Tag: 1727371236-644674 X-HE-Meta: U2FsdGVkX18cSEkSV5mwS2u38PmkkhVPi5oxFzbcp19SKZ3ny4SD2Q0Xvoi1DcDXN0QFmvI8uPpdP2Sps6pXm4xR7YFPscMh41lgC+c9oSA5rQ4db6VeSy3lOUZFW7/NSr7lmo4G9dVqsK01xSXleRWaJJxRBFceL94UmouwKB6jT6nemFfeBo6JlPthr0jnKowHI3fpJ/rtgcTVbvgwIcQiWY3KA/87vclBPnOJ6X39azYbFevhZ4tqvq1iguxiHQtkaSOGv4lFwdyIZLA/0QxueZc4YH7LRlwgLgJZwHWi2r8HS2VLdLUmtVJ/hYhxIObg8Qisz6sJq7cF9L/n7jGCyaTwYa8rASrl3ggVjQvXRvYpwUQPzX5r7y/9PjMfF5AIsaOis2ZkFGlJn1TR6hT+cbOW0GtY2tXK85gtzOWCwt6BOHGzqP+B59MSwEof93Betjjjh+JFNnSDVaQugHXIviFvGsFhYq0qkidjIJAhlurL/zg3PPmEoyDBQew9tCYwyKNBhkqnrUeiecGOgFRF7+YPU6JkI/kzdSvCopvLrZZbsj5OK3inkCd9YdYOVcp3qnEeDF9r8VOYPWvP2H6ETlXTejlALqFKS53SRLVoz6qHt2SjF4Du5kQrQEYNQ9Vg8zPpQLLcNo4OQKALKuZ1ECx1m0sOznDhRtoRs8sr0mnkaxc/d4AxmVW6D0WJLKRQGqsev9eRl+nt4JWpReDKOBWxHxRfE5oXR34IhMAcaf/S/lGBp9PekWJqvL5rQ0YQenW3XsdEvqDTxw/34AmDe7JnUYk0R8AtnXMeWlXtKISGCMNonBixcoQXoX0IFplIEUzic1FFxYcy28nPIPZ4Gag0tHIm+tRLwUFGYvlju3f957e1YJ6a2x3JiVVHrumtOYT0XtAPJJZav8p2ZEL95Pq9MR4PXPpG5R4JEmndjZuSMkQzyvt/6Gxbe6heEdRsYFoGNc9l4K0pXfb fMWht9e2 fKuW62EPBg0uz8SKwNPcLARIWygcp8wdSm5hP5w0DVyFastFP0eTkqpBXbgKfbDhciCchpYqfSTTyIZ65l1mALiCQsaQ3pWXMKDfKCZiDwtm22OZBRvSZ0ZGGtM8K3D2ALOASy8BLfNvV1LnMbXS49+5mhRJ5DT9qJv4tOMrVM85G+/00IYEJeS0BC2h4KyGdR7jCEYpQwFqeDheW3HhuRKe2nJ2O9AvXqOWucHReNRluLuSTfqbeyQ9VOznJFxiwuVCaCTCQVuPJkgDmFFqGSaBuoU5b0he2q6i3GXs4JsxXcZp8vYwJq99fN8mKDIaYvdUQtO6nxLtMzDjl7DRyaraIiUfd60zBD2M4WmbfRY480BWD3WhkIlP4B5rswy1aBW9UGE7fr/ULkE7dw83qgA6npPGkIe16OEPz/+wmza3Fa3IJq1Adm0pl17ZpXBdsHRffZRlWSqgj6E9QAeMqBx+WiNhGVFoiEWiVz4gdjiCWXvQb3rmKHXEnZoQQTajWOxCbWFMGbbUQjWjbXpgqqZX6hWsdvQS2Fo9Dr3511yBmpK30eqhWMxAQBtUu1ZbYPiZXbNR5lcYsjYrXjfdyn95mMEcvt9/Jtc1C1G2TXe2PRMQ= 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, Sep 26, 2024 at 9:40=E2=80=AFAM Sridhar, Kanchana P wrote: > > > -----Original Message----- > > From: Yosry Ahmed > > Sent: Wednesday, September 25, 2024 9:52 PM > > To: Sridhar, Kanchana P > > Cc: Johannes Weiner ; linux-kernel@vger.kernel.org; > > linux-mm@kvack.org; nphamcs@gmail.com; chengming.zhou@linux.dev; > > usamaarif642@gmail.com; shakeel.butt@linux.dev; ryan.roberts@arm.com; > > Huang, Ying ; 21cnbao@gmail.com; akpm@linux- > > foundation.org; Zou, Nanhai ; Feghali, Wajdi K > > ; Gopal, Vinodh > > Subject: Re: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in > > zswap_store(). > > > > [..] > > > > > > One thing I realized while reworking the patches for the batched chec= ks is: > > > within zswap_store_page(), we set the entry->objcg and entry->pool be= fore > > > adding it to the xarray. Given this, wouldn't it be safer to get the = objcg > > > and pool reference per sub-page, locally in zswap_store_page(), rathe= r than > > > obtaining batched references at the end if the store is successful? I= f we > > want > > > zswap_store_page() to be self-contained and correct as far as the ent= ry > > > being created and added to the xarray, it seems like the right thing = to do? > > > I am a bit apprehensive about the entry being added to the xarray wit= hout > > > a reference obtained on the objcg and pool, because any page- > > faults/writeback > > > that occur on sub-pages added to the xarray before the entire folio h= as been > > > stored, would run into issues. > > > > We definitely should not obtain references to the pool and objcg after > > initializing the entries with them. We can obtain all references in > > zswap_store() before zswap_store_page(). IOW, the batching in this > > case should be done before the per-page operations, not after. > > Thanks Yosry. IIUC, we should obtain all references to the objcg and to t= he > zswap_pool at the start of zswap_store. > > In the case of error on any sub-page, we will unwind state for potentiall= y > only the stored pages or the entire folio if it happened to already be in= zswap > and is being re-written. We might need some additional book-keeping to > keep track of which sub-pages were found in the xarray and zswap_entry_fr= ee() > got called (nr_sb). Assuming I define a new "obj_cgroup_put_many()", I wo= uld need > to call this with (folio_nr_pages() - nr_sb). > > As far as zswap_pool_get(), there is some added complexity if we want to > keep the existing implementation that calls "percpu_ref_tryget()", and as= suming > this is extended to provide a new "zswap_pool_get_many()" that calls > "percpu_ref_tryget_many()". Is there a reason we use percpu_ref_tryget() = instead > of percpu_ref_get()? Reason I ask is, with tryget(), if for some reason t= he pool->ref > is 0, no further increments will be made. If so, upon unwinding state in > zswap_store(), I would need to special-case to catch this before calling = a new > "zswap_pool_put_many()". > > Things could be a little simpler if zswap_pool_get() can use "percpu_ref_= get()" > which will always increment the refcount. Since the zswap pool->ref is in= itialized > to "1", this seems Ok, but I don't know if there will be unintended conse= quences. > > Can you please advise on what is the simplest/cleanest approach: > > 1) Proceed with the above changes without changing percpu_ref_tryget in > zswap_pool_get. Needs special-casing in zswap_store to detect pool->= ref > being "0" before calling zswap_pool_put[_many]. My assumption is that we can reorder the code such that if zswap_pool_get_many() fails we don't call zswap_pool_put_many() to begin with (e.g. jump to a label after zswap_pool_put_many()). > 2) Modify zswap_pool_get/zswap_pool_get_many to use percpu_ref_get_many > and avoid special-casing to detect pool->ref being "0" before calling > zswap_pool_put[_many]. I don't think we can simply switch the tryget to a get, as I believe we can race with the pool being destroyed. > 3) Keep the approach in v7 where obj_cgroup_get/put is localized to > zswap_store_page for both success and error conditions, and any unwin= ding > state in zswap_store will take care of dropping references obtained f= rom > prior successful writes (from this or prior invocations of zswap_stor= e). I am also fine with doing that and doing the reference batching as a follow= up. > > Thanks, > Kanchana > > > > > > > > > Just wanted to run this by you. The rest of the batched charging, ato= mic > > > and stat updates should be Ok. > > > > > > Thanks, > > > Kanchana > > > > > > > > > > > Thanks, > > > > Kanchana