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 45EC7C7EE29 for ; Thu, 8 Jun 2023 17:05:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BAFFE6B0072; Thu, 8 Jun 2023 13:05:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B5EDD8E0002; Thu, 8 Jun 2023 13:05:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9D9748E0001; Thu, 8 Jun 2023 13:05:24 -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 8A1476B0072 for ; Thu, 8 Jun 2023 13:05:24 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 4F36C40398 for ; Thu, 8 Jun 2023 17:05:24 +0000 (UTC) X-FDA: 80880206568.11.5EA6E40 Received: from mail-qk1-f177.google.com (mail-qk1-f177.google.com [209.85.222.177]) by imf15.hostedemail.com (Postfix) with ESMTP id 3DF9EA00F0 for ; Thu, 8 Jun 2023 17:05:01 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=ieRoR5nN; spf=pass (imf15.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.177 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686243901; 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=bbEH9BlfFfJdgMptRvUoSctdxcOmDDazNQWd/NtfJrE=; b=7CuzddQVBc1mCUJDDuRXYJmsrfoSvH/ho8HQHmt+ff5MofRYV7T/b0d5nREKSqPvBm16rS pBBhibqp5k8rlE76Qfjc/Px8NpR2jbPOQk8Pb9pmNgA7bipwsiKNRbR3ekyxjIMNc4kO+p dlXbfuci/2Vb5GZelPvl3O/ZD0ij8E4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686243901; a=rsa-sha256; cv=none; b=XuLNICzExwetKbpb+ytD4b2J8Jlsrw2afIvZ2Kl/hwdY6HxVm58TgZQ7KNSXzZbfZTlYyv y/MIh6KKg6LH3dTbnPbRwTvK+R4D++2bRBE3stSW2dD5uFRoeF6RNoak2jy0hgYzglnXdz 3atiWpKpLZr8SkWbg/Vu0Aq39q+VO/M= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=ieRoR5nN; spf=pass (imf15.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.177 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org Received: by mail-qk1-f177.google.com with SMTP id af79cd13be357-75d57fdb014so73573585a.0 for ; Thu, 08 Jun 2023 10:05:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20221208.gappssmtp.com; s=20221208; t=1686243900; x=1688835900; 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=bbEH9BlfFfJdgMptRvUoSctdxcOmDDazNQWd/NtfJrE=; b=ieRoR5nN/5HPTyKVwHVP0BEkQwQ9wvNsN/zAulSWc0Ol/uAuLEOAt1Z96gN5qNzHgN L+U3ldxPkY0ECJuef6B1DqX8drxR+O6z4wkfNtNSG01QHp5+oIRQdRSLC0vdZ+E+1M4c x8CHi7PvGPdiSJ8gPACnRHDoPtORC+XlKHr2fCq31foty1QbPWZdR7ZYa1+mG6lbsuRh pPEX427saRnQjrTZQ3PLdlBTPgRDHEVf2EdnG8w1ApcjjZBt877CInwG+LVHv0F/an6l J7YPOVt/EwGjmiYPsiTXgRsh9W9Ms92rriCEw4knegjO7MCrNRBmwTbgMm9RtVfy52c2 MnBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686243900; x=1688835900; 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=bbEH9BlfFfJdgMptRvUoSctdxcOmDDazNQWd/NtfJrE=; b=aXkozjd7SihbFlbCBxf2FphM42TY5/q/mKW7LjDpEUge9CwSBTxHonybFJ4y1eJZkP gDThH3rMKjYL9M72RlFWIXXrCbSkOBBRM8ui1Ug/Myd32G2IJ0RiyxlDNKUzquC/sfop Nj3djbx6yD6gV8AZ5eOS+GHtp76aE/8mVsiTkxivFGdh3iqIpLBhte8bsIPgatR8nDTQ JTqCb8Oj0LsqMTQTBAzC+kj14oW/8HBuDtEfaG+JIO1FkG7j5MsKPgnuRC3hF4QGh/yq EWvv5r5sh1CVyX2hfzUlLKKN8YnxvXWJyrog4lW6gLK+MJBzjyxcZKcuUV8lb2sgwrtU ePeQ== X-Gm-Message-State: AC+VfDwSQ08dxA+oJmlQo8sOppByxqUHGS4MFCRXGWJsSRzY88nGN44g HjW2fLWMTQLRR8MLd9+hUC5Ehw== X-Google-Smtp-Source: ACHHUZ6rLRRG2RURXpCbACQUa/Op54Jl3Jbm9YOJq15oChEqeuhuXJFYvOGy2n2MGgHh3PE4t1DtAw== X-Received: by 2002:a05:620a:25cc:b0:75e:d8db:5e4 with SMTP id y12-20020a05620a25cc00b0075ed8db05e4mr6668037qko.49.1686243900236; Thu, 08 Jun 2023 10:05:00 -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 pi48-20020a05620a37b000b0075b053ab66bsm453084qkn.50.2023.06.08.10.04.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 10:04:59 -0700 (PDT) Date: Thu, 8 Jun 2023 13:04:59 -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: <20230608170459.GH352940@cmpxchg.org> References: <20230606145611.704392-1-cerasuolodomenico@gmail.com> <20230606145611.704392-2-cerasuolodomenico@gmail.com> <20230608165250.GG352940@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230608165250.GG352940@cmpxchg.org> X-Rspamd-Queue-Id: 3DF9EA00F0 X-Rspam-User: X-Stat-Signature: qy66bx4byb6rdjh6th87cjxyusdo49pn X-Rspamd-Server: rspam03 X-HE-Tag: 1686243901-567650 X-HE-Meta: U2FsdGVkX194cBypcVjGOJ0hxVXKdj6ojJ7iGuyeprXE5RctqJCwIMj8DW70kRH+2XZ3RQiddgtPWPs7im7v8RvDjLZtxR8008ePWSUiFoOqrd751HfqZ54+jfbuiUNXTgWQvMq9o5xfGJdQvg9HLUCcDRpVbFyVgI9+BVE38l3kJvh88ptVTwX2b7DhXlgOa2sCklzFhFqq+LdAL1tZzexMqJcCwUIuy8k/MGylSUYkZ6MecrepADy1ahIYH8FEwdGdG3FfkuJ2K8Xrkd4eEuD5R8hovxdErB0TijR899zcxzO5W7OpPC7zjCwAe1SsHK1Hgaw8opULCZAGlrwwdA3GYyfr+GtUlP3WwUdFYH/uvMlkNVYpXqfqtqz+YNR2maKZmcEz6P26R4/bUcjaMJy7R0snArszFjFDXMOy6BeAmo3zwgfm6Q3ouKO5pb4o+TDgBpO6I5r0NXmawwVG3ijS6mMPrHiZj1UIVnIIW5iGigLOZpdeh7gx0uVV2kMY7nFSBz4gsfJqdmPbZai7ZHlvSIkiUGnZ9Wb0fVeT/yumJi1Q+ElV43YB2z2IxC5mcgKQd/V4Z6ykA5ULdf8K9qh6PU+g9rIR7qQYowzzaILjYvH7dMAy5SFpHABfPv0fYuZT/SHNUde8SA6EQniOV8u3P+12PVI3WlLbPgjyCqz3yT7wv4DCFUNt4Q9n8Gw3FG6djkmxv/Zp7TR1d+yoJjyaMjrPKqPTunYAN5CwgQtXxM12jhkzWjnlkGKJvQWFnRNnuHgB707viYrFCLLhtNemsFY+fv7r4TpryMqId3I0bbuyBAsGXsh8/1i7TmSTlQ6eIgyDIIIhc/LgMOAFF0SKg9h6KDoTLWsaCPoN9uVFth1AANkIPuztL1bwdcAKJuFxzB7X1a42qmzkuVKoQNwAVE0GvFEmGfJA1zkeTuYoeZP4ypF5FVgHf9rOdS9oIEyxB6LFkK7OCoQjO6u Abg1QP91 Leg02F+adnRqR+257IuIrHPkJJ9Hq5Wgwzt7nNMXhX/ZjDw1c8lc00sHjrnuOIqljn+Hh/OwwXXNW5gLaOfXGPjFNRFHxUOBT/1xSZQmuf9pgQc9alVhPKPLvrCY5VvRbl/QiHCa42C1KdTXQdIh6YaIGoadVDvuj05grNebfX5U4iyZRtyzFxOLRL+kYs9W3efgCaKA+RxkgKxOZzyyV6edJgpa4UWbuX03ne2TsWDc37W11NaD0EZKNTmvf8PKMmRNeTEC58vE2hEHdspKERP6bEHPR+P8Bb3YkJ4HEXymC2iH2AiT3yPxPPy0W9YDsUxMjEz/WFuLjXC8lt+2HaI0pZKho9rxTcGr3K1x8DsQ41ijtCA+2My92Xz4wBkLcu8kQ39H8bdefjvc= 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 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); ret = zswap_writeback_entry(); spin_lock(&tree->lock); if (ret) { put_back_on_lru(); goto put_unlock; } /* Check for invalidate() race */ if (entry != zswap_rb_search(&tree->rbroot, swpoffset)) goto put_unlock; /* Drop base reference */ zswap_entry_put(tree, entry); put_unlock: /* Drop local reference */ zswap_entry_put(tree, entry); spin_unlock(&tree->lock); return ret ? -EAGAIN : 0;