From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-170.mta0.migadu.com (out-170.mta0.migadu.com [91.218.175.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3DF721798C for ; Thu, 29 Aug 2024 02:36:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724899009; cv=none; b=VjOwT0A4a5QB/tefLaXDf5mH1CjAHI6rYZhYYSyLKD/tEbC8tAtpPPzlS6/7ZjfJJykWcIQWquv7qLQ1B9VwkA9DVq7aIAI1ADv0bgw+//LxHhrXCjrGQr88ucK/TY2av3+I8nWjn6j17w68p+yR2CDZGxT/5PAUU+eaaVM32yo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724899009; c=relaxed/simple; bh=bGlQAlKtPj4qnjGq3CAst/qDUBkSaSOOIVGCqBBJ/L4=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=YHTsMZYHiuWuPyqpl2fpepz8FeWBlDURQENUkqu4DZmMNueH7WtPJn5xOcrAoqYd1oTYulNc3a/z4aogFMLClE2Ta4a5HL6DDKl6/ZqpjFbbz7CWASCmOoBeLoueBjStV82VE6xDj4NuG4W9OSdF12DQ7BiY4s6jZJVsuwPMCZY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=vsqOzIQO; arc=none smtp.client-ip=91.218.175.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="vsqOzIQO" Content-Type: text/plain; charset=us-ascii DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1724899005; 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=bGlQAlKtPj4qnjGq3CAst/qDUBkSaSOOIVGCqBBJ/L4=; b=vsqOzIQOPmUD/dI//siE4+IIQngQgXxQBpdpB7E/BzqliMg1RHeAWk5x8XpoV96GvDofCS Y29CW/+TU573wsCN6urLZsB1zRwanhC84C4aE2tiG1Z/svyHG5ZrK7Oyf0Kkzr2vEMAfdm r6jlMDdK5S140QjzXQ26lhat3rHG19o= Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3776.700.51\)) Subject: Re: [PATCH v1] memcg: add charging of already allocated slab objects X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song In-Reply-To: Date: Thu, 29 Aug 2024 10:36:01 +0800 Cc: Roman Gushchin , Andrew Morton , Johannes Weiner , Michal Hocko , Vlastimil Babka , David Rientjes , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Eric Dumazet , "David S . Miller" , Jakub Kicinski , Paolo Abeni , Linux Memory Management List , LKML , Meta kernel team , cgroups@vger.kernel.org, netdev Content-Transfer-Encoding: quoted-printable Message-Id: <97F404E9-C3C2-4BD2-9539-C40237E71B2B@linux.dev> References: <20240826232908.4076417-1-shakeel.butt@linux.dev> To: Shakeel Butt X-Migadu-Flow: FLOW_OUT > On Aug 29, 2024, at 03:03, Shakeel Butt = wrote: >=20 > Hi Muchun, >=20 > On Wed, Aug 28, 2024 at 10:36:06AM GMT, Muchun Song wrote: >>=20 >>=20 >>> On Aug 28, 2024, at 01:23, Shakeel Butt = wrote: >>>=20 > [...] >>>>=20 >>>> Does it handle the case of a too-big-to-be-a-slab-object = allocation? >>>> I think it's better to handle it properly. Also, why return false = here? >>>>=20 >>>=20 >>> Yes I will fix the too-big-to-be-a-slab-object allocations. I = presume I >>> should just follow the kfree() hanlding on !folio_test_slab() i.e. = that >>> the given object is the large or too-big-to-be-a-slab-object. >>=20 >> Hi Shakeel, >>=20 >> If we decide to do this, I suppose you will use = memcg_kmem_charge_page >> to charge big-object. To be consistent, I suggest renaming = kmem_cache_charge >> to memcg_kmem_charge to handle both slab object and big-object. And I = saw >> all the functions related to object charging is moved to memcontrol.c = (e.g. >> __memcg_slab_post_alloc_hook), so maybe we should also do this for >> memcg_kmem_charge? >>=20 >=20 > If I understand you correctly, you are suggesting to handle the = general > kmem charging and slab's large kmalloc (size > KMALLOC_MAX_CACHE_SIZE) > together with memcg_kmem_charge(). However that is not possible due to > slab path updating NR_SLAB_UNRECLAIMABLE_B stats while no updates for > this stat in the general kmem charging path (__memcg_kmem_charge_page = in > page allocation code path). >=20 > Also this general kmem charging path is used by many other users like > vmalloc, kernel stack and thus we can not just plainly stuck updates = to > NR_SLAB_UNRECLAIMABLE_B in that path. Sorry, maybe I am not clear . To make sure we are on the same page, let me clarify my thought. In your v2, I thought if we can rename kmem_cache_charge() to memcg_kmem_charge() since kmem_cache_charge() already has handled both big-slab-object (size > KMALLOC_MAX_CACHE_SIZE) and small-slab-object cases. You know, we have a function of memcg_kmem_charge_page() which could be used for charging = big-slab-object but not small-slab-object. So I thought maybe memcg_kmem_charge() is a good name for it to handle both cases. And if we do this, how about = moving this new function to memcontrol.c since all memcg charging functions are moved to memcontrol.c instead of slub.c. Muchun, Thanks. >=20 > Thanks for taking a look. > Shakeel