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 C99B9C282DE for ; Mon, 10 Mar 2025 17:59:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCCCB280006; Mon, 10 Mar 2025 13:59:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D854A280004; Mon, 10 Mar 2025 13:59:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C4548280006; Mon, 10 Mar 2025 13:59:34 -0400 (EDT) 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 A6B14280004 for ; Mon, 10 Mar 2025 13:59:34 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id B1A06A9926 for ; Mon, 10 Mar 2025 17:59:35 +0000 (UTC) X-FDA: 83206403910.26.F9E1E47 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) by imf10.hostedemail.com (Postfix) with ESMTP id B799BC000A for ; Mon, 10 Mar 2025 17:59:33 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="C7qvBGE/"; spf=pass (imf10.hostedemail.com: domain of inwardvessel@gmail.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=inwardvessel@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741629573; 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=sNBr4INgu8tA+aI03AqyX19SI22Ahkw2EyMlvwGrywg=; b=PLCAkExG7Cl8PUpVRZHAogc3ODfPLGRfxRQ4fdGEmg6lX4o32Ky+5CRTUeKfAYa8LRngMJ VUlvE3Dcl1nnl/Nlt1ols5iv2CTkbZGmSfqdQcT8FrSfH38bqfAHZrmsCmI+sdOb/d69ZF cvkRMXqNGmuwgvXepO7jUxvf/vLFTqs= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="C7qvBGE/"; spf=pass (imf10.hostedemail.com: domain of inwardvessel@gmail.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=inwardvessel@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741629573; a=rsa-sha256; cv=none; b=Eebo6CN5sEn5N4TFKRjkQHdFVRt3asdvnrJUKWy5DXUyC71JchszpDnXHo1sogz3jdia6J JDpYGfg5wqSTwkbecl8qxD0JuZYh9+1t1lmx+OLkVmbzbwUUGmWKVU0tH7IC6/ZAH4NY1Q RWxE5u15bImkzCvSloryyrvv2uOj+Qw= Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-2239c066347so79627675ad.2 for ; Mon, 10 Mar 2025 10:59:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741629572; x=1742234372; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=sNBr4INgu8tA+aI03AqyX19SI22Ahkw2EyMlvwGrywg=; b=C7qvBGE/SstPP8fqIWBWooYyBtpcn1BrdWr+KwqcdlOHHiAKhRKjUuIsLqUZUXurSb 9ymSdJqZKo+U4awqubdgtTDt0Ux7m6GS9bquAobRslFEPiGAgEBqa3Z2x6XRZI0GrOsG JJjKRJhPKqWJJzrKBQKn5t48PrVY22IKnjemCuuL0BpinskK+jD+JH2XylcHkXuLCKT7 T38h1yblHnLYcNCwjnwQEWb3ZNAAIkRvP/SrxV78UQFeQK5VKIdJYZDXmU2AFl34U6lI lb7qZdCZXOT6DUz275grtV30cLDvzoZgK3CPf9sg2nw94hOM/9uwev98K/XS6fDPxxHP neGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741629572; x=1742234372; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=sNBr4INgu8tA+aI03AqyX19SI22Ahkw2EyMlvwGrywg=; b=D1eFsaLZJZQzV0ALWVnkIBUO1yq6AaIdC0xbLy5uo/8vlhKcTZEacxqhomu1iddObH WQSB6dFGPR+WrDqzhM8e7DbH4u10qSZFZdZ4ol3HAmxo8EabSfvmP4bKF0sG8OlQCvZA vlNN12ga1qt2ShPlWx5gz3pIwQLg0tokl+3CnfY9iv792Q7Fxmo6loxtxzUIbdV1RxLy xDDKD5dRjmxPissf3qfUyGQNJ5eHGNwAm7GT3NhMMQ1pwOO1+938Vh0g4ZVctqMczIpC tdE+QodDJWE+OCpIVeljISF/qiOqKRcd1bo4GPNV4OZpNFGNN5XuPsdkhiuCA8vgty4f vbHQ== X-Forwarded-Encrypted: i=1; AJvYcCVLb5CxfuB5rAxwjV1E7zjciFkZyzNKqJ+6DGEioWHDLYU0VFaMQRwD7Gt1QH6NCcT+525ArS8fWw==@kvack.org X-Gm-Message-State: AOJu0YxVkIwqF3kSDLq59follsM4oEaMPzTOIyb4uokn2JRF7w0T4uxZ zjjlJ+ilS+6jwk19GyfDF9jKBrPIehNMgu7IC8usdvzAkWtS9o0Isxil8g== X-Gm-Gg: ASbGncsgCgZARFwRPJ0Mg0IkLetd16PFrojIG/sxA7fYYOh+/wcKbABqjCse5RFL6zJ N10XalFaBhIl833qjUqRdlMXlXN5fWv31rlp1R/I/9/cqJyDoXHtdft0zziuB+AzT1u3SY96IPw b4mThu6AgndjRFBAAkkXh+CjGHXP39KzuABlXcTonBybfZErO7RSqfCORnZ5OtIClNTGFyM7seR ozCSt62SOw1zdS3rkwHeHH1iixL0oX1jkVgod/ElEV9Av0NI3c0R2RccN/YmRVlIX2j7bDU4Liz 6cvZpVsG6b4ceBsF+9OrUvG9cbEGyPs03Z5t8MdSwAunCE+PlcpYqM92ZiT07C4BBw41qkT5xGS PvQ== X-Google-Smtp-Source: AGHT+IHjl1aHKMZyxpEhQWcWi+UKBfOyPwBhyBPqoBuUmfXOH5Ljw9HIYiAxibizFH/qZI5kLXoDmA== X-Received: by 2002:a17:902:da90:b0:224:3db:a296 with SMTP id d9443c01a7336-22428881fefmr246310595ad.2.1741629572569; Mon, 10 Mar 2025 10:59:32 -0700 (PDT) Received: from ?IPV6:2a03:83e0:1151:15:9d6d:b3fc:984f:cf7b? ([2620:10d:c090:500::5:9a53]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-22410a7f93esm81183565ad.142.2025.03.10.10.59.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 Mar 2025 10:59:32 -0700 (PDT) Message-ID: <9c50b4ac-7c04-45ff-bf42-9630842eec21@gmail.com> Date: Mon, 10 Mar 2025 10:59:30 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/4 v2] cgroup: separate rstat locks for subsystems To: =?UTF-8?Q?Michal_Koutn=C3=BD?= , Yosry Ahmed Cc: tj@kernel.org, shakeel.butt@linux.dev, mhocko@kernel.org, hannes@cmpxchg.org, akpm@linux-foundation.org, linux-mm@kvack.org, cgroups@vger.kernel.org, kernel-team@meta.com References: <20250227215543.49928-1-inwardvessel@gmail.com> <20250227215543.49928-4-inwardvessel@gmail.com> <6no5upfirmqnmyfz2vdbcuuxgnrfttvieznj6xjamvtpaz5ysv@swb4vfaqdmbh> Content-Language: en-US From: JP Kobryn In-Reply-To: <6no5upfirmqnmyfz2vdbcuuxgnrfttvieznj6xjamvtpaz5ysv@swb4vfaqdmbh> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: B799BC000A X-Rspamd-Server: rspam03 X-Stat-Signature: q6crdwgc3i4cac1zpi74s1skpr7i5twc X-HE-Tag: 1741629573-331208 X-HE-Meta: U2FsdGVkX19dcQYEorpTOtSK2GIZMJ0AoF5RZk/eMhlT218oghCvbJ8R3jMXtB0XzstIn8kQXwZW7CZML607eNOeS/StbAMx7Md7V/s+X+kMFgbdEUUZ3IjPuKtGrdu4hUrQqWjD53OE6mqse5mVjPGQTS5Nw9QEszgzCT37zhjpyM6tqW05XcD13W3ciyRFXZpbGVrkHvUJLeifT4pRlwxgpumOlhkbwueYCa7pgmGtaGo6RuTYKhtjJC1C/iOkfug48iB11Uqy5K+NpoJxFTuYPSYRyIO/MNRRwG8QWKeWiV85YdX2YAWfjUOgZl76v81VygALDO75M9nF7uAGNxyGFQabNl03buHt+kKBwut/xI8KW+nhpGphjoxmuQOFbqJyfb0++vfE4VxlTchdB2gyG8e/juMbjAiN70YLYsgKeu+lQd0Klw7Ixg2LGHOJFdn8OapOlM7ProPyfSdwsPhGmvYXMafBJJVMEipf6PGblH5gqYt054P0rP09ZVxqtzG742xQdcEo29g3uL9RhlURekMVsYw0ggmsEkQsVm9JJwCvc3OS2FSO+y0Vshk7v1AQNlXOYslT3J0vDju3edeMqceZYUPRB6TrNqJo5MdFotJAwCdPDhgFW71Whsbw1KZCgZPnToSorMI+fB6E2fmTH9YyJJEPvUB7L7ByTL2slqXI8iNIm+5ePdn8m8fZGSxzvQWJaQ9J1Ac2u8s0iOZEJ2gguzdcIPn0dT0/DspOA2Xsm8Zk0XDvOcQ0YlWIO7v8Ol29BdfqeWSADO1l1Yas3ZyGfyraJhsd3XJ3LRx1p5wC1/+xKpc21ECR2xrTdwS34Evdkt9ZJTdPIZhNA3aQKW2Q3v77nKDdDvpSkR/4WBsUGStq+BEqEa9ho/XUsJ18eBKMpcdKGkbEPKGJfHNXGx701CdblZTrvm/4vOLuuH6fOQnQNwInAWBmfwfnxtmnY2bRemfyjYUzzQv PT3Oviae 9iXb4GiurQR8ZgKEdXFRP5P577VYxEdkXtx7o72UxA/hQVM7JCdstIXsJdm6Lcz3Z1DXlrFZJHarUvWuivfSuAFewxayckBesq9MZM+Qg7883zCa9A+bKELp1PFnxZ4SXzrTOPZ+U3rZhxGNiCsNI6X7B7KMSY4a7Ij2bYGuYlBPOJfwEVIPVSs1p5PrlSJF2mpg4kh5131wyPb3RLPUyBofNoYPej/Er9aDlRGOhEoW55rCghkI+2kJY5Q2ygQnQAx1NYuHN2W26siUCcQ3I9JXeEGvsiDjXjSIItGlrH7U1hX8W4nQn7CvavCxDc2DhY95G8z39bVAQcIYS5GwSbmbrEsX9I2OMqPGin0FIvx/5xNiZCvMKmQ7HDxm09oT1Yy1chtU69VLMgJVqtlO3jO3zK/Sp7m5G9arH/53MkEfmJ297GL/HwsKc3FdJwIpSoZoLgctYWaj2jwoc0ne9LVYB9w== 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 3/3/25 10:49 AM, Michal Koutný wrote: > On Mon, Mar 03, 2025 at 06:29:53PM +0000, Yosry Ahmed wrote: >> I thought about this, but this would have unnecessary memory overhead as >> we only need one lock per-subsystem. So having a lock in every single >> css is wasteful. >> >> Maybe we can put the lock in struct cgroup_subsys? Then we can still >> initialize them in cgroup_init_subsys(). > > Ah, yes, muscle memory, of course I had struct cgroup_subsys\> in mind. > >> I think it will be confusing to have cgroup_rstat_boot() only initialize >> some of the locks. >> >> I think if we initialize the subsys locks in cgroup_init_subsys(), then >> we should open code initializing the base locks in cgroup_init(), and >> remove cgroup_rstat_boot(). > > Can this work? > > DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_base_cpu_lock) = > __RAW_SPIN_LOCK_INITIALIZER(cgroup_rstat_base_cpu_lock); > > (I see other places in kernel that assign into the per-cpu definition > but I have no idea whether that does expands and links to what's > expected. Neglecting the fact that the lock initializer is apparently > not for external use.) I gave this a try. Using lockdep fields to verify, it expanded as intended: [ 1.442498] [ss_rstat_init] cpu:0, lock.magic:dead4ead, lock.owner_cpu:-1, lock.owner:ffffffffffffffff [ 1.443027] [ss_rstat_init] cpu:1, lock.magic:dead4ead, lock.owner_cpu:-1, lock.owner:ffffffffffffffff ... Unless anyone has objections on using the double under macro, I will use this in v3. > >> Alternatively, we can make cgroup_rstat_boot() take in a subsys and >> initialize its lock. If we pass NULL, then it initialize the base locks. >> In this case we can call cgroup_rstat_boot() for each subsystem that has >> an rstat callback in cgroup_init() (or cgroup_init_subsys()), and then >> once for the base locks. >> >> WDYT? > > Such calls from cgroup_init_subsys() are fine too. Using the lock initializer macro above simplifies this situation. I'm currently working with these changes that should appear in v3: move global subsys locks to ss struct: struct cgroup_subsys { ... spinlock_t lock; raw_spinlock_t __percpu *percpu_lock; }; change: cgroup_rstat_boot(void) to: ss_rstat_init(struct cgroup_subsys *ss) ... and only use ss_rstat_init(ss) to initialize the new subsystem lock fields, since the locks for base stats are now initialized at definition. Note that these changes also have the added benefit of not having to perform an allocation of the per-cpu lock for subsystems that do not participate in rstat. Previously it was wasted static memory.