From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f47.google.com (mail-qv1-f47.google.com [209.85.219.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 919883A1D14 for ; Thu, 6 Aug 2026 19:22:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786044186; cv=none; b=Ahfc1pjTsORec51FLVr7FBwIAm4i2y9i9V6MmD4I/31YPbdtDZ1IgQRmKw1KkuKkZSJfQBlRTmFtrALBmXBfeQHLU36EGi7emNDA8g+LdVL+LlPWpV13Xt0MMWQieXi+DOrynkMFPFX3/Je0aNE8jxaCZ52v2G98C6wbn6kns6o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786044186; c=relaxed/simple; bh=ME7CCmjQSMxbxa8R/0a230jAQXzXTBvmZaSkqnRSTRs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=m1+PDa1woxpFPofV49Iw6YdrQgnVOa5Xuh3zHV5k96dlft7q4k+QQ1PFUni/vHw8KhygCR0O6LExYnUjudBbeHsCjIc+I7X8TvCgNrOaOZQvywAMZ9iyPlhbnziPBsqTQJpXyesQTddN4JC7kDudjraPD/q5jRryOBqb5TtlVLY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=cmpxchg.org; spf=pass smtp.mailfrom=cmpxchg.org; dkim=pass (2048-bit key) header.d=cmpxchg.org header.i=@cmpxchg.org header.b=nq4ZPLbm; arc=none smtp.client-ip=209.85.219.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=cmpxchg.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=cmpxchg.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg.org header.i=@cmpxchg.org header.b="nq4ZPLbm" Received: by mail-qv1-f47.google.com with SMTP id 6a1803df08f44-908239232adso7920806d6.1 for ; Thu, 06 Aug 2026 12:22:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg.org; s=google; t=1786044172; x=1786648972; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=Ed/LSBroZxO0PfqLTAzd0hHdaL+KK4UGWxAVAlrHj6I=; b=nq4ZPLbmS95ANh0DLp/PNHeV0PsEKwswoBK7SaMtY/pGormBLk5+i/JeFCGtkBdRf2 PczvFwzLgGQBETd4n3wb9LqZm72B+Ucf4LOIbtDZfHQMBjkOPih4hlduWLIH6kLWrGHy 4R1yYbIh9Hd0/Lsy3hK02LR2W4STMMBmexQGMeq/vo7yAffMI7d3/nEMi18F3vUXiVEV X4WT9lQh6xJW5RRb+vQKXEONRruDp6D7cGRv5nNM4OZMh53AydJYwT2g6Sei+/Zx3UI9 PSXACM73PUlhG1UYS6QmelBoqR9qnsswfoGCmDAsnOA5B2dSXMpGz7v4+47xBK+3A9cf cA8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786044172; x=1786648972; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=Ed/LSBroZxO0PfqLTAzd0hHdaL+KK4UGWxAVAlrHj6I=; b=iiUnFz5cqEro2KmXP1xUBS9aA2DA0i1recyn+vL4z3LCSCU8UZsYFDTq5Q4ULkuz3+ xRYgSiMl+xd6Igo3Vw7jNY3lBcswssWFJcaYAO9TM6GdmsmR808cv6C0Zm8Np77UVnWt Miyv/kjUtJmiiHeSMnUxVjpKX+lmLPKMBCgxKPi5xeBLWn7LE+lBt7W6URVDWp4EqNyc nL/SMRc09B3anN80qMNn/IQ7x+8l0cXZe5oTrfHpDebWV4qKHd5Mruy2Qgf9raWegfle 8+7R6QL3Zbpp6HXkIwEKSH8SJwayIjfwc0SurAVweJiHVRzyEc48C+3SJkd95hLXWvIq FHGA== X-Forwarded-Encrypted: i=1; AHgh+RrRTYQ9ld1yY+Kir4qm91DW3gvDfZQCa/8tzgpSG5s1jyErP5GRCRFnt1cd8Clj1VEd5fDJj8oD9GzmoCY=@vger.kernel.org X-Gm-Message-State: AOJu0YwuoM8tgGlVBn/yGGujAXSTOOegsANmeQynfTw16ves7iffvdSs 2FZYfs9Xh9G2yIuwkiYHskm6tsy0bCTpqy1zUH7PrW7aiZDvnulKKDMVECizSGSTrns= X-Gm-Gg: AR+sD135GdPciTJufmYjjdXcP7N+mEnuK/WxokIbUzaOxL1FP7b65coKueTMyqQFBj4 ZX9uYAuL2UGctrn6EKsq6pmCNjmf+RMDFaeiRQFs45ukM35fW4se+A7oWxQMoLMKrjDcuy/tbbg VOQAfiFrd9jyj6RCbkfpchm15hSmtPy2gson+k2s39DJJ0Wy/8WzezRP3fH/TqusweW58H1FNfa MCOb0jHb8ZYavPSTgoHbqn0+2+Dhdc9ioiAucjuKxgLgRFgM71L9Di2+o6O5mJpsym3Po7p9Iac SVCeeHdgkM852mXYFheQiiHHs62wjXkWGcEkQkvmualBL+EH575wvWG4UGWH69URfPrz284Wskq Z0PH2Zi3w0yHUqBhZKNUgLw9LWXs5S1KB7C8psRYNp+W3tu70sIT/3QxjJD+uLWQq9avf/fdpmk zj+rgvGqolRwYz908aJgFJInP1QRf534Zlj44PS+tC/K+QbPyGA0/Bsl2aKBs= X-Received: by 2002:a05:6214:3210:b0:8ee:756a:bc32 with SMTP id 6a1803df08f44-90893232a46mr86763646d6.15.1786044172481; Thu, 06 Aug 2026 12:22:52 -0700 (PDT) Received: from localhost ([2603:7001:f100:500:365a:60ff:fe62:ff29]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-908800cabedsm58786346d6.46.2026.08.06.12.22.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Aug 2026 12:22:51 -0700 (PDT) Date: Thu, 6 Aug 2026 15:22:50 -0400 From: Johannes Weiner To: Shakeel Butt Cc: Andrew Morton , Michal Hocko , Roman Gushchin , Muchun Song , Qi Zheng , Meta kernel team , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Karl Erik Hofseth , stable@vger.kernel.org Subject: Re: [PATCH v2] memcg: keep folio's objcg same as its node Message-ID: References: <20260806165813.2526415-1-shakeel.butt@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hm, it's still not quite right. When there is a root mismatch, we cannot fall back to committing the old objcg. That would reintroduce Karl's issue. Not as broadly as before, but can still happen if the memcg tree died up to the root. So we have to commit to the new objcg, always. What obj_cgroup_is_root() then comes down to is whether uncharge will balance the page counters or not. mem_cgroup_replace_folio() gets a new charge for the new page. We can just conditionalize that right away on whether the new objcg will actually uncharge. mem_cgroup_migrate() currently trades the charge, but that won't work if the new objcg is root and won't uncharge. So we have to settle it right then and there. This? @@ -5319,6 +5319,46 @@ void __mem_cgroup_uncharge_folios(struct folio_batch *folios) uncharge_batch(&ug); } +/* + * An LRU folio must hold the objcg belonging to its own node. + * + * memcg_reparent_objcgs() reparents a dying cgroup one node at a time: + * the folios on that node's LRU lists move to the parent and that + * node's objcg is redirected to the parent, atomically under the + * node's lru_lock. folio_lruvec_lock() relies on this to provide a + * stable folio<->lruvec binding. If a folio holds another node's + * objcg, its list membership and its lruvec resolution change in + * separate lock sections, and an LRU operation in between can re-add + * the folio to, and strand it on, the LRU list of a dead memcg. + * + * So when migration transfers the memcg state to a folio on another + * node, re-derive the objcg for the destination node. If the memcg is + * dying and the destination node has already been reparented, the + * lookup walks up to the nearest live ancestor - which is also where + * that node's LRU lists went. + * + * Returns the objcg to commit to @new, with a reference for the caller. + */ +static struct obj_cgroup *get_migration_objcg(struct folio *old, struct folio *new) +{ + struct obj_cgroup *old_objcg, *new_objcg; + int new_nid = folio_nid(new); + + old_objcg = get_obj_cgroup_from_folio(old); + + if (folio_nid(old) == new_nid) + return old_objcg; + + rcu_read_lock(); + new_objcg = __get_obj_cgroup_from_memcg(obj_cgroup_memcg(old_objcg), + new_nid); + rcu_read_unlock(); + + obj_cgroup_put(old_objcg); + + return new_objcg; +} + /** * mem_cgroup_replace_folio - Charge a folio's replacement. * @old: Currently circulating folio. @@ -5347,21 +5387,27 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new) if (folio_memcg_charged(new)) return; - objcg = folio_objcg(old); - VM_WARN_ON_ONCE_FOLIO(!objcg, old); - if (!objcg) + VM_WARN_ON_ONCE_FOLIO(!folio_objcg(old), old); + if (!folio_objcg(old)) return; + objcg = get_migration_objcg(old, new); + rcu_read_lock(); memcg = obj_cgroup_memcg(objcg); - /* Force-charge the new page. The old one will be freed soon */ + /* + * Force-charge the new page. The old one will be freed soon. + * + * The rootness of the committed objcg decides whether the final + * uncharge of @new goes through the page counters (see + * uncharge_folio()); charge them only if the uncharge will. + */ if (!obj_cgroup_is_root(objcg)) { page_counter_charge(&memcg->memory, nr_pages); if (do_memsw_account()) page_counter_charge(&memcg->memsw, nr_pages); } - obj_cgroup_get(objcg); commit_charge(new, objcg); memcg1_commit_charge(new, memcg); rcu_read_unlock(); @@ -5373,14 +5419,15 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new) * @new: Replacement folio. * * Transfer the memcg data from the old folio to the new folio for migration. - * The old folio's data info will be cleared. Note that the memory counters - * will remain unchanged throughout the process. + * The old folio's data info will be cleared. The memory counters remain + * unchanged, unless the charge moves out of a fully reparented ancestry + * and has to be settled (see below). * * Both folios must be locked, @new->mapping must be set up. */ void mem_cgroup_migrate(struct folio *old, struct folio *new) { - struct obj_cgroup *objcg; + struct obj_cgroup *objcg, *new_objcg; VM_BUG_ON_FOLIO(!folio_test_locked(old), old); VM_BUG_ON_FOLIO(!folio_test_locked(new), new); @@ -5401,12 +5448,30 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new) if (!objcg) return; - /* Transfer the charge and the objcg ref */ - commit_charge(new, objcg); + new_objcg = get_migration_objcg(old, new); + + /* + * @old was charged through a non-root objcg, so its charge is in + * the page counters. If the re-derivation walked up to the root + * objcg - @old's entire ancestry is dying and already reparented + * - the final uncharge of @new will skip the page counters (see + * uncharge_folio()). Settle them now: this is @old's eventual + * uncharge, moved up to the point where its charge record ends. + */ + if (obj_cgroup_is_root(new_objcg) && !obj_cgroup_is_root(objcg)) { + rcu_read_lock(); + memcg_uncharge(obj_cgroup_memcg(objcg), folio_nr_pages(old)); + rcu_read_unlock(); + } + + commit_charge(new, new_objcg); /* Warning should never happen, so don't worry about refcount non-0 */ WARN_ON_ONCE(folio_unqueue_deferred_split(old)); old->memcg_data = 0; + + /* @new holds its own reference now, drop @old's */ + obj_cgroup_put(objcg); } DEFINE_STATIC_KEY_FALSE(memcg_sockets_enabled_key);