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 77998C0218A for ; Fri, 31 Jan 2025 03:34:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0418B28029C; Thu, 30 Jan 2025 22:34:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F341E280299; Thu, 30 Jan 2025 22:34:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DFB1A28029C; Thu, 30 Jan 2025 22:34:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id C22A1280299 for ; Thu, 30 Jan 2025 22:34:56 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 765C21C73EA for ; Fri, 31 Jan 2025 03:34:56 +0000 (UTC) X-FDA: 83066330592.21.13AD21F Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) by imf09.hostedemail.com (Postfix) with ESMTP id 87BD5140002 for ; Fri, 31 Jan 2025 03:34:54 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b="j/p4qDpF"; spf=pass (imf09.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.172 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738294494; 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=32D3HUGqP+ZFdFEF3wCZMSpfCGFHL5f2rNBX8ncFDZw=; b=YNGNi7XC5qkB5S/FN3IM/mLiiOTZ8l3R/ccvYZue45x0ERnY1H6Lnk+9Dw+ik7F7+hYuZM pMYNXuYPrlii4EDJuCphpqr1YCGjniKfpmLchubk3wbcTibxDJOcllBT2wgiLkfSj8MUPA xtAXLeMCGJs9mWi1SofNze/t9yg1H90= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b="j/p4qDpF"; spf=pass (imf09.hostedemail.com: domain of senozhatsky@chromium.org designates 209.85.214.172 as permitted sender) smtp.mailfrom=senozhatsky@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738294494; a=rsa-sha256; cv=none; b=igSsnP7Ld1fn8oYVHLrPce8I4i/J9BAkwyhTxMh5UDvqt+DJz9C9+ALSAbo7CBCK8rkfZe qgWmJ0WlkR6xZFoX7m1Sb7eRQbX0B7vuDikX8rmo0mQrUDTgZKuZB4/p9Ji9Z1BMDHbDkj Ud3TPgTWS+EfWcknLbnHiRjcxBQV9zo= Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-216281bc30fso34843735ad.0 for ; Thu, 30 Jan 2025 19:34:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1738294493; x=1738899293; 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=32D3HUGqP+ZFdFEF3wCZMSpfCGFHL5f2rNBX8ncFDZw=; b=j/p4qDpFow2N+d2ZXWkGQSt7C5DVODwSvaGVKb9uDOmPsJndBVUndl2ODq7ekTxBCn WqZ5A8RDYyj2BXWmpYJO9FgxXJc8ZwIwgVMl4myS5T80WnFulNno8ZeOPQG0kIbxKMjZ QNt4eOKxb3MinqypsxVQtrySgSfoYmVE2sqb8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738294493; x=1738899293; 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=32D3HUGqP+ZFdFEF3wCZMSpfCGFHL5f2rNBX8ncFDZw=; b=mc2MD4DR7H75k4k7LJZUWnPtW3F4TkCU+JuFel5ep+aMuTR3NFqa9JqeQCgZNnrAaK 5IP0g90pxot6x4hrf4nt9NSz4prZ/+Kvpy7qoFDEWAHDHrnTllemKPObU6sXKS3qAITI +G0RAv2K6hGFcLQMH/AfyhJRfLhWsMRyxsM5WKk80CE1JKWqGmCKn10M5GAuMIE8pV0F HUCppk+luEW0ymS2B6TrIqeQPqCZI+WzKPqTfe7QiL2xAr/y6ut6CLom3KanIKIhwiVO ZqVupZRlln/0JtQqQ3PicnPsXqBcVKmHxsctVDytHrT5o2VfQ/HD1EuSX67mj3rzKdjC yl7Q== X-Forwarded-Encrypted: i=1; AJvYcCXg5SCBdRbdRMM5JV+NnybZ///wOeyQ9IZgiNGHK370r2xzUUMGUSFIo8HUtAsX6tCY+lHcq9Yl9w==@kvack.org X-Gm-Message-State: AOJu0YxsG75gGgV0/eTDObdV2W0xhFCLXBs6xXyxhMAuOp/KuFa59D4b W0NwN62RHzUQz+Cmy10EEAFlPahWNf7K95mXJXO3tfeDeWa8Jn0GfgY8cInfdQ== X-Gm-Gg: ASbGnctZFW4NF3AcL9VPnOlGoWKAfkdw+etZZqYZXTAolEAz32bAspEZLtBaU2r45OV dXNLNwIH4AvV9j7crJ6l2yNsF9vv3OasNVIG3xaO+kK17o/qGNXXlvbCX/C58SKidPn/JT4QQzn RhlicKAanThNORiXUXNZuzlq+8pmionuwNl+wPay49bm7HpvaKhznIYpZeG5vKSvsaTdylDmOIM 68MKLxS75nU0F4AbMqptK1TUxvFwVO2zO5Z+QLm4tysciMmscOSK5sexLsZpnH0sDPAzL9bXjSJ qIW9HCUE1dQLwYQ2/w== X-Google-Smtp-Source: AGHT+IEsFmLz+qIkj1EMr+GV0ijOaTbWPrqW3Z9zAfyz97XMlXwIn3rQRlPiOINIpfAoYRkCaZtcHQ== X-Received: by 2002:a05:6a21:78c:b0:1e1:9f77:da92 with SMTP id adf61e73a8af0-1ed7a5ee31emr14721480637.33.1738294493029; Thu, 30 Jan 2025 19:34:53 -0800 (PST) Received: from google.com ([2401:fa00:8f:203:d4ce:e744:f46b:4fb]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72fe6a1a75bsm2224113b3a.167.2025.01.30.19.34.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2025 19:34:52 -0800 (PST) Date: Fri, 31 Jan 2025 12:34:47 +0900 From: Sergey Senozhatsky To: Yosry Ahmed Cc: Sergey Senozhatsky , Andrew Morton , Minchan Kim , Johannes Weiner , Nhat Pham , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv1 1/6] zsmalloc: factor out pool locking helpers Message-ID: References: <20250129064853.2210753-1-senozhatsky@chromium.org> <20250129064853.2210753-2-senozhatsky@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 87BD5140002 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: u9u6o6843f5zestgsd3d6wyytzgq6hpj X-HE-Tag: 1738294494-162409 X-HE-Meta: U2FsdGVkX1/JsZiES+PQiMScMezscu/x6ckjO2mlvaysCYRvQ7t0xWs5QQkgBtTGNR1Kz2oB1+ZCJIBbwLCQroWgdlknHv2IVx+RU5afPHACT4d2RY52l8ASkncgcxafTHRjuyobwATdeD3pB4t9ie6oHRKHhrwNdN0RHV83illvSGYzNhDlCoUOU0YROa0yOh6XC/ggMDygtG5p77JcDpTIQcntheGo/9NMwMd4V4oZhokoQFvOUFP4jTJH2Sr8vGsENWD/DwWytXUF8PXdQmYQcobDxecP2LPAX1lZgz/qlRsGIfaJRhmBDTNNnw7HKsdadmKw6Zbls/h2Z3PwzO36xUFCfcCC1PlmQL0fGrnYR1GKUiC52A5RnCEqTq37caePI5ataeUaRAQaIOLieWm5T7Lk0L0AqRCsOewHVk8Wy8BeDWBGXoo+G6SYdEWBlIEIvFkHRXzE70SMTUPUOjl/XeqdM8FZuUkM3ZCTu2aB9MTQrXYMpsbdLZTAFnQrIK+a9WJGUeVVtuWeaQzyz7EdbYrcyBfebLOamMxgBl93fQs68U/OJrT14OwfKI9kScHggec2xULbVPiK5BLlxfnnvQfQOujuE119Cndhe0+gfEX0Ssdr2fkmWp0SnbaaDI9Y/7zihxm+DSsmxY2E1Ghz5UVCdjW861DfnFLZqiPEpDaXKcZQeKvxQpjk2QIYNfj/XgWRnbhAMoyaQYkg4LmeYmpDGmFkCb4yBpxQFmE6nWZsPtoGR0lx68AeEGlIwx8hQyRMHWQ48eEw5UtMszR+4za8zlY0V9xwL+uxHD0L6n7KY8cgt0nhh+9ucC64FtIf46olfvZUzWAXMKAi7e5JqM4f8PUoTnt0+pzzIO0jbvvhB2spUyOnGw9UGlxtv156zFIL4DZjLRGIK3vA5EauZp01zL13bNvfqeF3VIg4rtV5aEgv/miTsd9cqLSVX6djfTluVuLbv5cCvmB QlYrXXoy fdoUC0ElEaooq3bm3ph7FamdqiM7mE7VcXWEHqfOXyNXwpuaTo9S3AU0Qx3iY7OpguMRIPMxOch5rx1YnSRUddI64EG3HHe3/UU784baNa56klQGIa6HBngeYS/Jn1ArmF1oqCQ0ousk1g2gUoOxgzgt/GyW48WBP/VwyAUrWWcGr6Dk40O1YvI3fWs5JnSKClOIY++eUS9SVohOP11fXgPCTXzMIh9Yuv8ZXDbHDfHbk2PCpRGxxCBHHC0kJX9N3gz7O4/NvjjdVmvlsm9+V61KQUxkqrbhri9Ojdw7iMF0gB75S/qAUkmgx7HR63qGzCOG+RID3wLqwKmPVDqhiECUQ7gs8V8YRHK2Pv3qEPmko2uw= 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 (25/01/30 16:25), Yosry Ahmed wrote: > On Thu, Jan 30, 2025 at 01:01:15PM +0900, Sergey Senozhatsky wrote: > > On (25/01/29 16:59), Yosry Ahmed wrote: > > > On Wed, Jan 29, 2025 at 03:43:47PM +0900, Sergey Senozhatsky wrote: > > > > We currently have a mix of migrate_{read,write}_lock() helpers > > > > that lock zspages, but it's zs_pool that actually has a ->migrate_lock > > > > access to which is opene-coded. Factor out pool migrate locking > > > > into helpers, zspage migration locking API will be renamed to > > > > reduce confusion. > > > > > > But why did we remove "migrate" from the helpers' names if this is the > > > real migrate lock? It seems like the real problem is how the zspage lock > > > helpers are named, which is addressed later. > > > > So this is deliberately. I guess, just like with page-faults in > > zs_map_object(), this naming comes from the time when we had fewer > > locks and less functionality. These days we have 3 zsmalloc locks that > > can "prevent migration" at different points. Even size class spin-lock. > > But it's not only about migration anymore. We also now have compaction > > (defragmentation) that these locks synchronize. So I want to have a > > reader-write naming scheme. > > In this case shouldn't we also rename the lock itself? Yeah, I guess, I thought about that for a second but figured that on the grand scheme of things it wasn't all that important. I'm going to resend the series (after merge with zram series) so I'll rename it, to pool->lock maybe. [..] > > > I am generally scared to move hot members around unnecessarily > > > due to cache-line sharing problems -- although I don't know if > > > these are really hot. > > > > Size classes are basically static - once we init the array it > > becomes RO. Having migrate_lock at the bottom forces us to > > jump over a big RO zs_pool region, besides we never use ->name > > (unlike migrate_lock) so having it at offset 0 is unnecessary. > > What's special about offset 0 though? > > My understanding is that we want to put RW-mostly and RO-mostly members > apart to land in different cachelines to avoid unnecessary bouncing. > Also we want the elements that are usually accessed together next to one > another. > > Placing the RW lock right next to the RO size classes seems like it will > result in unnecessary cacheline bouncing. For example, if a CPU holds > the lock and dirties the cacheline then all other CPUs have to drop it > and fetch it again when accessing the size classes. So the idea was that offset 0 is located very close to low size classes, which are a) not accessed that often and b) some of them are not accessed at all. I don't think I've ever seen size-class-32 (the first size-class) being used. I think for some algorithms compressing PAGE_SIZE below 32 bytes is practically impossible, because of all the internal data that the algorithms puts into compression output (checksums, flags, version numbers, etc. etc.). > > zs_pool_stats and compaction_in_progress are modified only > > during compaction, which we do not run in parallel (only one > > CPU can do compaction at a time), so we can do something like > > But other CPUs can read compaction_in_progress, even if they don't > modify it. Yes, well, that's the nature of that flag. I'll drop patch, I guess. It's not worth our time.