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 8FF5BC282C6 for ; Mon, 3 Mar 2025 18:30:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0B5EA280002; Mon, 3 Mar 2025 13:30:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0651A280001; Mon, 3 Mar 2025 13:30:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E6E84280002; Mon, 3 Mar 2025 13:30:07 -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 C89A0280001 for ; Mon, 3 Mar 2025 13:30:07 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 7BB5A120E2D for ; Mon, 3 Mar 2025 18:30:07 +0000 (UTC) X-FDA: 83181079254.30.BE0AC10 Received: from out-172.mta0.migadu.com (out-172.mta0.migadu.com [91.218.175.172]) by imf29.hostedemail.com (Postfix) with ESMTP id 8ED38120014 for ; Mon, 3 Mar 2025 18:30:05 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=WLWcPu2N; spf=pass (imf29.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.172 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741026605; 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=aYe/ysoXPqob+w5PBERcOUKELy+j85iPqEHh+yndyrU=; b=F9bfgZW3Lhb8gk/tLu0gHvBZ6H9cnEFDsfqcrMs40JvPibPBkJKzlUqr6i8ZYmqciyEAog XRg9SYlSLEAqQKvD65RtAR4ADbPDBwBLrJU3CXT/FVKVsYdjP7/c7qEnsgGy6C0hdR/mf9 OISVNGm6KCcRMqYi6oAmFCFP9P2yUGc= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=WLWcPu2N; spf=pass (imf29.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.172 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741026605; a=rsa-sha256; cv=none; b=Xwu35wE39CNH4KvjPvTbKO+UXJlWFSY8I/i4q+sn68BVFPol9jdFZhZYXWHUMAZLYrLZQw wDiYjtlbysGaVGJyrVhRWKobk0yLsuLloeeQTJwKvhu13ykkmI99mlsEQqHlmyB6XpvL/s Wh0zXk222B3gGbRtx3Lm21JyVT8XRhY= Date: Mon, 3 Mar 2025 18:29:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1741026603; h=from:from: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; bh=aYe/ysoXPqob+w5PBERcOUKELy+j85iPqEHh+yndyrU=; b=WLWcPu2N4Ygl/oFh4nOsmorI3MtdySSTP5StfdE2ANLNfYS29bfXKIg68hhrkBkgNEUKEj a8dmKXQxudIQbPlm+I2gzXJXMJZ6SjrftZkOhCTdBXUK0RdoHEWa4i5XLu2+4Sm0pIxP7o aj57ygZ/QKnwSnJW9Q+GHH+qNxD8jp4= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: Michal =?utf-8?Q?Koutn=C3=BD?= Cc: inwardvessel , 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 Subject: Re: [PATCH 3/4 v2] cgroup: separate rstat locks for subsystems Message-ID: References: <20250227215543.49928-1-inwardvessel@gmail.com> <20250227215543.49928-4-inwardvessel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 8ED38120014 X-Stat-Signature: 5cdw1bb1agp5pm5tm1sad5a8rhgtw3yq X-HE-Tag: 1741026605-788244 X-HE-Meta: U2FsdGVkX18G/jlEPNRUZlefvLZebEr45snmlk7RXAYH2kwPLIqsnSzlByKHKEMPnfWaqzKT97+p/VgIOjnxOJ7+MscB37oFMgNA0SBmEKjMf3ER0f4EbAdOpnZ0+RpItzC/mvPaFU5aedjo1ZKDZ/l3Y/7HRGWvKy6A0RczBmhb9Cr92pW5EckuzUr7SoyA8UHaH37VcLjEsnxmSWkwUi6ar6QQdtaKGoCNSODKepbblCI1Y3H92HljI289PWNeI4G0Gx0vhGsguZxRzPGdvCnLn/+BUJomVJjyGV938Wc9A+qrwNRkH/b4XjLCjM5Lv0XGu8WOxbrwxu55XGDJ75jSqxXj9LWAN2ORUNC0xsW8aQrJrr3Rc+xGVvYS9Bu47u6blDqiouUZHqurIeGfE1Lg4I9Oza9UtwXqzosGhDaOK305Bd91vjiF3luErc3fpaN/6o9434sdrWow8dKJNxhdz6haeM1xReihf87J5zCwvDr17dHyBcpV0bsbMU5ZkXzQ1PTZ41wd9ohXrLyzEfQA+YzMC0cBSlj/OvF4F5ZycGZi2zKTlvPlwMS/Sg0r+I0BaMkcDN6K5mEnq30qW2UI51y/VUBWOCpIbT9kFBtO4GVFad6Uw5gbLTLe/6TFFir9oLK7/MD1xLZ/49D3e+bKlSH1iIrfpnN1/S4bgs5ueZIBp/Mhk11Tlbwmlu5XKrQoSp37c/b4jicfVCVYykqINIEJ4vhrOWPtePq5EIyyKJjfFcC8q489bpUJ8X+dko0/1kdZ8tctr8NA8sT4Fa7nQgY2Rb1Nv3xcaEiI7AbaMMYHlVjxo0uJLQ5ZSwUSYKrwRLG3FtqIDg/dlfBQDCaGMusNlMZ2FOHwO29okfyQ1oD2evrtJxEiaQVhzqRlQddaSbUmpNdT2RS+UZdAM79IavXgdiV1+5FsMcoqL0E0Mhpm1/T8VM08iu2tN+19N7laLKzX5GG1TTs0QFj l1idag86 JDSqNdd1Yg9OMIXrx7YVSVYsSCNSHZLciV4BkscTUfvpVLVX7b4PqRL5vVGEEfdLq3jtshpjVCJete9bMa6vcOnTYF7FbbjVgwp/Ug9W9KcHhP6jePmFyDYHf8khIbzja5yugRwWnXLq073/6+HGtXC/53egzSTjAPO16A9ZY4JdB/vJAxhF2uEJmmZXxjGTUFX3WycZyL94IjXcjkRLdkHclcVevrFImTb+5Wcjw9K9Adjyuz6UCwIszZ57SJsJ1bcTZt3I1nogwSGY= 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 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). > > > 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. 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?