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 4418CC369AB for ; Thu, 24 Apr 2025 17:10:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4487E6B0024; Thu, 24 Apr 2025 13:10:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F94F6B00A7; Thu, 24 Apr 2025 13:10:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2758C6B00CD; Thu, 24 Apr 2025 13:10:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 044046B0024 for ; Thu, 24 Apr 2025 13:10:16 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id EB7591C9F08 for ; Thu, 24 Apr 2025 17:10:17 +0000 (UTC) X-FDA: 83369575674.20.027F400 Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) by imf14.hostedemail.com (Postfix) with ESMTP id 28100100014 for ; Thu, 24 Apr 2025 17:10:15 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=jgJtDIuB; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of inwardvessel@gmail.com designates 209.85.210.176 as permitted sender) smtp.mailfrom=inwardvessel@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1745514616; a=rsa-sha256; cv=none; b=Fjmc48R5GothDSfN2i30mQLHyRLzFhQFRU3td5f5hM7E4u7U4yh+7+y0VjXT83VCx7A+k7 yw3KMoxPgA5WXphWoKnoPFK87LNGXo3kZDF3eFbrGHH25RrjhxphWC5Vh7nEnD/kug9BOl w6MM3GctSttQ7zZrd+8ealJPKyu/Bns= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=jgJtDIuB; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of inwardvessel@gmail.com designates 209.85.210.176 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=1745514616; 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=AoAsMcihc1xMqVGN/kDHY7HQTkDLG+HfsxoiVJN/raU=; b=HpeMmTv8lxZPtQmYJDKblaeYMipvS9oQeInxnevEh6th5BWDoz6TEy6j2FaAXzzeFVbnJd iQ25pcDM1W9jZZ3OVLxOysbeHRmuq4eE0hQLYqQHQlse1DrIRpGMHPMpY7h6s1CjLsPAWL yymFM/+6TpmLSV/z6CBeirEDGg0201A= Received: by mail-pf1-f176.google.com with SMTP id d2e1a72fcca58-7394945d37eso1126328b3a.3 for ; Thu, 24 Apr 2025 10:10:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1745514615; x=1746119415; 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=AoAsMcihc1xMqVGN/kDHY7HQTkDLG+HfsxoiVJN/raU=; b=jgJtDIuBIdupxhbFto+DbVzZmWd5KEM7iHOU5bxlC+d6c3LxFkpP3HR7cGwXspVQz0 NkvAkdMJx8Nhzh3RW90qM/2yDfSiUgSXzCgUmi7nq1Spt/fzcF68+wTIAC5Iwrgs1C4C R/eTSY4OgocwuzNc+iovbqC/t8qoI6x3UuQbA1avsJg59nV7rYVT2hNeFBBjtxDOhb8s IdDrofQjelueeNYjJ69x+ycS9HOnCOzJgE9+O6sKKsOfJgBldGOVmFymdmtGiOzzcIDl aQ2OLIHycbH/0lAppXZxwyu2VB1M7z/UzLG+uEWc68tcI1Ew2+/tiYCWtA9121+sagA4 PTaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1745514615; x=1746119415; 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=AoAsMcihc1xMqVGN/kDHY7HQTkDLG+HfsxoiVJN/raU=; b=vb+02+h9690LhALkFJ7lrQAAz8Rv2HdVFbMTg7oVqHT7hl01DJx6MaUD42KMh41rmj xhzx/lAFUklR+9cChmJPijrUjysoQjIPg8OkHaCKKw3rGykBT+btZeLGlWHixWQfaHbW CjnptA1a6M+TfD3QtC341hs4zQvEw1atD0BcLv4EtcYw4v3V8D/+LiALTGl1377zFFP5 UAqqVp2UbUlKHbGTBoqAcrLhnJuialN30b+6v5VUyQuk9fDuFewT+UCcT76L1UkxzJw0 wcH41pRPgt945YvYHOBHQcX9iSDRhHqU4TMt5XksTLKYVuWjPQemh3TXccasCZ1I6NvJ LoGg== X-Forwarded-Encrypted: i=1; AJvYcCVnVYE15v2BPrger68CYM7Qb7ix5937oOK1uaepCkjuDHrd7UhKpcOyATodz7bQOBuyveVDMNctyg==@kvack.org X-Gm-Message-State: AOJu0YzjM/rAG8eBRIm85gJzreHOAVQDPhEYvo470O/lyqBSFX8ZWJlo i7yww+XyhGSvRGckBTKiDwrPYHU2j/MMDczAyM0XJKkbCR+tkR++ X-Gm-Gg: ASbGncs13Te1zFesZqfr1/HKWx1/dPDsrnVAT7S8ZoUxHb3XPxt3/C5ge+fxEWyDtP2 g6Si4AMMXZJncn4wzqUYnRe41pdgNXdyVzB14ygjdrk1dCWHoGAGCHF2JbSfRKVWnujcNYM+n+P IQP9diu0NeamhO8SUDENs0oiORfNFuiCnnBaqeEizXaKKJHRMpasqPds9DeCXOa3M3innKdzJWE +UIDJUs4c34X9iu328wxD/CZxXdpLn1XumFXZpMAF4ZHogDFolFJImnr0ANrS1Mc8aTem8ylpZB X+soMY0ONcwaQOT7Oasl3ncImktaUT9jPHM5eJVIx7D0G7QR5hZcOIVnoPQaH7pj0w0lCq2t X-Google-Smtp-Source: AGHT+IEyiYe2D0MV4Icd5xkf3+nsK9hIVYlp8alPoA4Fvjuv9WE7h0WKiI4D69ihGnNN9rlEzVQ2Aw== X-Received: by 2002:a05:6a21:6b82:b0:1f5:5903:edd3 with SMTP id adf61e73a8af0-204565f453emr136078637.11.1745514614992; Thu, 24 Apr 2025 10:10:14 -0700 (PDT) Received: from ?IPV6:2a03:83e0:1151:15:fd6c:bb6:36da:5926? ([2620:10d:c090:500::5:5d68]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-73e25912fb9sm1673830b3a.34.2025.04.24.10.10.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 24 Apr 2025 10:10:14 -0700 (PDT) Message-ID: Date: Thu, 24 Apr 2025 10:10:13 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 3/5] cgroup: change rstat function signatures from cgroup-based to css-based To: Yosry Ahmed Cc: tj@kernel.org, shakeel.butt@linux.dev, yosryahmed@google.com, mkoutny@suse.com, hannes@cmpxchg.org, akpm@linux-foundation.org, linux-mm@kvack.org, cgroups@vger.kernel.org, kernel-team@meta.com References: <20250404011050.121777-1-inwardvessel@gmail.com> <20250404011050.121777-4-inwardvessel@gmail.com> <68078d3c.050a0220.3d37e.6d82SMTPIN_ADDED_BROKEN@mx.google.com> Content-Language: en-US From: JP Kobryn In-Reply-To: <68078d3c.050a0220.3d37e.6d82SMTPIN_ADDED_BROKEN@mx.google.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 28100100014 X-Stat-Signature: 15rpzi96p1fhzfja4mtjefjj95dqxrbc X-Rspam-User: X-HE-Tag: 1745514615-41080 X-HE-Meta: U2FsdGVkX1+8Xaa1YDHKsvCxjfY8GKvDJETQNA3JbtQkMNRjHtLKXTmpwDmqHvwtiB/bwuTVVSucC4FfFUblVuSBbrsjoI283dPG1tv7WfL/hKE/rbzEB+X/WOI3bjf1cw4bc38rwFui0FxJlT03vsDLXGzfEocVIOJbIVsQ/yWzoQAPxmdDY14yJ2HZTGKVqEcD6J1U4WenX0/APuGx+762M0ptgUbWd0dxLhvMTeObB+O9SXsGFOgkIHE4wGvDLTbDw5FL3yDPKFac2S31Gx1u/hzWkPlaL1H3VL3G3QI24pzV9P37stBa7JyZt/nESx/JecS+5C3mRqb37oMKZc2YR/d7SVrB25AUrs+1uc7snGqvHoKHEglUSoSMtkEKaHkipZZPyx/aXZYOj4ETzny6M/Cz0H0hvrxirmfjxQx3ZdsQkNwWcblW5YIrvPUHOLBYwaeMFQeelreAU7S+Hj8tiL4r2mVa9Xlg8LpXtvAcsgzFI975uNuTUxCn80cbNQYeYTw/5lqtQhF3Yim2BBloe4SlXCxCt/2vptHU3GlQcmJFztjv2r3O3hKi3IafK9N2Yx3wh7q60wlkJSGgUuWQqsdPVkp31J2EzFOJ4boyVb5pH+VMhphofEzTRF4ViHNQRTk4KgjMfMU2pjw+jCl3xK34FvlLuHiScALZWrqJdbV2K/hQ3SnZDAEFDHsnWczljybwlhJCAH9pilwJKGlXgBuhBBYWwCb7qXdVSNGCobGtlJk4QmhuGX3B7uWG4YysA5cXNQRXyxKip0cWdOPMeoF7PejQnWP5O17HgGrZDwSKrAVkePi5NvVct/Fv6q83MDXES8s5xD7xOMfpIuwz69aN9Upd8KWb9dOjhrTXfCf6ejti/pOc+JBpKfYRCjtwDeeaobw/6nMoYaCPaWA5lcCd1Ttj61tKZrwwnssJC0an1BpzN47f/xScp94EX8/a8Nvuh9V3NTceFCE yHamZn+n vvnH8YY3d6XGMKHd1atAm7adI4w== 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 4/22/25 5:35 AM, Yosry Ahmed wrote: > On Thu, Apr 03, 2025 at 06:10:48PM -0700, JP Kobryn wrote: >> This non-functional change serves as preparation for moving to >> subsystem-based rstat trees. To simplify future commits, change the >> signatures of existing cgroup-based rstat functions to become css-based and >> rename them to reflect that. >> >> Though the signatures have changed, the implementations have not. Within >> these functions use the css->cgroup pointer to obtain the associated cgroup >> and allow code to function the same just as it did before this patch. At >> applicable call sites, pass the subsystem-specific css pointer as an >> argument or pass a pointer to cgroup::self if not in subsystem context. >> >> Note that cgroup_rstat_updated_list() and cgroup_rstat_push_children() >> are not altered yet since there would be a larger amount of css to >> cgroup conversions which may overcomplicate the code at this >> intermediate phase. >> >> Signed-off-by: JP Kobryn > [..] >> @@ -5720,6 +5716,14 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name, >> cgrp->root = root; >> cgrp->level = level; >> >> + /* >> + * Now that init_cgroup_housekeeping() has been called and cgrp->self >> + * is setup, it is safe to perform rstat initialization on it. >> + */ >> + ret = css_rstat_init(&cgrp->self); >> + if (ret) >> + goto out_stat_exit; >> + > > Sorry for the late review, but this looks wrong to me. I think this > should goto out_kernfs_remove.. > >> ret = psi_cgroup_alloc(cgrp); >> if (ret) >> goto out_kernfs_remove; > > ..and this should goto out_stat_exit. Thanks, will invert in separate patch. > >> @@ -5790,10 +5794,10 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name, >> >> out_psi_free: >> psi_cgroup_free(cgrp); >> +out_stat_exit: >> + css_rstat_exit(&cgrp->self); >> out_kernfs_remove: >> kernfs_remove(cgrp->kn); >> -out_stat_exit: >> - cgroup_rstat_exit(cgrp); >> out_cancel_ref: >> percpu_ref_exit(&cgrp->self.refcnt); >> out_free_cgrp: > [..] >> @@ -298,36 +304,41 @@ static inline void __cgroup_rstat_lock(struct cgroup *cgrp, int cpu_in_loop) >> trace_cgroup_rstat_locked(cgrp, cpu_in_loop, contended); >> } >> >> -static inline void __cgroup_rstat_unlock(struct cgroup *cgrp, int cpu_in_loop) >> +static inline void __css_rstat_unlock(struct cgroup_subsys_state *css, >> + int cpu_in_loop) >> __releases(&cgroup_rstat_lock) >> { >> + struct cgroup *cgrp = css->cgroup; >> + >> trace_cgroup_rstat_unlock(cgrp, cpu_in_loop, false); >> spin_unlock_irq(&cgroup_rstat_lock); >> } >> >> /** >> - * cgroup_rstat_flush - flush stats in @cgrp's subtree >> - * @cgrp: target cgroup >> + * css_rstat_flush - flush stats in @css->cgroup's subtree >> + * @css: target cgroup subsystem state >> * >> - * Collect all per-cpu stats in @cgrp's subtree into the global counters >> + * Collect all per-cpu stats in @css->cgroup's subtree into the global counters >> * and propagate them upwards. After this function returns, all cgroups in >> * the subtree have up-to-date ->stat. >> * >> - * This also gets all cgroups in the subtree including @cgrp off the >> + * This also gets all cgroups in the subtree including @css->cgroup off the >> * ->updated_children lists. >> * >> * This function may block. >> */ >> -__bpf_kfunc void cgroup_rstat_flush(struct cgroup *cgrp) >> +__bpf_kfunc void css_rstat_flush(struct cgroup_subsys_state *css) >> { >> + struct cgroup *cgrp = css->cgroup; >> int cpu; >> >> might_sleep(); >> for_each_possible_cpu(cpu) { >> - struct cgroup *pos = cgroup_rstat_updated_list(cgrp, cpu); >> + struct cgroup *pos; >> >> /* Reacquire for each CPU to avoid disabling IRQs too long */ >> - __cgroup_rstat_lock(cgrp, cpu); >> + __css_rstat_lock(css, cpu); >> + pos = cgroup_rstat_updated_list(cgrp, cpu); > > Moving this call under the lock is an unrelated bug fix that was already > done by Shakeel in commit 7d6c63c31914 ("cgroup: rstat: call > cgroup_rstat_updated_list with cgroup_rstat_lock"). Right. I had been working off of a tree that did not yet receive the patch. Moving forward I will be using the tj/cgroup tree. > > Otherwise this LGTM. Thanks for reviewing.