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 F322CC46CD2 for ; Tue, 30 Jan 2024 17:15:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 86E816B00AF; Tue, 30 Jan 2024 12:15:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7F8A56B00B1; Tue, 30 Jan 2024 12:15:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 697FB6B00B2; Tue, 30 Jan 2024 12:15:53 -0500 (EST) 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 5753C6B00AF for ; Tue, 30 Jan 2024 12:15:53 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B6201C0BA9 for ; Tue, 30 Jan 2024 17:15:52 +0000 (UTC) X-FDA: 81736629744.16.E452DE9 Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) by imf26.hostedemail.com (Postfix) with ESMTP id 0A1E0140004 for ; Tue, 30 Jan 2024 17:15:50 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ptU4vSBF; spf=pass (imf26.hostedemail.com: domain of 3xi65ZQoKCD0xnrqxZgldcfnnfkd.bnlkhmtw-lljuZbj.nqf@flex--yosryahmed.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=3xi65ZQoKCD0xnrqxZgldcfnnfkd.bnlkhmtw-lljuZbj.nqf@flex--yosryahmed.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706634951; 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=ljX+DuGhu1oX6Z5GCA189BtLMB06zqPcIW20thBcqTo=; b=ZLXItqxDF886o2tsMd/wVPXrVOTkpzL1u/yj1d8zWVGw1Ma2jKVkQdgsA2qVCYdQy/BjEf /NUIBcVWNbdXh/uP22h21Spi97qzfFk4wfeesxTvnvod3dcdsEN0mWlGm1oBy2ox1pvq+p SU4iySnM57VkfEcJ3yg/hE8V+s3kydU= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ptU4vSBF; spf=pass (imf26.hostedemail.com: domain of 3xi65ZQoKCD0xnrqxZgldcfnnfkd.bnlkhmtw-lljuZbj.nqf@flex--yosryahmed.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=3xi65ZQoKCD0xnrqxZgldcfnnfkd.bnlkhmtw-lljuZbj.nqf@flex--yosryahmed.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706634951; a=rsa-sha256; cv=none; b=hwf+MS1G5VmMkgdGWbgvw1YuxOo5aPOZ5Sbqr9CNtSiWSbZolMcSbN00lqMDm0gLKUq+Fv nvDEJDMm+p13JT+01m/x998hlbvFwqzXIhSWtCFECy+28DKtg0f8L5sarguyu7/TLwjJ/6 mydaTq4ApUvHdqYDz0SixyM58Vm5Dr0= Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-5ffee6fcdc1so74961197b3.2 for ; Tue, 30 Jan 2024 09:15:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706634950; x=1707239750; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ljX+DuGhu1oX6Z5GCA189BtLMB06zqPcIW20thBcqTo=; b=ptU4vSBF91BK/8pj1i6oIwWgL5qqIEGx8HVnBJIkynH+JRBuxDkoHNP2fd0e2PaT+5 ZFf/o55lUcIH/b3v9h3n6675QIfLb27+TANgiV6FtQgonIjwx6MTHah7GZe8KiYvsA39 D3U1vNlYB/+N/VurgKGznBb5r2WPspQj7PxYdbYVdTu46Ki1SaJUu4tyl2x/n58EEYOU s25JxMCIi5+5xh5+gV0E8alfswaKhSysmexoqcEaCrxJlGf0orQyg4AW6g02ywP4Qkr3 iTfmDBUymc7DnYzRErzzPL4pBPX2zdZQdC/qznwT9yBVcc0ZZP2Jmvm/yIlP9pJNaNdd 5Jvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706634950; x=1707239750; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ljX+DuGhu1oX6Z5GCA189BtLMB06zqPcIW20thBcqTo=; b=OiRqVhNZdjDjTCe1C/xW3HXlrduHfZQtw3UPNfXljEc60E2yMjPqShTgOUor0YCaFD RdZJHC/n5oUL5ET/JrqARnHpQ4k2gVI4T62ZJRmQOvBdf25FI/UiDcaB+3ttn690b4RC vXiHTHBTJvgjCKJg7e881HpXikIbM+gFYMvzevWqAXfHwDGIJ7QWMihRbSzQBmmY72VA 1K1mzYqScUHWVBjBC4FI+0vxxyMRy4hf6rIHGGPip6mXpPW28pxvbKkrn0kIUysM77wW SH1s9ugeUQTyQpsUfFKr4nm1xRqfDezUaT6psN81YDRk6Znp3WG6ORgDrrJT5CZuQk1i cF6g== X-Gm-Message-State: AOJu0YxzJWNcyxrXgd5v9kgu/PVSIjmyln4xfAVvzc8RJnC9tMV8W2H3 JaLFXPDTu8DG+uVcCp1vHGmVRvGqp1J73E7+YFkdr4ZTeFRF7Zrlkjmw9eReJVCOQnY7lfD+ZM7 eHsSFaDb+q37ZUmXoJg== X-Google-Smtp-Source: AGHT+IGFWkMUxyhX+tCuHFDlz775XADs3S2PtN0E3PqqbVP7wLG0M0EORo8t9IHM7cv1qMgSfyyRcBQufD0n1spe X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a81:9a42:0:b0:604:594:2a58 with SMTP id r63-20020a819a42000000b0060405942a58mr48745ywg.2.1706634950186; Tue, 30 Jan 2024 09:15:50 -0800 (PST) Date: Tue, 30 Jan 2024 17:15:48 +0000 In-Reply-To: <20240130154632.GB772725@cmpxchg.org> Mime-Version: 1.0 References: <20240130014208.565554-1-hannes@cmpxchg.org> <20240130154632.GB772725@cmpxchg.org> Message-ID: Subject: Re: [PATCH 00/20] mm: zswap: cleanups From: Yosry Ahmed To: Johannes Weiner Cc: Andrew Morton , Nhat Pham , Chengming Zhou , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" X-Rspamd-Queue-Id: 0A1E0140004 X-Rspam-User: X-Stat-Signature: yxtta4ekuj6qfou36fxbo4ubp4mr5g31 X-Rspamd-Server: rspam01 X-HE-Tag: 1706634950-212815 X-HE-Meta: U2FsdGVkX1/AfhADmezNE0BxyfdJc0iZdo64a/oNinIXeY1aXuXmB3udZN2AtmB/4nHy/+KvBGWF9iSb3roUyVNqulvv51hrkU+hq+JfnpsZ3CbPxTWYQl1cW4HB4NyXAPsy6tZHgEKdkCeRDUGJh8/GShi+Pg8DE/DKyoUnM89GJ4Z+UFLhSMiQ85Wr6w/ZI2WP0rHlNvv9ZiDDRoZhN2PUZv7UeTO723+vQzxuu4IvgK3b8VKr3Jykhm26uF6hStSyIF2dImjevMv1cNw9YmJHcLw3+qc7pkWmJF0Z5gBCELlZ/P5MuCxzXsS191TgPc1KvVl6zxhZcf6cV6IZWSVJ8t1BbJnEMspUCgVd/W1+Z9K96sENJZiIoONICaZTess4mqtdBM96tUukDMOhUmyQdaiqA8BfiWQI9yjZKdAFPw0h0i/XXzesRmsZIlR2L1uqegGS2LwOLfb98+0G/cLKtI9fKv10dbyfz9par7WgboJO8VNylL9nMQ3nEBKdLyQ8JGN0yaN+uPL/iQSpT7/GGck/jx2jzJSo6vMEfRUBftY1dIwWBYMPCoa/EmjzIETk/y098LKT3kjVRPv/sair53Eays8bzqKL4fIaiUPTIDenhhHBNRjk5t3x+IQmaVs9Z+5xYDcs0wXUuUorZFI4OzfKnqluC7wW+3xdAYsPMpVgwP/6yjzGzrce3NKY4BVr8JTB7nVEW06e/mpd7BJXGqEG2q3iQJUg3FnX08jPfHUmYW2QPvpjgjfzgXfkDObXQ/Dk2d0NH4ztVQfl6PcWPSrj40/l5H3PyEi8aACU//wZF1ybizSAFMWrCODk/GzVnhwum5r10958Rkq3Q66xniiXqg0Myl0ZJKWT6bnDiUs65wIJU2WtGM4rLskfTQF91TBWXy9sQwwgKwjBqdE1DFbkKnaJh5McRvV96ILw7xglazk1a1xzMl3u6dz4Mqq+TtIEygPdfpvnxAf e4ibfSpW kIg7E6xwpsvC9PKrHrQlmiYRVr98zkHMNoaZWSTnlN+16ujDDOKkAOkLSVsDOWdosVsIo2faLZJHMF7czxw6ohloPtRLPHo4xz+E2nG72RAg2VZSk39PkvMa5RkmVGkq67wZRvJKA/yYbN9QYTq2WEb6UVGVF5HE6sSMwkuQ+sc1ebNLhJyAkWRlsz9Tfi5aubNQrBLwJkY2HWmY5a4VXTu6itbjILOk/uZP8HUd+hlpoKLLuYdIFtGpT9495bKJ52mNlB0ocymiQzP4UMFvKdSiYdLTU2gnB7JQ24NJMv7jt53LCSzKfvcv34Pgy3hsMcSgtl0ciLcz0xKomK/ha3OoMnG6n3ShxCBAOJXtb0WOD+/g6eVOOtAl+t1Wn0gGu3s5Vbm+oopWsYRxNVZrBHNhEj6Qya+TUhyyz602dqDXv7sj6Ju6AF05QLrHRAEsWG/YZZ/X4qAjupCqIzp+uG+m/fTJy7ozcGvUbb6wapOA+dlTEAQ+TPaqzJcigqCfm33Sz5ZYRw5v2OlFDwSLXI8/c+IMigv/wecerZ0ZqAbnfsUf7cNHhpC1mH4Gx1kQmDh/cGCdvpGtVr/21pPlOh4N1bqAoa8YkQpBaB4V/4ADKSnE= X-Bogosity: Ham, tests=bogofilter, spamicity=0.001623, 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 10:46:32AM -0500, Johannes Weiner wrote: > 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. IIUC with git blame -L won't really work because the lines will have changed significantly. In the future, going through several layers of git blame will be less convenient because the function moving will invalidate the lines. > Backports are easy to > adjust and verify - mostly, patch will just warn about line offsets. I usually use git cherry-pick, and it sometimes gets confused if things are moved far enough IIRC. > > 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 agree that if we want to do it, it's much better now than later, just presenting a differet perspective. No strong feelings.