From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f46.google.com (mail-oa1-f46.google.com [209.85.160.46]) (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 25D6C3D9DD1 for ; Wed, 24 Jun 2026 15:23:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782314616; cv=none; b=Y43HJ7796dzv7tHRfvr9mUp/94EDbEDRaoimgw23QMCpJ0KmHCQonwXxgs8orR2I6pcForuawm54RzVpLEeAx70DxwS/9q6Y+rKA99flSOWx/F8o1DRe11gvIySsTDNj3PNGwZdl7JL8fdu8L9VHPU0f5h4cSgYBrJ3uUKJy97E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782314616; c=relaxed/simple; bh=TDpKuBop+fiDvr98096WtJv0rwGE5xpN35yQ6puN03M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=mZVuk+uhh+V64i9Pj5OsIyX3vLxR4Z12xJN/hkI5ZR7VHpPX5oVTQDYY6iOAxg8+GFKtLClqSE/+zqfekqhsaYwjSxtE0+d8tgY3emfiFiqhb4KmkUXsNfJW+B5dR/VCvPJmNfKigqQw82BVpN/7MBMUjb7fnlVhRr2QUX9wnN0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=DCIjiiPi; arc=none smtp.client-ip=209.85.160.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DCIjiiPi" Received: by mail-oa1-f46.google.com with SMTP id 586e51a60fabf-446f87b6de1so537815fac.3 for ; Wed, 24 Jun 2026 08:23:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782314614; x=1782919414; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=WU7DP8gS2bWrp9HjVS61UBsZ2iorph10vTjTK+XSa98=; b=DCIjiiPiiq8I9VLSjmy0J/lauUNSR+7BCyM3dEgtX1WGdKOVxqa6kIahC1c1XB5FnY 5ACz3vqt+gReplGlKbQq4pQ1beUeYxzPg4pZg1j1edXUF6BUceqwW+jMMGmSwIdecQDe /F1Pc7XWOlTQonoDRAbSbO71T7yo7ECV3eIVGBWGyVAhIXEPtKaXmWH1Ny5sIshrF1d7 LY58CRhYuu0ZHMk7A0UFrzT3gOY9PyApyFRitRMoCvTYsHN0xoDUX0ZIPrZ6aaQn0uO0 d6STZ6Xc/qgjooyw+Z9rVFhE15aVKypqsn+OL4/GnZQGaUwcIkO5dsHo/alptWOukqHD ckqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782314614; x=1782919414; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=WU7DP8gS2bWrp9HjVS61UBsZ2iorph10vTjTK+XSa98=; b=h5F+ndI8kE+Qu8I47wtKk8z8KPHi78K8da+/2UoCw3C5ImWJXgsQQ4BKETUfhJTY3i bGFI/1ApT8U7PuJEoB9rkwRe58ZqFAyPToNeVUVr6Z0o5yP9/hlhAR/rrsDMhwZsCtoY lttX14oQPGz9JOkA4w5hi+iNRfQVNDEpYxNiocmSCqySNQXGMGp0CmQaCAt+gWFLTKCJ JK/uWWygOcpxbnnHVfoHJzbUEgqMmRsou+MS6W6oE6KJ62N9qrMw2CyLU3rhsre58Kg5 QmVb0tp5L5vJZ/Tqx2+TdgQSJgeVa1zY7HQKCfHP3hU4BGiESMf725TbmJ7IB1S0cmWP GGIQ== X-Forwarded-Encrypted: i=1; AFNElJ+ehk3cySd6jBnw+LW0rj0BS2QkGXSJ736xPLI1P+nWWflg23E+m/DyTpERcISX5L5TlFNYpYO96ZJ+MbQ=@vger.kernel.org X-Gm-Message-State: AOJu0Yz6Tofaacipo/xyleC2y8/rnDkU6GKxrCqrHAeOI8LIFzmZ3Gbf +qPAbzm+TiNdzSIpYwPipnGTkqsd9SC26HnCGtDo08JJVRob1XPhBJ68 X-Gm-Gg: AfdE7ckJm1/CPoyV7D5bYDto7lF05JUa1ElAE9hEGlrCCdWloKUU5o9rIA7uoS5fXa+ 3uh3z4eIB1qNzs+kq3RwuY9s0heH25HYreFBkOJKaafoX32JgrFfiGllG86eY77i7tSPb7pACuc xlN+csakQlRM50Gcc6Y+xK2ElTz/RkNogobmbRBbZRuDD5eAv0dw0J+HXY83PGFDE82jXMD6GLQ vVMPDQVwNQmUpvUnDpHp6m8AXs6Q03QLdnYsrALfg0IIaI2peZslab7ZW2N4dUwqf/BXLg2dPAI fyu5+OtXDhrzqU3nBMpiu3i59TD3fxA8138/TZ7Nibg+3nQvzxChuGtEr4bmCWqSAOZ/EWsT/fc npm/n3ldgCJpb6UNRM8+TZNd15wr/4b75e/LdhdMH+qiipLv5CzE8Dr5rgwa9RiPGPsvdVDKeyH p4sRPNTdCvdZFHrVVxlmJYFV/aL79D8n9zroq34d1B6A8= X-Received: by 2002:a05:6808:c185:b0:464:5f3:ed1 with SMTP id 5614622812f47-48f29793dc2mr5618299b6e.26.1782314613671; Wed, 24 Jun 2026 08:23:33 -0700 (PDT) Received: from localhost ([2a03:2880:10ff:41::]) by smtp.gmail.com with ESMTPSA id 5614622812f47-48aebb9f1d7sm9220420b6e.4.2026.06.24.08.23.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Jun 2026 08:23:33 -0700 (PDT) From: Joshua Hahn To: Usama Arif Cc: Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , David Hildenbrand , Lorenzo Stoakes , "Liam R . Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH v4 4/5] mm/memcontrol: convert memcg to use page_counter_stock Date: Wed, 24 Jun 2026 08:23:29 -0700 Message-ID: <20260624152331.2228828-1-joshua.hahnjy@gmail.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260624144348.4117578-1-usama.arif@linux.dev> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Wed, 24 Jun 2026 07:43:47 -0700 Usama Arif wrote: > On Tue, 23 Jun 2026 11:01:22 -0700 Joshua Hahn wrote: Hello Usama!! Thank you for reviewing the patch : -) [...snip...] > > @@ -2595,7 +2596,6 @@ void __mem_cgroup_handle_over_high(gfp_t gfp_mask) > > static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask, > > unsigned int nr_pages) > > { > > - unsigned int batch = max(MEMCG_CHARGE_BATCH, nr_pages); > > int nr_retries = MAX_RECLAIM_RETRIES; > > struct mem_cgroup *mem_over_limit; > > struct page_counter *counter; > > @@ -2606,36 +2606,30 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask, > > bool raised_max_event = false; > > unsigned long pflags; > > bool allow_spinning = gfpflags_allow_spinning(gfp_mask); > > + unsigned long nr_charged = 0; > > > > retry: > > - if (consume_stock(memcg, nr_pages)) > > - return 0; > > - > > - if (!allow_spinning) > > - /* Avoid the refill and flush of the older stock */ > > - batch = nr_pages; > > - > > reclaim_options = MEMCG_RECLAIM_MAY_SWAP; > > if (do_memsw_account() && > > - !page_counter_try_charge(&memcg->memsw, batch, &counter)) { > > + !page_counter_try_charge_stock(&memcg->memsw, nr_pages, > > + &counter, NULL)) { > > mem_over_limit = mem_cgroup_from_counter(counter, memsw); > > reclaim_options &= ~MEMCG_RECLAIM_MAY_SWAP; > > goto reclaim; > > } > > > > - if (page_counter_try_charge(&memcg->memory, batch, &counter)) > > - goto done_restock; > > + if (page_counter_try_charge_stock(&memcg->memory, nr_pages, > > + &counter, &nr_charged)) { > > + if (!nr_charged) > > + return 0; > > + goto handle_high; > > + } > > > > if (do_memsw_account()) > > - page_counter_uncharge(&memcg->memsw, batch); > > + page_counter_uncharge(&memcg->memsw, nr_pages); > > This needs a transactional rollback. page_counter_try_charge_stock() can > succeed by consuming memsw stock and charging 0 new pages, but the > memory-failure path unconditionally uncharges nr_pages from memsw. > That turns a failed allocation into a real memsw usage decrement. Hmmmmmmmmmm....... I'm not sure. At this point in the code, we are either (1) using cgroup v1 with memsw and charged successfully, or (2) not using cgroup v1 with memsw. So I'm not sure if this really is unconditional, we're just distinguishing between cases (1) and (2) by checking if we're using cgroupv1. Or is your concern with taking a charge via stock, but uncharging with a hierarchical page_counter walk? If so, I think there's a case to be made here with just simply returning the stock. I just wanted to keep it consistent with the original memcontrol code, which only used stock to fulfill charges, not uncharges, since this could make the stock grow without bound. What do you think? Thanks again for reviewing Usama, I hope you have a great day!!! Joshua