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 BF9E8C7EE2E for ; Fri, 9 Jun 2023 08:40:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 481C58E0002; Fri, 9 Jun 2023 04:40:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4327B8E0001; Fri, 9 Jun 2023 04:40:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2FB6B8E0002; Fri, 9 Jun 2023 04:40:10 -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 2085C8E0001 for ; Fri, 9 Jun 2023 04:40:10 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E4CE980177 for ; Fri, 9 Jun 2023 08:40:09 +0000 (UTC) X-FDA: 80882562138.19.E9A91EC Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) by imf07.hostedemail.com (Postfix) with ESMTP id 2AD344000F for ; Fri, 9 Jun 2023 08:40:07 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=MHb8l0Eg; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf07.hostedemail.com: domain of cerasuolodomenico@gmail.com designates 209.85.215.172 as permitted sender) smtp.mailfrom=cerasuolodomenico@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686300008; 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=EeBp3LDqacG6dkXx6JUslaCBZFzIXO6+G11dxA8Ke/s=; b=e+FzSBCMdaeq/GvHHPfBdetYJVXPCj106vjbFcLjGiFB8njtqPl/CrkC3KMUCtfkqrSbT1 DxZYe0m8tptmgnlOSu3QTpDbTqYh1vO192CO7ZDlDBuwrRDhBdSH6fdIogjOg6dlLVu9y8 Bt2Us3VIKloS6+in8FM0S9DBnBhur78= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=MHb8l0Eg; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf07.hostedemail.com: domain of cerasuolodomenico@gmail.com designates 209.85.215.172 as permitted sender) smtp.mailfrom=cerasuolodomenico@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686300008; a=rsa-sha256; cv=none; b=E1z94hxiiglF8PY2GCjl44bfha/k9ThPulR8CbLHlneeJ5dzJ32rC2ifM+YyYUE4q8nRZy qKbBQ0uG4svMUCbaH/U+CsDAvCZQvzqW4zQTj29Sgvk51+qycy7uupEcQ/Bek9GlQ1t1QO 2bvOTmqnt36cFgSns7O3xaCEQdwnZWY= Received: by mail-pg1-f172.google.com with SMTP id 41be03b00d2f7-543b17343baso437585a12.0 for ; Fri, 09 Jun 2023 01:40:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1686300007; x=1688892007; 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=EeBp3LDqacG6dkXx6JUslaCBZFzIXO6+G11dxA8Ke/s=; b=MHb8l0EgEXMRA16v/paq8cbTXztgj69apT6QbfIrMBjssZlOUe1Ik+GX+KX126LiuK XRYVxXNM7FFT0HElUPko6MT9LwPKCYeltHPpDTnSYKzFQNql2n4EE/T4uanuEvP8iDIk ksn8WJeaGIVaQqip0ntgnb8plHn4LWbhG3hxty+4g0LysUsiIMkkNKRYEGju6ZK+7A12 RKMxrMGwlv5UHg32EXkh40IwTEVRq8EPGBWWVUkh31Pp/X40z7Ph4yQdsHqPqlXR0SwQ dia6IkrMRFpWxFdpO8SPTIIwiD4kg4iYRK8cPvdkVLOuJgv2O2E41iiLQgfpaAWlr7pc vr1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686300007; x=1688892007; 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=EeBp3LDqacG6dkXx6JUslaCBZFzIXO6+G11dxA8Ke/s=; b=JJEu9Q7t+y63EMe82JnbwnDdqstEd0UY3e6w8Sb2HsuPZ2zSVQVYBDZZwpVSXM2tgo 8ge4tEH8s2Rm8jdXIFIOBDS4+/mAUM3n0qEqTHGzMYwhYCmvsBuEAx87waKyGDDLYjzi BWvzmnIDy3Ab8pJa5esGMm4sVBuRJTbjJdX/RNFYv8tub0hmqRIjgIJZslgYMlue+9c3 FLK+Ghng4uf5aVZ/gCeJyISE9kFeQmhVW1ZXLeoDV3AROyx0sYE8eoMFn+y1oO2PL3TC TN/mzEkhmE2Bi12/PfDrnD7txJ8DWNKo51UPfBX0eycTrcepxP9iAk6y+/hlznBgkVtW EPXA== X-Gm-Message-State: AC+VfDx/xm6FEgbQyH2k8+USkrEZUvNq8YlC8kreD9wBcVOKi4mcSQP4 lC0bqTj85iRjRb4jc9r68e5k0FIx6ZKbqd51eac= X-Google-Smtp-Source: ACHHUZ7bJ3jCLg4oTcEEJ/UTneBwOzhQYRw5k4G6xp2b8bpURPESSI8BdkY4VNnmPZ4i8w5L/9c5mY+GdGriQucAym0= X-Received: by 2002:a17:90b:4f44:b0:253:727e:4b41 with SMTP id pj4-20020a17090b4f4400b00253727e4b41mr370270pjb.34.1686300006768; Fri, 09 Jun 2023 01:40:06 -0700 (PDT) MIME-Version: 1.0 References: <20230606145611.704392-1-cerasuolodomenico@gmail.com> <20230606145611.704392-2-cerasuolodomenico@gmail.com> <20230608165250.GG352940@cmpxchg.org> <20230608170459.GH352940@cmpxchg.org> <20230608184516.GA356779@cmpxchg.org> In-Reply-To: <20230608184516.GA356779@cmpxchg.org> From: Domenico Cerasuolo Date: Fri, 9 Jun 2023 10:39:55 +0200 Message-ID: Subject: Re: [RFC PATCH v2 1/7] mm: zswap: add pool shrinking mechanism To: Johannes Weiner Cc: vitaly.wool@konsulko.com, minchan@kernel.org, senozhatsky@chromium.org, yosryahmed@google.com, linux-mm@kvack.org, ddstreet@ieee.org, sjenning@redhat.com, nphamcs@gmail.com, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 2AD344000F X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: un7hnsc4yfdxief11jh8neia96dkh85g X-HE-Tag: 1686300007-92344 X-HE-Meta: U2FsdGVkX1/pnTuvhqaWmaZlLkZ6oHxe9IKhjzDNRcGUsUmP9VCE3+yEBXsX4HrKgLJnB2Pjc8LbgLP0eon9oMEYe0prFv288J0RhxITe2mMr14aTOLMjQJ5NdeZ8AMNiP1Cl5fiu3pEdx/svfTKZWl7diGMkW7LG92fLVqzXfblzxKf38uK7vALBOcrZ3gBSryydzzMrDWN/nNeLdRl4iZoDn6O5rfvhGdKv+GGdbilip+ctSB29SCLeQEgoMOsMBQ4xY9o9J9INANbM605wngrynBqs/q9aeBSdtqQpofqJHUzAjULSy1HIX7K9SiI4+7l7cXQ2r8Yngc7ot+sPFlzXeolWpPeqkSaPl5mjL4thWSFmNGHvl91Dr5MJif+Dm0oLe+KlozQDNyLvm9YXFOR8sNWZynjds0IKuAJ3X87lj9OCkR6rSlhYZv0DAATSWfJzr7Yf29Ms3QHgDQvEgKArU98mYm/Cg7OvDzDuC+CXjdkxqoNAwYJbfK1cMIfkXIosRph8ox9oJa5NfwgVrAMSnGuHO/XkmGdS7npAITxb5S6DTlVoO+63Mjxk4DTmMP3kiAojFzwaIkB5PsIFint1ze1bmVAvJeBf87IptJa15mhhso6o3D+HBeIMjP337/tV0QFb8XjL/RivxN+2mgVLgI1zlz/yMIhp2ujtvaxFbw6g5MNG6bDJ7wdLRvOuwY/81NmgiIm6WtIM9f2HJ/80Uw7nJ3hJ+V5htfVZ/WMb7IqyT3FHokYy5Cgt1L83N7HfOCILGO/Tpeq3PrRSY1sEfpCGpjFgAOw9vJ5NymagAzzRcQ+Jy8XYVTIm8n4HNJScG7vWvmhStMPwc8biFE/CqEmw/wmnbp3rjx2gsbJa1soNtuMAWftLg13M3lhEfzvGLoYJ/Afzq+mxzhSy67QEu4D4kJVBS/8WVbLv9O74sjUAXj4BAOD6mvbosjNcT7V4PDPVIeaEpNa+6l SVrunair su69aJG16+yZGjPKtGMZAtam6WCois8h5swsumJkXHYh4AagAexxGjzHs6LewABI/aXbcSk3eo2yUY+YC4Arwe4BYWlzRHMLVc9DhA8cupaa3DZCfgBwLQ+aadpLdUtl1jFzd7OY3ivWgxwD2lEkVtSeuG5grY1AHriWPmeRWOrqxZi5MS2dhkpf6vyV9M0+NOpaogARBMmnBS8fKp945qiKPNnk9TSsRBE1FK60LCscCz3kBfqqv+ljEgb4l4p4UXElJcxe+g6MAsymhw0LwKa927b463gB3SaU+zk94JQ6Jb8o3BGm2gvyFOow8mDWwG3DI7NW3hkLutujTcEi6Y//CmK7r4kCVXZvhY1ts4Btpx38rqM/5nRNy95i1uTs7uSpzrpi+TgyYbwDUkQxCHky+6zBa+fgjiZO7 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, Jun 8, 2023 at 8:45=E2=80=AFPM Johannes Weiner = wrote: > > On Thu, Jun 08, 2023 at 01:05:00PM -0400, Johannes Weiner wrote: > > On Thu, Jun 08, 2023 at 12:52:51PM -0400, Johannes Weiner wrote: > > > On Tue, Jun 06, 2023 at 04:56:05PM +0200, Domenico Cerasuolo wrote: > > > > @@ -584,14 +601,70 @@ static struct zswap_pool *zswap_pool_find_get= (char *type, char *compressor) > > > > return NULL; > > > > } > > > > > > > > +static int zswap_shrink(struct zswap_pool *pool) > > > > +{ > > > > + struct zswap_entry *lru_entry, *tree_entry =3D NULL; > > > > + struct zswap_header *zhdr; > > > > + struct zswap_tree *tree; > > > > + int swpoffset; > > > > + int ret; > > > > + > > > > + /* get a reclaimable entry from LRU */ > > > > + spin_lock(&pool->lru_lock); > > > > + if (list_empty(&pool->lru)) { > > > > + spin_unlock(&pool->lru_lock); > > > > + return -EINVAL; > > > > + } > > > > + lru_entry =3D list_last_entry(&pool->lru, struct zswap_entry, lru= ); > > > > + list_del_init(&lru_entry->lru); > > > > + zhdr =3D zpool_map_handle(pool->zpool, lru_entry->handle, ZPOOL_M= M_RO); > > > > + tree =3D zswap_trees[swp_type(zhdr->swpentry)]; > > > > + zpool_unmap_handle(pool->zpool, lru_entry->handle); > > > > + /* > > > > + * Once the pool lock is dropped, the lru_entry might get freed. = The > > > > + * swpoffset is copied to the stack, and lru_entry isn't deref'd = again > > > > + * until the entry is verified to still be alive in the tree. > > > > + */ > > > > + swpoffset =3D swp_offset(zhdr->swpentry); > > > > + spin_unlock(&pool->lru_lock); > > > > + > > > > + /* hold a reference from tree so it won't be freed during writeba= ck */ > > > > + spin_lock(&tree->lock); > > > > + tree_entry =3D zswap_entry_find_get(&tree->rbroot, swpoffset); > > > > + if (tree_entry !=3D lru_entry) { > > > > + if (tree_entry) > > > > + zswap_entry_put(tree, tree_entry); > > > > + spin_unlock(&tree->lock); > > > > + return -EAGAIN; > > > > + } > > > > + spin_unlock(&tree->lock); > > > > + > > > > + ret =3D zswap_writeback_entry(pool->zpool, lru_entry->handle); > > > > + > > > > + spin_lock(&tree->lock); > > > > + if (ret) { > > > > + spin_lock(&pool->lru_lock); > > > > + list_move(&lru_entry->lru, &pool->lru); > > > > + spin_unlock(&pool->lru_lock); > > > > + } > > > > + zswap_entry_put(tree, tree_entry); > > > > > > On re-reading this, I find the lru_entry vs tree_entry distinction > > > unnecessarily complicated. Once it's known that the thing coming off > > > the LRU is the same thing as in the tree, there is only "the entry". > > > > > > How about 'entry' and 'tree_entry', and after validation use 'entry' > > > throughout the rest of the function? > > > > Even better, safe the tree_entry entirely by getting the reference > > from the LRU already, and then just search the tree for a match: > > > > /* Get an entry off the LRU */ > > spin_lock(&pool->lru_lock); > > entry =3D list_last_entry(); > > list_del(&entry->lru); > > zswap_entry_get(entry); > > spin_unlock(&pool->lru_lock); > > > > /* Check for invalidate() race */ > > spin_lock(&tree->lock); > > if (entry !=3D zswap_rb_search(&tree->rbroot, swpoffset)) { > > ret =3D -EAGAIN; > > goto put_unlock; > > } > > spin_unlock(&tree->lock); > > Eh, brainfart. It needs the tree lock to bump the ref, of course. > > But this should work, right? > > /* Check for invalidate() race */ > spin_lock(&tree->lock); > if (entry !=3D zswap_rb_search(&tree->rbroot, swpoffset)) { > ret =3D -EAGAIN; > goto unlock; > } > zswap_entry_get(entry); > spin_unlock(&tree->lock); This should work indeed, it's much cleaner with just one local zswap_entry, will update!