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 F294CC46CD2 for ; Tue, 30 Jan 2024 15:46:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 45A136B0074; Tue, 30 Jan 2024 10:46:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 40A136B0078; Tue, 30 Jan 2024 10:46:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2AAE36B0081; Tue, 30 Jan 2024 10:46:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 178C66B0074 for ; Tue, 30 Jan 2024 10:46:45 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 795321C0F8E for ; Tue, 30 Jan 2024 15:46:44 +0000 (UTC) X-FDA: 81736405128.24.4707B1B Received: from mail-qv1-f44.google.com (mail-qv1-f44.google.com [209.85.219.44]) by imf09.hostedemail.com (Postfix) with ESMTP id 30A3F140037 for ; Tue, 30 Jan 2024 15:46:41 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=oXdYbKHg; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf09.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.44 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=1706629602; 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=LSHrhHBVRB/N03Yv4tNXKIcG1eMofMSVs9poHGu9roI=; b=xv102owy/F7Wj+FQi8DesGQ25xHgz+Yc7s2m1ciaoX++7tIdRVx7dBeDaLNH8ckUkc5/jp 05OWEXqu/C4tdqMOAotkWGl4qRS1hYxk+g+nM6EwPKiV6w7GOge83zCnrNLcanx/cG5Shg Oq/fd3616LSOX28clzK5bQ1qUsLHUOo= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=oXdYbKHg; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf09.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.44 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706629602; a=rsa-sha256; cv=none; b=EL35NY/UMc4v++XB8j+DPrtadS/5Mina1qAeOEIxq+2NEHu94JDJc4kshUNQpgv9DIgU+3 9YZMG9HV3BxL3bXfDzUi0Xlol8LNfdI3mBEL6SidQ3FptiKCDjPpTUBT+rVl915Ldno3K9 bu6+hRVjtNSCuBhUqXirok/JKadY7jg= Received: by mail-qv1-f44.google.com with SMTP id 6a1803df08f44-68c3d51ecebso23167276d6.3 for ; Tue, 30 Jan 2024 07:46:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1706629601; x=1707234401; darn=kvack.org; 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=LSHrhHBVRB/N03Yv4tNXKIcG1eMofMSVs9poHGu9roI=; b=oXdYbKHguCuHsQqBjhykhI2NvYFBexsxdvs2wbLx9Itx9QeoTz/JhbALhzgSd3Ucg2 Fj+EZHcDxZiFaIFayp5uWkNnrl1yZkIfzTThQm2e3ooJH7g5fqFC/X0YenHc0vUmXtSS OlASdTZZkU5DuVDe59uQVCFqRwuryONhWrv5J1/UUlB9iY66suHPrZdg+pYIgl8qizam jUFVwGdcI/jIs8O9j48/+JDBkwywSz+q1C6fH0oiyB2ZkKwxPoITMJfxLk1gdiV8FQiS NbA1H/z9IwtJK5q32S8ekvr4F6iw9K83pg6xOzQorjK7dX2Z4EZJqc3VRvqXMrSk4U+o RfKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706629601; x=1707234401; 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=LSHrhHBVRB/N03Yv4tNXKIcG1eMofMSVs9poHGu9roI=; b=W2+AWf/ghq71bhjB8A5jYy4YMg4I4t71Nly4qCWo8HEhklqhFi+PcYfRRUm7fXeTgR DDOjww2EvIja3w4oUwa3aLk+aTYJze3wHs3E7AvI2ZQ9OIme1sq19R53idlciQHO13i8 6B2kWtNed4AMjXP+t2Gtd2rq1t1dbL+Q6TqUErdCsycBlzMhvZQey6s2aHVW8VPNhfQ8 geBYI5Dbo+yC42OHbU6B8wbJ5T988+BijByhe+ysXH+Uv2r5AKVUwz4nO4Si7WedAjaB 1opn/WMHm5/rpNpaIFy21f69VIKbm8v/sdp9NSSDURXnuY5ItKcvpO4rEPoczqIXnudZ EWmA== X-Gm-Message-State: AOJu0YwqfKd5PDwNwoiZH67OfZu+Xa2b1948zYEa5N8W6N5a+i8Vw9p0 C/1THPDbnEO6dYPUPn3y95s16QNV7LYRFn2TDVCTWz/DI4qCt1TvBSOq8zALi5z/vE6kWOS7x3m 7 X-Google-Smtp-Source: AGHT+IE3fiKrWSe7S6ZuM1WBCgRmbxyZCIZYYdg2jOCfvPFEw9m9XnVh5trG28mBdcgkRiPRIGCptw== X-Received: by 2002:ad4:5ba9:0:b0:68c:6216:35da with SMTP id 9-20020ad45ba9000000b0068c621635damr1602767qvq.55.1706629601193; Tue, 30 Jan 2024 07:46:41 -0800 (PST) Received: from localhost (2603-7000-0c01-2716-da5e-d3ff-fee7-26e7.res6.spectrum.com. [2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id pi8-20020a0562144a8800b00681785c95e0sm4595066qvb.46.2024.01.30.07.46.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jan 2024 07:46:40 -0800 (PST) Date: Tue, 30 Jan 2024 10:46:32 -0500 From: Johannes Weiner To: Yosry Ahmed Cc: Andrew Morton , Nhat Pham , Chengming Zhou , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 00/20] mm: zswap: cleanups Message-ID: <20240130154632.GB772725@cmpxchg.org> References: <20240130014208.565554-1-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 30A3F140037 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: p5xqpb4raxqbrib4nswe1ojdiq7t3t3p X-HE-Tag: 1706629601-980116 X-HE-Meta: U2FsdGVkX190NbLw02xJtBIFHQNQzCcZLqRDXt4aOORjfDYKVSQl0aKhM/xIuC8J22euSYLC4eboxL3T89cTcxcDD59H9hFTD8dQdxCJqggosSQI3wUD7k2El7xzw1wR4BNFDTF+KPIFM8V0THF7HTnt28pGNmbh0CX+Zm190e2XJggYmfFi/zx7u8qNfNcuUfE/2wMTdbvsOyqoz7/F4eiC4ZiFwYfX7yr+d2AY4l51/JJuifp2f55i58tKLNNhaMX34y5Qhy/Rlm89zq3Cu+8ydtG32Y2VVli8GlrFPUuBMQhGzTPRYZVfozfspmsorHhs7n/ZVfY61FXAsBZOvDtE1+6V2KsgNZwPJsmMfiMxZGA2lo8bJMnTlWUID23XcpgKj/yR/jotGG0CN6P/DRmWUHESBFfDo/ic6kr3+VhlJ4PaUlQmMmm9hqT2Ul43F/2jhRw3x9UBg53OOMnTYncEmewpalSrJqFasAoeXJjuWMpN43Zvv6aJ9KPFoaMwW3nOr3ifTIw05nCBikgiOrE4imQohMYFsvqPv+uNLm9iiJmolITgsc9rN/jJxye6sPAE71H+KuoboyQ9hgE+mWqjfT2icbFeezUDPeAfXVQMqhSYVAImaHc8bhp5v3M4f0XsJFvGnnW8+RnubNxpRnlc536jVeCByhDfG8868hWRFHxFIJB+kuKZ9ta22uWf+hAmBhG0WHHASS0sgnPPAF1fvfBZFCGdXnzGa4IGDNVMyumyNGvHY98bHR4iHs321CzhNfQd9qtukL+VZUp7Pe0h2RVKuSHPTsZDnBZcvYYG7RVv7CsKe7zzDtEukK6gItzfOS9x1jxIC9IA/K9MnQJU4guoW1rcvQ6GPktubIdNBIGtQdl4nGaH4yuwjcF0EgaXBSEIKvrK1VNl7EIBsOOQgkMdsDSboMCrBw7xNDV86ob6slGMCVKoqTkxWu1C8pTse+pn0d3cyzyUjrn 4YXRVwj6 rfuhB6okw0tWahlu5HHlwLzZK57CiarVC2PBOgmh3G9nRtlQ428vEHPR1mrphcqR2yEIrkENStkwMd1ghWAZk/9Oti446efDfjfbmc/aStr9OoBTF0fR1jqAiaCEN0L3tdGG3T5HTjdZz3JB2vwofZELLEilj/dTGrMlwARgCBdmZh+aoAdhEV050ACdwLlixR6Sh5Fp8FyQbiTV3aPh2nhoVFRPzrege5WL5aj6b1wk13T6qndWVM4fYwbb5WyqbvS/7zU/Tx/thDIDJcE1nXanBQnOUN/te8b6rC8BVwLJp0RCaTMIDq5xJLCj6Ip68aldw7O9tmkjZgx9wTKZlVLbWbIY3s0D8cuCDYVCm0Yldf9XfnuCi1btEoHNNdu1kqdK5Zo6JNkIXeJo= 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: List-Subscribe: List-Unsubscribe: On Tue, Jan 30, 2024 at 08:16:27AM +0000, Yosry Ahmed wrote: > Hey Johannes, > > On Mon, Jan 29, 2024 at 08:36:36PM -0500, Johannes Weiner wrote: > > Cleanups and maintenance items that accumulated while reviewing zswap > > patches. Based on akpm/mm-unstable + the UAF fix I sent just now. > > Patches 1 to 9 LGTM, thanks for the great cleanups! > > I am less excited about patches 10 to 20 though. Don't get me wrong, I > am all of logically ordering the code. However, it feels like in this > case, we will introduce unnecessary layers in the git history in a lot > of places where I find myself checking the history regularly. > Personally, I tend to jump around the file using vim search or using a > cscope extension to find references/definitions, so I don't feel a need > for such reordering. I agree that sweeping non-functional changes can be somewhat painful. However, moves are among the easiest of those because the code itself doesn't change. git log is not really affected, git blame mm/zswap.c works as well. Backports are easy to adjust and verify - mostly, patch will just warn about line offsets. What motivated this was figuring out the writeback/swapoff race. I also use search and multiple buffers in my editor, but keeping track of the dependencies between shrink_memcg_cb, zswap_writeback_entry and third places like load and invalidate became quite unwieldy. There is also the search in the logical direction not finding things, or mostly unrelated stuff. It's just less error prone to read existing code and write new code if related layers are grouped together and the order is logical, despite having those tools. The problem will also only get worse if there are no natural anchor points for new code, and the layering isn't clear. The new shrinker code is a case in point. We missed the opportunity in the memcg codebase, and I've regretted it for years. It just resulted in more fragile, less readable and debuggable code. The zswap code has been stagnant in the last few years, and there are relatively few commits behind us (git log --pretty=format:"%as %h %s" mm/zswap.c). I figure now is a good chance, before the more major changes we have planned. > I am not objecting to it, but I just find it less appealing that the > rest of the series. Understood.