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 DEBFEC35274 for ; Mon, 18 Dec 2023 14:04:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 836BD6B0074; Mon, 18 Dec 2023 09:04:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7BE588D0002; Mon, 18 Dec 2023 09:04:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 65EBB8D0001; Mon, 18 Dec 2023 09:04:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 4E3126B0074 for ; Mon, 18 Dec 2023 09:04:00 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 1C4DA1413CD for ; Mon, 18 Dec 2023 14:04:00 +0000 (UTC) X-FDA: 81580107840.07.325D5B2 Received: from mail-qv1-f42.google.com (mail-qv1-f42.google.com [209.85.219.42]) by imf06.hostedemail.com (Postfix) with ESMTP id 0F6AE180030 for ; Mon, 18 Dec 2023 14:03:56 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=c7HFmX30; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf06.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=1702908237; a=rsa-sha256; cv=none; b=1A4mfOFOlkq0TglcPX8AqaA2zZmjDHdwFqJ6d41MN2y4XYGtOrC3wLVrghCcGqyKiitNKh kGg0AKvIjeV1p+mo8zeGg+m/v3rqzVGSMEwBlOlDC3v2FSCoyg/Oz4G7Mk2Cji++KV18MP 4Xfc9x0pVYOEtnlpOm3y1ex1yveG1X0= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=c7HFmX30; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf06.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=1702908237; 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=SeIfmjTD20WCr2YilJIxer8YjFa9qmpum3ZmpdpwHyc=; b=MEPK8mle0sciQ5XKlwtMrB7Am/MdqY9xLT+TyRtjDOYbWOkTx8gfoS8sdR9Z8L+2g71+SA x26mjqHb4KFE2zwUFXIH6jcYIsisQZ0feyF1wKGge2c4orw5dIyS9l/HOl/hzZcZZSjuxR zzPkQuc7IqZf83k97s33OtET5c8c4hA= Received: by mail-qv1-f42.google.com with SMTP id 6a1803df08f44-67f47b15fa3so6219676d6.1 for ; Mon, 18 Dec 2023 06:03:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1702908236; x=1703513036; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=SeIfmjTD20WCr2YilJIxer8YjFa9qmpum3ZmpdpwHyc=; b=c7HFmX30vNd+gALJdLLH7D+HkgZ2iY0YEEg8JXPF37iHLBecVdqBy++hk3y/AycfGk 3hfnCVNWn680ABkIjoP5M3V0dI+c/K1zu4CS+PwZxWMlmFpsrrjsVDuqBv+ytAfjZj4w WaZ/vDbVqFEzVxRERZVRA9Nlfrk0BSiV/TJRPx4i6A0BY8550nR6JGlXiuevqOilW2MT 29OwrwxdnC/2V0kI9x4t/TYvukRzkpyKamvqeu+EGnrHt+w8s6rZN3aHlY5jwFSpXEQm 7egrLIvm1ju+mRbPcs+GjPVAu4h7rhPP5w+Z9tlfFZ7G0OOoYP8lG5sB7DLu9gaua6qm lnEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702908236; x=1703513036; h=in-reply-to:content-transfer-encoding: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=SeIfmjTD20WCr2YilJIxer8YjFa9qmpum3ZmpdpwHyc=; b=LUxPQ2jkLw5+mbSvj1cGARHg4ZWVg6dhomNFQiXhQNhFwM+qoYT8x5/cJwNlYzstzE 3Wc8tj33vT/q6qM2B7SiNOL9A+zgo6W2V01wZUw+zQycWbgI81hnpOcKrhIZolE/3uzb LOsdU+a0UUuBBEFBAU3qGWFP5l+tqTh+TYPXHXkFfzrUt2t2bdQsNMZJvc3ZlpHTOTuL i7B4UHAZyGlfNxJJGTFlbhW5MDsT9IgF7WT+s1lPawvXVUvB67OnCzUmby8k3vcavKx7 dDGL7/tGx5QiaM5sAUOutTpgV8JNh8nHNBDCrXC1x6xW93SwbLCqKjUKrNgBngA9FnZg cbxQ== X-Gm-Message-State: AOJu0YwlLrUjNN1K5Bo+JSIZCPvuj1Qf8GjlljgfDUYXyw37E6d/NiRO ilLzM59Q2rdFOHp/IMpeygNHMA== X-Google-Smtp-Source: AGHT+IFz9FW2U2XDVOM4MX6gEtXNBqhd0ARPDkjoUkA8I/HFGRb8YMvCepMaTFm2xwQMUsHHTOgkLQ== X-Received: by 2002:a05:6214:daf:b0:67f:3d1e:ad22 with SMTP id h15-20020a0562140daf00b0067f3d1ead22mr2663046qvh.31.1702908236081; Mon, 18 Dec 2023 06:03:56 -0800 (PST) Received: from localhost ([2620:10d:c091:400::5:c6bd]) by smtp.gmail.com with ESMTPSA id mx2-20020a0562142e0200b0067f566221d8sm175076qvb.121.2023.12.18.06.03.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Dec 2023 06:03:54 -0800 (PST) Date: Mon, 18 Dec 2023 15:03:54 +0100 From: Johannes Weiner To: Yosry Ahmed Cc: Andrew Morton , Chengming Zhou , Nhat Pham , Chris Li , Seth Jennings , Dan Streetman , Vitaly Wool , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 5/5] mm/zswap: cleanup zswap_reclaim_entry() Message-ID: <20231218140313.GA19167@cmpxchg.org> References: <20231213-zswap-dstmem-v1-0-896763369d04@bytedance.com> <20231213-zswap-dstmem-v1-5-896763369d04@bytedance.com> <20231214142320.f5cf319e619dbb2127c423e9@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 0F6AE180030 X-Stat-Signature: euocbt33f3w7pstto1z3cojce6onh9eg X-HE-Tag: 1702908236-989975 X-HE-Meta: U2FsdGVkX1+ds1/oOQeH3vqSKwpD9M4BhqqAbkaOdjvD2AwkmwNA7Twet8BMROY4pW4HFdCZjEPeIkX1esYpI2FhAaj+ipP7UsHmTslqDNnFXOiT01j9L9+ybtE/uhwB/LqOLltc6uEIyx/LRMrcxA+7gf5aWZjQopzGNN49e0UvOaj91CK/DJum3lgxwo91xeygbFLT/O+qsV6U2oXaheAfaw8AsK929qWgAu9a4l0xv3s81sQtnxmyc3MWLsifmfv/ibda/D8XTwgWQP/P8v9nVnCjNDgYWU8Yb5nWoBzknziK/ZozDv8+JbaVmrKrFks2TNFWbVjxR2ZtrJ9CKvTX5084pIzc5ngen7MybpOXbKc8YNoCzJA1XjNQdSui2wFtJ2NmvUVwDey+z9DXcmZ3u07Blj3y1hJcIqJBbgssnNcy6OFDyGBg98krJ8MuwcvO7mqEXuytWHv8DKdoGoJX/0NxfoZ4FVzB9ejuJJN1LLgWETpqrL23yaFND9mg/GKaYPrQ2gBD7+/N9V6DSdCO+TnVu3aAcJ7kLe+FOU/vjAySR2K9g/XxOu4xJTVkSh4YlMnwuAMMC+5h6G6wpv1Yd2sTZkWe6PFFqTm7lngAaNWsf9RvyqgSs0WFX1vAplTpAzSM3B4Mx3Bz55j8I4tLlxzxLyW/Y7g4hNHkyh9sSQj1U4filoAh2I9c4jJl71PfZLat7ZP1jnLiLXvwvKoRV32+ckzIyyuM6rQrcM8LMxbQZZJ9I3fbuh1IUBtAAispesxM3ylT5y0ZkZx9UdcqagkvasldVu6Mvl6S7YSbRWrq9wTO0jta5mj5ljophxppdOfH9y8xucilBSILm40AKtQxVHHcxd6SIlp5TxGq/ioGKYm9nEIwG7VH13pPB1Xm2WIBwGrhWQlYoMrWBrOK+P/j7vfKsNEJwkJFZydd836XKy0dvIFUAZuFUP8r4hshyeWejC3qzL+JxHU eEAFpnF9 AvEjIfhm7HnW5mrpRQdpDV+cxUvrMukS2yCuD5dy73JUfzX6ASFUD1tyy5o1pTVYQv4lDK3B3q+RswQziaKmxnpfscaXApLhbm3OUhyyPBujqzBYGhUT1qybG7X9gwBL+4hGUX8YmHXXYRmxBc7bM9E/q9G9V4BK805dj3EKzKWjv/P8P4tyVRP2cQWkopUGn+XN4n1E8HFie1HOrKcLbtthpjNHtAorWgsWZ1VAED1aq0dwZHQVXeQqlX8+YWke9EvFhYgQNvqkrDWMgPgjQUVmKhpZFhJIlx/DqGsLbYIgtuRyQsyQpOVJDiVNhc91V14tqOSCUUtCQ9BnmJi1LCYON40dseNZZAPAuCgKimjBzEAZXumddGyS0Fu7tBCYo1voG+gdl7/DuKRgPjHoPS/KpCOYEgqM057HsATgPI/po3sAm6vCidANbEL2M+eI3r2gjk3kX3iDhDdAtewlHEa1IDWrNvLvSyT5S6qJmoTCwK4GqLaMQCKImmmfLzDS7h9pGREYCBtEd3vEbFSp9F0PAw3M9cmhYLB9ljRDlOiolw6UXcHia3Q7OlIPBlZ3FT/WXYvZqRERbKT9zYkRMx5Chu3XpoQv8w2/GwlBspSVa+XZ/ZTGl/9GgyHEurMbHXik9ZRisqrVfzgkxmddBufPwBas+7gvOJ58yrwx+W7tAWTlcVY1YO5b8cASYdSOj+FvvKNVcOjDmsAus9QpGnPdv5uzNkouFa+om X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, 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, Dec 14, 2023 at 02:41:26PM -0800, Yosry Ahmed wrote: > On Thu, Dec 14, 2023 at 2:23 PM Andrew Morton wrote: > > > > On Wed, 13 Dec 2023 17:02:25 -0800 Yosry Ahmed wrote: > > > > > On Tue, Dec 12, 2023 at 8:18 PM Chengming Zhou > > > wrote: > > > > > > > > Also after the common decompress part goes to __zswap_load(), we can > > > > cleanup the zswap_reclaim_entry() a little. > > > > > > I think you mean zswap_writeback_entry(), same for the commit title. > > > > I updated my copy of the changelog, thanks. > > > > > > - /* > > > > - * If we get here because the page is already in swapcache, a > > > > - * load may be happening concurrently. It is safe and okay to > > > > - * not free the entry. It is also okay to return !0. > > > > - */ > > > > > > This comment should be moved above the failure check of > > > __read_swap_cache_async() above, not completely removed. > > > > This? > > Yes, thanks a lot. Although I think a new version is needed anyway to > address other comments. > > > > > --- a/mm/zswap.c~mm-zswap-cleanup-zswap_reclaim_entry-fix > > +++ a/mm/zswap.c > > @@ -1457,8 +1457,14 @@ static int zswap_writeback_entry(struct > > mpol = get_task_policy(current); > > page = __read_swap_cache_async(swpentry, GFP_KERNEL, mpol, > > NO_INTERLEAVE_INDEX, &page_was_allocated, true); > > - if (!page) > > + if (!page) { > > + /* > > + * If we get here because the page is already in swapcache, a > > + * load may be happening concurrently. It is safe and okay to > > + * not free the entry. It is also okay to return !0. > > + */ > > return -ENOMEM; > > + } > > > > /* Found an existing page, we raced with load/swapin */ > > if (!page_was_allocated) { That's the wrong branch, no? !page -> -ENOMEM page && !page_was_allocated -> already in swapcache Personally, I don't really get the comment. What does it mean that it's "okay" not to free the entry? There is a put, which may or may not free the entry if somebody else is using it. Is it explaining how lifetime works for refcounted objects? I'm similarly confused by the "it's okay" to return non-zero. What is that trying to convey? Deletion seemed like the right choice here, IMO ;)