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 29A84C7EE23 for ; Thu, 8 Jun 2023 18:45:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 806448E0002; Thu, 8 Jun 2023 14:45:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7B6678E0001; Thu, 8 Jun 2023 14:45:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 67F3B8E0002; Thu, 8 Jun 2023 14:45:21 -0400 (EDT) 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 5790B8E0001 for ; Thu, 8 Jun 2023 14:45:21 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 06770140441 for ; Thu, 8 Jun 2023 18:45:21 +0000 (UTC) X-FDA: 80880458442.04.3C692C0 Received: from mail-qv1-f42.google.com (mail-qv1-f42.google.com [209.85.219.42]) by imf11.hostedemail.com (Postfix) with ESMTP id F1F3E40010 for ; Thu, 8 Jun 2023 18:45:18 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=QyYiglSu; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf11.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.42 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686249919; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=vB1ola4E25oLoCTIaa0hZ2bCKEZyZNPX8kWPehiyVw8=; b=NOjSf2lIY07DIPIFLNH7T7JuRx+zwhY86IBZT0vPtaAWqC8/dXvEE0YZwoKbMJNfK304ib 5BzAoD3iPmhiYG4eW2q1+og8adg9fwB6LGch9mqo5HQnNF9PEmWq9ccy4Z/vjMc8y4B67G ogHZTgasFUQ6eIhcY+RqNv0mtSnK8eI= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=QyYiglSu; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf11.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.42 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686249919; a=rsa-sha256; cv=none; b=T+ToPZGx8HDlDpYpGRQL+26leOGLE2Q6trZUk3lRRvGDOnlClJp86ZScyVvfqy03bCmsX1 eDOKpQ4/Tvt1QLwTvzd9+T5crS1fYajoTjvnhtiSlx4KoFhpMPGcF59EFFnwpZ+ziqNYof HqZyFJpETkH4vo4UIJ+l+rXSkIE97+0= Received: by mail-qv1-f42.google.com with SMTP id 6a1803df08f44-626157a186bso14428236d6.1 for ; Thu, 08 Jun 2023 11:45:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20221208.gappssmtp.com; s=20221208; t=1686249918; x=1688841918; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=vB1ola4E25oLoCTIaa0hZ2bCKEZyZNPX8kWPehiyVw8=; b=QyYiglSuM6pfS/XYtSqOVAeCLqbULZey2IGYw6NDTVuF4d4pFXmQPAomMQSeLGpQ76 zmXaEGlXqnAvgUkfwk1BtThj5hm3fRmDgaHojc2IHEHs5gmG8S98LebzUFxlwb72UuzF zB2lQA464bh/1WsiDq90PVQYMBttECP0AbRMDqRtzv8aZPyyGJfYI0dxCe1PA3jpY43g Gi/jN32iMlD6csqkxr0JJkZLI40tKqsacMKBayzwHkl/05UQeR97/ZxqS7GTzYatkXYA tP5haPX5ZKd8GAyh87TlRnoqMzItj/BvoWgTfUxjY3mfdjidZQwnLOrE9R8lyXjU8gsQ WOLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686249918; x=1688841918; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=vB1ola4E25oLoCTIaa0hZ2bCKEZyZNPX8kWPehiyVw8=; b=caRzHfXeAnmPM7ZEqVkQrfvUGUuXXj9KjexCCB5Uj4/vj47CQz3UcNzgr7KsAhhKeG OqW4kqv/mAdNvax8VJAcjzTJxNj/YhpbQszNePi1+BQO3IuHnqHhSqME4XOh2RgEz75y ixNCqWhDUkXWCxzNWnlNGnAT+B6syvZ/DH2jpEJ7oxBxD1H8KP2ksje6+hv0ijqxXFWU uNHWf9BHPi1DoykB7Okc8rGArufEi5EByRpj9TDsTt2OQJCRiNoqfTeIl5PC9QwD2k0j v7K3KWyIKnkdbN6NJNqtkaY7S4ILRoYn1UBKGZlx4ml3Jcd5mf6dFVd23iMYURZ4Pjkm YeQg== X-Gm-Message-State: AC+VfDwGSxmSgqkT/7llOScBHZbvG6UWy1b66cmvwminYMy0jT/uttYC WZVhzOZCgjNrozVOWuDv7L4acg== X-Google-Smtp-Source: ACHHUZ4RGfCjbZ/Eww4hIR8wUeuQovzbHX1r3YXBGPpdAJEG1NRD5nknwVZUKoJJ6MdTAJYBPMF/Vg== X-Received: by 2002:a05:6214:20eb:b0:626:f35:ab95 with SMTP id 11-20020a05621420eb00b006260f35ab95mr2350120qvk.17.1686249917969; Thu, 08 Jun 2023 11:45:17 -0700 (PDT) Received: from localhost (2603-7000-0c01-2716-8f57-5681-ccd3-4a2e.res6.spectrum.com. [2603:7000:c01:2716:8f57:5681:ccd3:4a2e]) by smtp.gmail.com with ESMTPSA id kr30-20020a0562142b9e00b00626286e41ccsm589592qvb.77.2023.06.08.11.45.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 11:45:17 -0700 (PDT) Date: Thu, 8 Jun 2023 14:45:16 -0400 From: Johannes Weiner To: Domenico Cerasuolo 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 Subject: Re: [RFC PATCH v2 1/7] mm: zswap: add pool shrinking mechanism Message-ID: <20230608184516.GA356779@cmpxchg.org> References: <20230606145611.704392-1-cerasuolodomenico@gmail.com> <20230606145611.704392-2-cerasuolodomenico@gmail.com> <20230608165250.GG352940@cmpxchg.org> <20230608170459.GH352940@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230608170459.GH352940@cmpxchg.org> X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: F1F3E40010 X-Stat-Signature: h7whkrdnb8b49sp79ox6wmtejz9ygdyt X-HE-Tag: 1686249918-751640 X-HE-Meta: U2FsdGVkX1+cThFw6c0eauiY+acwjtPwSQyTWtP0AKrqjgthcctfTeKDvcLiIvpQveI9TI0SjH1B9iBMKIh9o6Zxfbvh+yu9vXCE9VHoKlPmkKzlfsIP+tQG5HzhUqCedERLcMzAfyF8QPRmZdWkJHlgcHfJXaZKCzcngSPhge3yGfSMnYBfoE20xTQcpxL429m1uZjOKNe1i5nk9l0PPvk2CO5qeEYNQEYIgHIM2ZnCOC8nlmooH0efZkTfa538ZvydKDgxApLcEu3Fe2r2aqElkqN4j/HnHyGnTZcM+6S7JVp0qaUWX8MKMHZukcdXlWlA4UxAOQCIKRumG49+rPUfa0eYSbLeDqEyhxyburJf/T5BCF3dJLkekA1NnaJek4sWBk1pyJ2yspLoC5GggjTQLukZbOwM6PQxsiHXHBozUKZPiqueyakDt1r0dIMeIr6/wGMQxFqE9FSUkPQL0Hjh9ZWxjvytJ4VAsuvhRuIhQ3218xckD5teSBScbm4f2mxxajMRsJOCiZXblATk4kSP+YfHSuo79jQv/pxiaHf3VRbqPF17YQt9HlmYENAhViB/N+rSyzq3ev/h+cHwW2EJbbUIBVUhzAzn4O9KtDUMptplg7aMWoKyCwGgJOV7nR5F8YikSWDR2JB89HQMOZjuM6OysXx1lSvbsNhbYbIuPnR63u9drhM2snhkqZtSWZ/fr8VWbmvWXgeOcWIEG4LkUUj3nxqLTfCc1Ll89Auma+eN+Q9KFajQAOqKeN0cJ2To8TXNIMuTBZg1cAsOVlV1wLzlZtlgztCnut6X/LFDkNy2igGAx9MoqA7aOxAYoYJ3GOoSmhFtg/U1CXZFeHI5LIsNb0ax8z5ejKWuK/lvwVxV8Bf25LIAZv8xl2Xm+pNwEP/+EZ/Eiw9YA6cdoL9I9KiKE6Ph2XPHan0KJ8uk2N3JpxwgLVNejl3wzFY8VIG561enC48KgOnZUY3 h4aBJQ6v K4JflH7NxbZAf8tt7yXC6ILV8Jx5ZTGbZ9gn10Egc3Qjqkfmn9ZLGsJKQ9cDuTwvX2rpqwTWYQSBWoeAsBWYvo33qAeqFs06LLI3ZcmwQ5H1ir9JlSOwrIwdWSnmcnLp5+vP2oUAZfM/BPxn3CUDygtkdLBMndco9bFGymokbal91V2S+TP9/ndqmYioh/xZtHLCNh2OVQ8Pyd5fk8U7hMCsJZ9f4zMPsq/jhxtCyaOtGT1J9pbZxeyHS89l95wcvqwIOj3nbAPoMD4qe5EUJp0s7+qiPe3eLLo7+TCdkORKpV4Dm3goLe3OdJE3MOVM6DAKadPj+DVyEgn8N/xHqMurc+Pger8gqK4vlpFcLp6SndfhfUzzZ2yw3Spo8fbf6JhM/KUGgQxaxdNU= 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 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 = 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 = list_last_entry(&pool->lru, struct zswap_entry, lru); > > > + list_del_init(&lru_entry->lru); > > > + zhdr = zpool_map_handle(pool->zpool, lru_entry->handle, ZPOOL_MM_RO); > > > + tree = 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 = swp_offset(zhdr->swpentry); > > > + spin_unlock(&pool->lru_lock); > > > + > > > + /* hold a reference from tree so it won't be freed during writeback */ > > > + spin_lock(&tree->lock); > > > + tree_entry = zswap_entry_find_get(&tree->rbroot, swpoffset); > > > + if (tree_entry != lru_entry) { > > > + if (tree_entry) > > > + zswap_entry_put(tree, tree_entry); > > > + spin_unlock(&tree->lock); > > > + return -EAGAIN; > > > + } > > > + spin_unlock(&tree->lock); > > > + > > > + ret = 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 = 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 != zswap_rb_search(&tree->rbroot, swpoffset)) { > ret = -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 != zswap_rb_search(&tree->rbroot, swpoffset)) { ret = -EAGAIN; goto unlock; } zswap_entry_get(entry); spin_unlock(&tree->lock);