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 61330C77B7A for ; Tue, 30 May 2023 18:54:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DB90B6B0072; Tue, 30 May 2023 14:54:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D41846B0074; Tue, 30 May 2023 14:54:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BE1D5900002; Tue, 30 May 2023 14:54:56 -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 A84B36B0072 for ; Tue, 30 May 2023 14:54:56 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 800E4402CB for ; Tue, 30 May 2023 18:54:56 +0000 (UTC) X-FDA: 80847823392.02.46F4667 Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by imf22.hostedemail.com (Postfix) with ESMTP id 1F5EFC0020 for ; Tue, 30 May 2023 18:54:53 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=34SfncPA; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf22.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.170 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=1685472894; 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=MOjS6CVqlBlukMRZLNaJyPM2L7QGvu6hUflH8GHgCWM=; b=0yBC5WPqswCZxtD+1cKoWvIKSQTQM0zXU5SX/MNCyBwMXYmH4AOp9LfgJWv7t99qcosJaa W88zxHQFJAS7f78xL/1h+hUcgre7BIRywseVdryzEO6ecakWhPPoC5H4Z+w6tDxpFiHBDn KxzDAEDOOf1O5K304Gp3gVRJoSy2k98= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=cmpxchg-org.20221208.gappssmtp.com header.s=20221208 header.b=34SfncPA; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf22.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.170 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685472894; a=rsa-sha256; cv=none; b=bgk5caIBFWq5wuAFPDbgKT1xKq4Klxan2ZU5OahDZ0w99nimlm3NU36Ogf5utejNKiUNm1 H//6EDTaKM62ZxhAdYV5dH3ch+DXv2pWydjrcFWmliTjzEY64xHy5eLRXi27VGWMrs7Kkk Rl18AD+282UVrlxaiOpCKg/ONItsWqo= Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-3f80e055549so24824391cf.3 for ; Tue, 30 May 2023 11:54:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20221208.gappssmtp.com; s=20221208; t=1685472893; x=1688064893; 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=MOjS6CVqlBlukMRZLNaJyPM2L7QGvu6hUflH8GHgCWM=; b=34SfncPAo+S5Zx6sK6HG/1bJhx8/ATdMJVZMHEQaRT7Y4f3gXOLTXCLR7rblv6YwFk TTaLRJERwM1s6a2jYaBhQMVNtdHEdPoJozenRUFkkQUaITKX2I4Hwv3lhTsbsHgyJiCH zDezfydRMcPlAbojRKJIvoD3zB2AyUBxWKXdDf3YwNKgU2IGL651kVxcxyUjevXQygmY DpedRmrmRkq2pCHQvBhaYwXMwVI8BeVTIAHyizdkAmi7MLlckyT1lI31NWHO06HGZ9S2 36LEDysP9nZ1wbkF0yv2NxaLRh677Pb4wEhLX62qlVeVjyKvpvL5Tv0edR8iNNCuTxX8 fJwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685472893; x=1688064893; 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=MOjS6CVqlBlukMRZLNaJyPM2L7QGvu6hUflH8GHgCWM=; b=jzKb/GjolD5Xs+r2pEYEvb9hy3IKLcp2JbthYd1BOJib7IlF37C84MxGyCO62Dw9eN aWpyZ6rzd5W5fJ6Lq0nnwoJkf03h0bmwAAfsbODUfNFVHGUDxp7b8NKu7m1D4p3VuPqL yPRQ0Bz5lwefJaUwefhyegEWVJGnFof5j/ewKu7dpkqiwur5thryaKEsZlWQNdzP+UPN U7y61bSY0RbQEguQsugrMdzzJFLwrrO9g3etSl0syo64XlD/bsobaLUOYYjAoj04Q9oL OoNuvydGSmjdjXk14jQDyTaQFZSUNBe++wIBadTR48jMj8lHRC5f2v8Uhfvi98D6aUQ3 3fzg== X-Gm-Message-State: AC+VfDx7fMcuurC/8hxartfdKPFKq3j9kD5NADZUudQX298szuifVQxU Z3cKLo6o0pjqIaMn8sG8EBvUZ9/7MhV/IZBd/VQ= X-Google-Smtp-Source: ACHHUZ4zOpT7wylBmFPvgJ9K6sJTZ7a/0tYIBptFJ9spvNGkLTEJ51s1qpvDB12N7yf9NabvwV+iRg== X-Received: by 2002:ad4:4eae:0:b0:625:aa49:19f3 with SMTP id ed14-20020ad44eae000000b00625aa4919f3mr3525901qvb.64.1685472893062; Tue, 30 May 2023 11:54:53 -0700 (PDT) Received: from localhost ([2620:10d:c091:400::5:8bb6]) by smtp.gmail.com with ESMTPSA id 18-20020a05620a06d200b0075c9779c03esm4318609qky.17.2023.05.30.11.54.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 May 2023 11:54:52 -0700 (PDT) Date: Tue, 30 May 2023 14:54:51 -0400 From: Johannes Weiner To: Chris Li Cc: Domenico Cerasuolo , linux-mm@kvack.org, linux-kernel@vger.kernel.org, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, yosryahmed@google.com, kernel-team@fb.com Subject: Re: [PATCH] mm: zswap: shrink until can accept Message-ID: <20230530185451.GA101722@cmpxchg.org> References: <20230524065051.6328-1-cerasuolodomenico@gmail.com> <20230530041341.GB84971@cmpxchg.org> <20230530155519.GB97194@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1F5EFC0020 X-Stat-Signature: 5gbkpxofn4jf1gocfmdpxgyfrtmw8sct X-Rspam-User: X-HE-Tag: 1685472893-502823 X-HE-Meta: U2FsdGVkX19eSJlfZD35uwlWonXgnecQVVdgbU1GHm4UtHv70LlVEspWuB1zeD/QS2jBwxFvBn7uuEvEWaVaAUN+z81GrpzlCkIi57rchp8vl7t0RC0wJDR4t9RhTAZwRfhvPE7cNJOSdXg8ine38F90j/bowrPINQCc89a65b/GYWVT1K9QaU+DY1QfJu3cQeTN6wHb+ckx3++h9pQO10/KX7iKEkblP+YVeNOLqmBCpyyH8YHtofRNeCDvACei/flDr2Hq1SO3UEqZVGsetKmIJGpC7f1sWm8VSlMJi1gh/oP0OBvEjQ6kLW9jADnI9I9Gt0x1oA7SD0AZQZMUO1F9jK1zgxwp2qLM5B7c16qtPIPG0+HwcihjsSMxwerhrCOHG3d7BwCXUFczTOhbE3bXF5hkQ0bOf97jf4LUqWzEjYHGAQz7VYgSRMjpq9NtOKccW2dBXcmJ5DdbMY8wGQKdAL9gpFiqvHeAgAmZq3F7A1yJQmrsuTSGIxMScSQ12V6MREWCeKBdvwGoKIK4NajxjDIX10Vg8e86pZMUjalzTQWDFWp3aPj7v36RmzmT300TJgTOdm85RO/YvFkXXMMZKUIgkhX7CWqXYXYoYflTrC8oGGfRiKcEz7RCZMQ5P1vIiJ0M3PHesu///ci0IqR863n/L6Wvq5/7i3CgabVHKxyhT8NZHyYvWTbgyM14M0vFSHclSDDs4NiXmT6Se5Nn1vhHApoxvBZUiDqPlxCYYK5xEssf9aLYEvFYdsd+Gc47SjHhIUEDYHV9X/hKBrtLc+H16ezNw1Sln2Ud9n2WG3Wi4Ha3Kzu2ZysGFIk+NErP60DyF5+pCvWWMLGjKLStOPxfC5WKY1Bg70LGfzcnQAtxyR/91Rdrye5GSCC4ucfV5y5M4YTCQmZcxJdU5NyskXmT7F7oziyeIxgL8mF1ZbkOk693npnhMq5rV0ZPtRlhS3xnGQz93K+w6uN 0Vwtl3FP 990G623WjbrKh2K79OUuftiXt+dx025kq9Q9B2Ebh0xmheBb9BHI++xBzGQsL2Yh3WmQg07q3koDicXXBA6afn43xLpbJtGomEuqNYbeJ/nsRaC1HIyD5WN12eLhs1d67nAEue3BmxDq+RSC8b2jcAHW2bP3K1V/r3R8jVpBQY/Erpr4dYsUQBMtA163p5h/Y147VcyY2DXYfCvhjmGsWcNVP3ofafufkIrsIaUaxHkafe7I2PDP6aYwy5EmG/uzkpIqA/yGYP2CYHvHSBy4AapobCaxQ0eBXYlMCTLDBIHk2gX4O77Du604xAxuvK/c7l0C1a4A8DvWKRfOtC12sv67QSnvGYGWM1Oa4yQwpuKQkrdKqAsZfMRvtGGf+QJddEWRs40S23qkz5S4B+UXe0DVYpK+0KJZcQEluwvC0IfBki4E= 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 Tue, May 30, 2023 at 11:18:51AM -0700, Chris Li wrote: > On Tue, May 30, 2023 at 11:55:19AM -0400, Johannes Weiner wrote: > > On Tue, May 30, 2023 at 07:51:23AM -0700, Chris Li wrote: > > > Thanks for pointing out -ENOMEM shouldn't be persistent. > > > Points taken. > > > > > > The original point of not retrying the persistent error > > > still holds. > > > > Okay, but what persistent errors are you referring to? > > Maybe ENOMEM is a bad example. How about if the swap device > just went bad and can't complete new IO writes? This is actually outside the scope of zswap, and handled by the swapcache (end_swap_bio_write). Once the IO is submitted, zswap will ax its copy and leave the rest to the swapcache. It behaves the same way as if zswap had never been involved to begin with when the swap out fails on IO errors. >From a zswap perspective, there are no persistent errors in moving a zswap entry back into the swapcache. Not just currently, but generally. > > Aside from -ENOMEM, writeback_entry will fail on concurrent swap > > invalidation or a racing swapin fault. In both cases we should > > absolutely keep trying other entries until the goal is met. > > How about a narrower fix recognizing those error cases and making > the inner loop continue in those errors? Right, I just I don't really see the value proposition of this complication, and I see some downsides (see below). No single entry error should ever cause us to stop the wider reclaim loop. > > > > extreme case where it's the only page left on the list, I again don't > > > > see how retrying a few times will make the situation worse. > > > > > > > > In practice, IMO there is little upside in trying to be more > > > > discerning about the error codes. Simple seems better here. > > > > > > Just trying to think about what should be the precise loop termination > > > condition here. > > > > > > I still feel blindly trying a few times is a very imprecise condition. > > > > The precise termination condition is when can_accept() returns true > > again. The safety cap is only added as precaution to avoid infinite > > loops if something goes wrong or unexpected, now or in the future. > > In my mind, that statement already suggests can_accept() is not > *precise*, considering the avoid infinite loop. > e.g. Do we know what is the optimal cap value and why that value > is optical? Oh but it is precise. That's the goal we want to accomplish. The cap is just so that in case something unexpectedly goes wrong (a bug), we fail gracefully and don't lock up the machine. The same reason we prefer WARN_ONs over BUG_ONs if we can, to avoid crashes. That's really all there is too it, and it strikes me as reasonable and robust design choice. It's fine to limp along or be suboptimal after such a bug happens; the bar is avoiding an infinite loop, nothing else. Your suggestion of whitelisting certain errors is more complicated, but also less robust: in case an entry error does by some accident become persistent for the whole LRU, we're locking up the host. We'd rather catch a bug like this by seeing spikes in the reclaim failure rate than by losing production machines. > Putting the definition of precise aside, I do see the unconditional > retry can have unwanted effects. I hope I could address this above. But if not, please share your concerns. Thanks!