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 62A5DC282D1 for ; Thu, 6 Mar 2025 21:37:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 31E30280015; Thu, 6 Mar 2025 16:36:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2CF75280004; Thu, 6 Mar 2025 16:36:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 197D5280015; Thu, 6 Mar 2025 16:36:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id EFC1C280004 for ; Thu, 6 Mar 2025 16:36:58 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 39381552F5 for ; Thu, 6 Mar 2025 21:37:00 +0000 (UTC) X-FDA: 83192436600.22.D30E6BD Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) by imf03.hostedemail.com (Postfix) with ESMTP id 36B7720008 for ; Thu, 6 Mar 2025 21:36:58 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=RSllrLxP; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf03.hostedemail.com: domain of inwardvessel@gmail.com designates 209.85.214.181 as permitted sender) smtp.mailfrom=inwardvessel@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741297018; a=rsa-sha256; cv=none; b=DpaugozL9RHqy05rxXEsensTofdCTS/ataUTheSNfAEIvZZG+x6Zu4ybIvEpZ46XA6Q+kn ZHn4AAcPa8B/0Gxx6ADQzfX37KbR7J/JTbSV0vTsudq/gw/oUWKNs8NZN8240dXXhsP3Qm ufzVpoHoVBjo6FrEhNwj3KPaLsPneRA= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=RSllrLxP; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf03.hostedemail.com: domain of inwardvessel@gmail.com designates 209.85.214.181 as permitted sender) smtp.mailfrom=inwardvessel@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741297018; 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=3IINhDf6Fw8o56JcRJ5e+93kJRMpCcvNit2/JwcLDIY=; b=waw9Qh+5YPaJq283OiJfxtbv9hXeSEh+fBSDlJo1y+2sawPX25XLIgSzNdlkxfaZ66UJCn RRKqmMMGFr6bv8/5rx7M/VbP94ZxXAexUV4Ou9qVaMUZ+pt6eXVTuKSnM11rzN4ebHMC2p 7f43d2ekXjobKkLDL+NASZ146WSeCP4= Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-2241053582dso25120035ad.1 for ; Thu, 06 Mar 2025 13:36:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741297017; x=1741901817; 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=3IINhDf6Fw8o56JcRJ5e+93kJRMpCcvNit2/JwcLDIY=; b=RSllrLxP7YpMhzddWW4WaqBtgUs+Q411/SEfHcGGHF3Z26fhdZSoNhJl9+jF8QQeGH hRvfb6qQrMT+8Yag2nJdoo3AWxOnyP84WNB4yq4XC9veb6eHQAxAXVtjRiF77xTk1RgV dcZnzKnJMdV3GOkNZrKDvPqWlaNT4w0F0LbKXwJVjU34RzzLpRz9PYMBrCmWDBpv6RdO crS5QGIAjggSUxh4WepnHpXqczKRMN09QdN8XBhP7DUSvroOx55XsTe2NbldGgSsHR7f HdpKfMLj+56YUy0j/qoj/geH0UxHhob9MuLELiq2AANM77iejPsDZnwyYv9L5IXumJpt tMvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741297017; x=1741901817; 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=3IINhDf6Fw8o56JcRJ5e+93kJRMpCcvNit2/JwcLDIY=; b=XOfUys5mBkZXMXuaPATfGACjsSu3uS5EJ2bSGUmMdBKrCvPgQcr5kXOTQv+MrRrKUq hkCCD0nugAXl4OwEKEiBuJ5q5+wtw8jYnr9tMhqam+JWOSoO2Nr34oy0tV0M+mobBSog w6MfRNhAxz6ZkUuVaPfdHWo888MjAzKZcZ81VvkPza5LZS07ZNyaB2ZThO8u1WZdVBDh bMB9H4qZeGqZvT1UXCkt5SZ3T6IHQh8fuwy47TE/F2D+lE6S5Fah+/M09oQj2E+PJIel uXMeHiW8gcX/KXKIoq8RyLQugmvwL3VK7uwMWSdlMI5PUSZD2vTq+N6l1mE1rRHMI52r ESAg== X-Forwarded-Encrypted: i=1; AJvYcCWDuZzqMa8JpKL3SKKkD4+tb+sheexmCx5dU1Bu79lFVsdjsF3mSvFzh542hs+iHEI2nss3U01iLA==@kvack.org X-Gm-Message-State: AOJu0Yxg1Efvg6304PunKcFm26It0QMK/7JyHcRHlf2WYsS4KIURGOx/ 9ARYP3sDqdD3/XCJpKYcDaTTFNisJ8ItuGM3dohaaoU/dREBBPI4 X-Gm-Gg: ASbGncsPa1qVSDKjHm/U31JHQPbrbjb3wRl+t2whyYAoj/fmGKRDFcc0C6gupJTXPBA xAF4XzI7d3zw+Qd6MDF13gGRmB67qjGKbIEOtDP3ADVODLu+LuT9EB4tMN2JUhtyv/KK45ds+Xa u2lSbM55hLB1tO1NHp8rwrCdmzw5wtK/tupgVFazxr0viPJ+nQt7eZBMJgWsvVEDyIMLg8GX+0q E45wEE+C8mvKqIJpvWHyiexFUGdVSM55Thnv3bfhPOfvy4itlRmhzqoPKfE5VMak080Fc4WDbNi tQgZXce1qqVEgq3pu7WsDz+vSp6yM2gu6j0+JFbng0YPPUqzclKVgpKyhLidOcliH2zjTLe4Ruo 7 X-Google-Smtp-Source: AGHT+IFqnaevbR7EZaWbc3xQVjHKp7P167o0Y2c+WpwOmU4hO/E+hRo1NTbAPz8T9gCEEukaEoeIiA== X-Received: by 2002:a05:6a21:a45:b0:1f3:3864:bbd9 with SMTP id adf61e73a8af0-1f544acd522mr1761699637.5.1741297016950; Thu, 06 Mar 2025 13:36:56 -0800 (PST) Received: from ?IPV6:2a03:83e0:1151:15:d431:c86b:892f:8e30? ([2620:10d:c090:500::6:d8b]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-af287c2d533sm1223692a12.23.2025.03.06.13.36.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 Mar 2025 13:36:56 -0800 (PST) Message-ID: <1dd79ba5-aebf-4e81-8aa0-5abdf063d124@gmail.com> Date: Thu, 6 Mar 2025 13:36:54 -0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/4 v2] cgroup: separate rstat locks for subsystems To: Yosry Ahmed , =?UTF-8?Q?Michal_Koutn=C3=BD?= 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> Content-Language: en-US From: JP Kobryn In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 36B7720008 X-Stat-Signature: wu5gc1uwmi4xtauoxo7uj9btwdi74j7y X-Rspam-User: X-Rspamd-Server: rspam05 X-HE-Tag: 1741297018-446218 X-HE-Meta: U2FsdGVkX19Gq3rrCM30eDn6QQkxUJWxJdDrnubEUw8ZNHsQxgIs/iBCswu8nCuj2zScF/gjoux7epoBjhHHH1oZ6xIPIRZozwELEYcJ7uOwVIb8BJ5aWsDI7xXt6AfHO1NQ7XICEq0enb2KmwfUlHAJ+iUq3zNrwovrKQEqcUm2RjsRH3X1mhHMII9S7jyGg+muHzS59c02xuqZ0oHLEyiQpLKQqjQtuD4rm0w3ZvZXB2WIeJsurkDnEMs//icJ9FXzHdR+eOb9c29uX3r9ZFYpgD9CA+rloY+Vxw+enQdsrASRAO0aciGExnsCRzriwQwcQDofFq/AisjhseJ1fEhWtAA/Ak9MZWT598fm4UT9q4VN1bCQgAqS/ikqXevCQGx0PxcxEk0NfxZ6X7HxqPf5sT3mXnXZHqGPavjrH6+S4OI+ukhg1hqmbvz2adIn63CeupHt9Up5A6kzB31zCgJWBx+04/MKdOHbIBl+veO44r5oUsxMTuw6ynBRyAXKSRcV78OtQPSXEn/CjoWZiUMub62hkn9yRPBR1ce+AIvXziuhClxEEpM6ghQLGMXM8eIeBw820K31oMj3v+V4s5WWKMrApo2lj94/K+tgMtUyLQEEQwNlw/8dOPomfIJwgnnDhqDNftmxk4hNlRnxR2o9gajukr4pkcn2oZFOxGNMMLg1+iUB8xXbAAgRWeSCf8fjy/uO1WqHniznGnBBg8mgv2U0Xx88VeHT3gLRohjx1AtiHWkDbo9kYupv/Zo+qvXnp+2z5rOvcue7LA0saS0I65Hsv9qfB1oESV1XvVyPkHxMsi7iQgHVkWeeijTuHLs0WiPMLuiVnYrTx3cMvGbyB6TgnRqJo37v8NzTTSaJaas2oGQ+XUmGKIYbxioxDG070mWhyAdUQalxx/EA/e+WqK8D+NBl2IrnBXsbFjQEg1NB4fH5HmZHVALm+0xjxL0LJQUe4hy2JESr8q+ 7F8s33Z2 AuBTa3+O5QyGQZ/k0kBLK0UJnV5udSgSJdCl+UJaru8Gl79j1OGrjZEQwtRC03D2bRNWjDRHBt/omERQpem0K0+1XRFDKlfkg4uJrNQCaypEzLzF+REXkpd6t3bAf5wuKPNIZ/kBs6Q99Jc3hHOPOBr+XF4PwCcnGJxllSD+5lGJ7bAihriHQB4oENnlVEccV4liZrAwmBTjKwH9FR9sWaxs4j8RqekQayyXtqJTW+lQlp7TK8XTaLOuPhqr5ejyjgNF0tpxHazrcAeQDyDFAynujUuLCVN/G8u50lrTXn14kfHKsdz2+AaP8wMxBziHx3s6oKYhWHoSlCpXq8cmouyI6Vgo118R2RN5D6U/qYxzlfO+0PFD/9qj1RO/y7OgHRuXdSv2U0VxRD5rMrYDh3ylI4hByNd08LwIKe01+9T3wgMeucMeNbAVWTttLiHWxxsjWq7djYkPweDM= 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:29 AM, Yosry Ahmed wrote: > On Mon, Mar 03, 2025 at 04:22:42PM +0100, Michal Koutný wrote: >> On Thu, Feb 27, 2025 at 01:55:42PM -0800, inwardvessel wrote: >>> From: JP Kobryn >> ... >>> +static inline bool is_base_css(struct cgroup_subsys_state *css) >>> +{ >>> + return css->ss == NULL; >>> +} >> >> Similar predicate is also used in cgroup.c (various cgroup vs subsys >> lifecycle functions, e.g. css_free_rwork_fn()). I think it'd better >> unified, i.e. open code the predicate here or use the helper in both >> cases (css_is_cgroup() or similar). Sure. Thanks for pointing that one out. >> >>> void __init cgroup_rstat_boot(void) >>> { >>> - int cpu; >>> + struct cgroup_subsys *ss; >>> + int cpu, ssid; >>> >>> - for_each_possible_cpu(cpu) >>> - raw_spin_lock_init(per_cpu_ptr(&cgroup_rstat_cpu_lock, cpu)); >>> + for_each_subsys(ss, ssid) { >>> + spin_lock_init(&cgroup_rstat_subsys_lock[ssid]); >>> + } >> >> Hm, with this loop I realize it may be worth putting this lock into >> struct cgroup_subsys_state and initializing them in >> cgroup_init_subsys() to keep all per-subsys data in one pack. Will give this a go in next rev. > > 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(). > >> >>> + >>> + for_each_possible_cpu(cpu) { >>> + raw_spin_lock_init(per_cpu_ptr(&cgroup_rstat_base_cpu_lock, cpu)); >>> + >>> + for_each_subsys(ss, ssid) { >>> + raw_spin_lock_init( >>> + per_cpu_ptr(cgroup_rstat_subsys_cpu_lock, cpu) + ssid); >>> + } >> >> Similar here, and keep cgroup_rstat_boot() for the base locks only. > > 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(). > > 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? I like this alternative idea of adjusting cgroup_rstat_boot() so it can accept a subsys ref. Let's see how it looks when v3 goes out.