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 251B8C7113C for ; Wed, 28 Aug 2024 22:11:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6FF0F6B007B; Wed, 28 Aug 2024 18:11:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6AEB06B0083; Wed, 28 Aug 2024 18:11:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 576046B0085; Wed, 28 Aug 2024 18:11:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 3B8536B007B for ; Wed, 28 Aug 2024 18:11:40 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 006F51A0156 for ; Wed, 28 Aug 2024 22:11:39 +0000 (UTC) X-FDA: 82503051918.27.E51F989 Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by imf04.hostedemail.com (Postfix) with ESMTP id 2DBBB40021 for ; Wed, 28 Aug 2024 22:11:36 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ZIYyoAM2; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724882998; 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=/cedSdGJF8fk6JLURCpMr14nIZjcEqUjClDXhK33nqo=; b=UmQYjKZbSq3jFFGgnlm8ZK1CICTeGFyxFoES0nULyDIBEEYppbkXRCnTGL6D/06e21E8sy Yu0FczzQaJ1kJ0MLWXYZFJRaEriFAVMIZzHQJz/gBeBuE6YKLMtRrBIrr3ZTC/Sojt482Y BqAtNBVjFPrlAGx28JAGka1flYSwxSk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724882998; a=rsa-sha256; cv=none; b=ykY7y4ttFgc9s27qC6Lrr9IdsXyYVk+ze0e1nV/N0OZSwybjUuLAsZfCe7cEHX/Ep2K+b5 KScd408z04Nih4lu1qXAPePeE7+usKWRVW6uTqyi05zVrwKVk+FtTqml0ZDsdrMqkNXZEj pVXu3yLLoDXbRubagLaUeDbBRjXLdSs= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ZIYyoAM2; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-533496017f8so9761630e87.0 for ; Wed, 28 Aug 2024 15:11:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1724883095; x=1725487895; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/cedSdGJF8fk6JLURCpMr14nIZjcEqUjClDXhK33nqo=; b=ZIYyoAM2bsN8BTzjgvlIiNyDClww5WrPj7EDv/Dcd5NiTEQKduKcusKg0p0dDrEH3N mxJwM1hbm5vpQHofBc0VCaxrA21ECphF3yJMc3CQe8HwGBRq6Kb90vE2QwcybRlYfp1n SW9dTkTCt0NRO7NfVIIgpNOXXLAGOndiE8L7xqrppulX4RdztRe+yTZN1i4aGNi8cMv8 FXRR0ArrHb5n75PL5kd5CtZDIQJyPUiimHUBcJ6V1ZmbiWmxfb44KzkZNmu0vBT60qiz mEdfMSyq2y6HATgBNY57wgkd1LiSmrKfX2A1xKVHZg0ctbYdqfB25VSerttQic2lpWhT zngg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724883095; x=1725487895; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/cedSdGJF8fk6JLURCpMr14nIZjcEqUjClDXhK33nqo=; b=S849XtWhCA4yWNBnYlriZ5hZp7ZbthZOcfblplsTgIyLSsXa/ZBCa+vobTIK55OoOF 9USvAsSk4Hi3Ul6iFUbqmt2QBI0rSz2zZ18C4ANkP8J/C3yZawa+29THjQ+Bx80zIYQo xpMK+AvunJMHpe2iIUjPGTFwM0vQt8p5f0hzJeWIQac4JPJ1fVjziSgDhy25RWpSKZhi 542XMIayhBXoi/n5rshqFUOVDf8RZHj0FKMXUT+fg37wwifNN/O51SgR5e/tiAei9ASI g2biLgquX0krk9gHdAwxf74/qRx/EeLBvtJas/LjTf6o5ntwzGvB65gvFPog6Eqkg68m 9M5A== X-Forwarded-Encrypted: i=1; AJvYcCUc/jKr3O0bhFVQLfqEd97OE1MihtQ+p8QDD4/SzX6NRUvUf8dXLXaPMgixwwfjdY+29K8n3jg4tA==@kvack.org X-Gm-Message-State: AOJu0Ywf/zsOEl68zEOPq2qBXYHC5iLjdr5DZDt0OK7G+w2F79Uc9v1X FaOOAP0vo3lY8lnQSsY2/pRXiZhCMrkj84wvYJDQsnSt9cwg12MdfFpt7YE1HphDDYU09KXzOVl IQ9IFxvN9CR7nPymo3oz7CzpNAZK244A7LOOo X-Google-Smtp-Source: AGHT+IE7FgxeEp3oK3h/EHwPYj0Q+GKF27lVD8LDpzLVsN055ZLe7P8ayw7s6FdJcYtn6UQg5NpMNs5TEutsWVNnHFI= X-Received: by 2002:a05:6512:3d07:b0:533:4191:fa47 with SMTP id 2adb3069b0e04-5353e5b1135mr381607e87.47.1724883094592; Wed, 28 Aug 2024 15:11:34 -0700 (PDT) MIME-Version: 1.0 References: <20240827235228.1591842-1-shakeel.butt@linux.dev> In-Reply-To: From: Yosry Ahmed Date: Wed, 28 Aug 2024 15:10:56 -0700 Message-ID: Subject: Re: [PATCH v2] memcg: add charging of already allocated slab objects To: Shakeel Butt Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Vlastimil Babka , David Rientjes , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Eric Dumazet , "David S . Miller" , Jakub Kicinski , Paolo Abeni , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Meta kernel team , cgroups@vger.kernel.org, netdev@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 2DBBB40021 X-Stat-Signature: tdw7x9cakh841q1cb1mydziccsurcqmo X-Rspam-User: X-HE-Tag: 1724883096-365196 X-HE-Meta: U2FsdGVkX1/ohiUpVR85uvqpLUKAwcbyRWkNobSqCEULp57P3trvzSoB23mDOBPx6J1t6xfEFYldOD4ftI7IqVVO57hCQ7eRpwfVhE4ueatfaoieYTMuaPcM1AosTIIaPSHdILz+NYp77X9r/gfOIYIfLPG/qR3N2xUvtQgqe2KboYs37+sPBppm2UkGddTiR9QKSsAYhu3IZzaNjeGrwAuV2sWdVo+llchsWEyoO6HbDDApf2/0uvfqGacFN4rVlsmMxHXrvbLSEMkO773f6tmDei0oGtLK1aF+u6F5nLG5xoN6gGqB0owzG5nQnpFqAM4gM8V4Sg05NS/++Ow8LTMCdyjejrqAwhwPIsik68K6kft3rBKDp2lNFl57QVZKTcY4f+MHuJbY9Ij51otBSjVvHp1xFkFapDy6Wk85EWvp2Fibo+v3P0YfoBUgfOcF75QpeRlImgbpfghzA64mR10ndmFIZh5/dr2R+ssnp//ME9opdqAnW+atLvhkdQI7/U+0yU204Y0RpSdRJrlErnB7VTxMl3QRFdkxWomX7rIgEjzmKZ6pP1yeFNDcKeX+jGHT11XNOESDqUIcVIgRjCdu1SmaCFtKpk9H+USjtUHdhTaL5jj3i5qImyZLxuohKzE9tqlxL0g2a7+nz5Z1gZY4z7RcdTSqNoEFXTgIzijtGhkjLdbHDvRl5PI2DtdzgWG2+UTVHD/KjTVZqMDHwMqMr8ThVL27LM8UiZxg4GvhxDBwmAM03uNVK6TfXuwVp7R932irOqRYqqSHCz9WCIDdQtegGbALOLPseQX1JtGjYeLYw1H9QseaYeU0+PPsBQy+JcMitoxTpbp7H9oTMkW82cO32fFcR2t6HkU3T/14U9yCbgpR1Mi1lNeYD+w5OuDRTX/Oneg4FHjb+5P3KfdGZju5434B9glxUZlXZkPH4nYOdr/+516J/0YxrD0CTKbY4j1EM3DBA6SWLGo lDAGf2Ps GAToZ1LFl1pFgTtZkJXzOtFLelFsnAor2eAysXjpC88udf7FtmBKYHIes2QgZMNDHacx08ZfCAdfaMEZ/RknVt70Orsumxr35gZhPtlMsybh0nNee2zzcOQLahB4LfYWlzo/huqf0R3y70Xqr4Z03Ta5GRAMTqqRh5DxlkbFC9mndQu1ePY03U34+I0W0aiyeqTHly6MHufn48d6gf8V/j4O+d0NWMBmBw3sQ86UHPuSK50LKQrTbMJhbaKjhdLUH4JPW0M6zVNSMs78= 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 Wed, Aug 28, 2024 at 1:16=E2=80=AFPM Shakeel Butt wrote: > > On Wed, Aug 28, 2024 at 12:42:02PM GMT, Yosry Ahmed wrote: > > On Wed, Aug 28, 2024 at 12:14=E2=80=AFPM Shakeel Butt wrote: > > > > > > On Tue, Aug 27, 2024 at 05:34:24PM GMT, Yosry Ahmed wrote: > > > > On Tue, Aug 27, 2024 at 4:52=E2=80=AFPM Shakeel Butt wrote: > > > [...] > > > > > + > > > > > +#define KMALLOC_TYPE (SLAB_KMALLOC | SLAB_CACHE_DMA | \ > > > > > + SLAB_ACCOUNT | SLAB_RECLAIM_ACCOUNT) > > > > > + > > > > > +static __fastpath_inline > > > > > +bool memcg_slab_post_charge(void *p, gfp_t flags) > > > > > +{ > > > > > + struct slabobj_ext *slab_exts; > > > > > + struct kmem_cache *s; > > > > > + struct folio *folio; > > > > > + struct slab *slab; > > > > > + unsigned long off; > > > > > + > > > > > + folio =3D virt_to_folio(p); > > > > > + if (!folio_test_slab(folio)) { > > > > > + return __memcg_kmem_charge_page(folio_page(folio,= 0), flags, > > > > > + folio_order(folio= )) =3D=3D 0; > > > > > > > > Will this charge the folio again if it was already charged? It seem= s > > > > like we avoid this for already charged slab objects below but not > > > > here. > > > > > > > > > > Thanks for catchig this. It's an easy fix and will do in v3. > > > > > > > > + } > > > > > + > > > > > + slab =3D folio_slab(folio); > > > > > + s =3D slab->slab_cache; > > > > > + > > > > > + /* Ignore KMALLOC_NORMAL cache to avoid circular dependen= cy. */ > > > > > + if ((s->flags & KMALLOC_TYPE) =3D=3D SLAB_KMALLOC) > > > > > + return true; > > > > > > > > Would it be clearer to check if the slab cache is one of > > > > kmalloc_caches[KMALLOC_NORMAL]? This should be doable by comparing = the > > > > address of the slab cache with the addresses of > > > > kmalloc_cache[KMALLOC_NORMAL] (perhaps in a helper). I need to refe= r > > > > to your reply to Roman to understand why this works. > > > > > > > > > > Do you mean looping over kmalloc_caches[KMALLOC_NORMAL] and comparing > > > the given slab cache address? Nah man why do long loop of pointer > > > comparisons when we can simply check the flag of the given kmem cache= . > > > Also this array will increase with the recent proposed random kmalloc > > > caches. > > > > Oh I thought kmalloc_caches[KMALLOC_NORMAL] is an array of the actual > > struct kmem_cache objects, so I thought we can just check if: > > s >=3D kmalloc_caches[KMALLOC_NORMAL][0] && > > s >=3D kmalloc_caches[KMALLOC_NORMAL][LAST_INDEX] > > > > I just realized it's an array of pointers, so we would need to loop > > and compare them. > > > > I still find the flags comparisons unclear and not very future-proof > > tbh. I think we can just store the type in struct kmem_cache? I think > > there are multiple holes there. > > Do you mean adding a new SLAB_KMALLOC_NORMAL? I will wait for SLAB > maintainers for their opinion on that. BTW this kind of checks are in > the kernel particularly for gfp flags. I meant maybe in new_kmalloc_cache() pass in 'type' to create_kmalloc_cache() and store it in struct kmem_cache (we'd want a KMALLOC_NONE a similar for non-kmalloc caches). Having a new flag like SLAB_KMALLOC_NORMAL would also work. Or maybe using the flags to deduce the kmalloc cache type is fine, but in this case I think a well-documented helper that takes in a kmem_cache and restores a type based on the combination of flags would be better. I just think in this case it's easy for the flags to change from under us here, and the code is not very clear. Hopefully the slab maintainers will tell us what they think here, my concerns could very possibly be unfounded.