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 B5F55C636D3 for ; Sun, 12 Feb 2023 14:12:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4D1FD6B0073; Sun, 12 Feb 2023 09:12:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 481FB6B0074; Sun, 12 Feb 2023 09:12:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 322EF6B0075; Sun, 12 Feb 2023 09:12:51 -0500 (EST) 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 1DF956B0073 for ; Sun, 12 Feb 2023 09:12:51 -0500 (EST) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id CEF52140ABC for ; Sun, 12 Feb 2023 14:12:50 +0000 (UTC) X-FDA: 80458830900.09.062B93D Received: from mail-qv1-f52.google.com (mail-qv1-f52.google.com [209.85.219.52]) by imf23.hostedemail.com (Postfix) with ESMTP id 27699140020 for ; Sun, 12 Feb 2023 14:12:48 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=O7qIQXiG; spf=pass (imf23.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.219.52 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1676211169; a=rsa-sha256; cv=none; b=1Py75NQWC3XD6AgmUFS1DjTeyW4j5HHMtLMTUvpDlA2moTFzg1TtqpOLyajn8AC2TeOiap KzZPDqQGqYaz67Pg/HA+lf5gMOBjHB/hHDYhTKt+kZFAqdzK4REFBPG2rYGBzfslCO3HrT u2uzFwZYstvuxmd5n/P4h+1WyrlNVyM= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=O7qIQXiG; spf=pass (imf23.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.219.52 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1676211169; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=VcCkWWdWQAi8LZE7cXwPDFJp+47BPPY+o9nlIRD8SBw=; b=ZQE9Oq5jqU4JlQcrGSxCgJqsz6+A6FfCKdhsVGtdlJnDYjZTJfAaWg3MvhPJz7Z+399UkG 1ub57Tbha6QMyomGKiPCCSMBOwwxuDZr/owWOfs/Sw3oRi0JCmwkYxZOLXzOXjj4/E20L/ 2QZiFUJibE8MArAmZZ0N9nyEBJ/aKV4= Received: by mail-qv1-f52.google.com with SMTP id nd22so4268645qvb.1 for ; Sun, 12 Feb 2023 06:12:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=VcCkWWdWQAi8LZE7cXwPDFJp+47BPPY+o9nlIRD8SBw=; b=O7qIQXiGxnezLAcc+s+WQD2jD1yhc30PJEpooPSjj0+Iv6zD+eJ+1YoGmhPXfu+VaM TW9ROhDErXya1oqpBI7JuT4Pr9VZdf1qPA/wnwN9JPA40DojQXEQ11wjejcupD6fdH0k SQSXHiOmHlaXu0miojPVepKdxkLTeO8FHZRyjpjNcbSnDs2aUJYHCeNKWe8AkS/KtxYC 19sOEu2JEc0mSQM6GtQs8mo00eT6L0VZQowVPL91LpZ9E3xZEmN5oCWWnDv3eV5Dl1oJ IbEVWixH7QC3c5icQ0w6v2GHRnbcAfVvcs5n4OZQvCUcmAoTBluWAfA3+WO+0YsPPu/u DyUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=VcCkWWdWQAi8LZE7cXwPDFJp+47BPPY+o9nlIRD8SBw=; b=1w2lBBVf60NZlY8oGyq20LputzU+MYU/LgN+tC683PI/3tIs/RiLcClsxBXb506ElB D2WPaw4GLdnsDr3iDGeJXfYBtY4jwMc7FpKfo3tF/tR0Ekl083smW9imgywfnxvVRsxE zsZmp5DtYUCoMi6Ir8zu5V6DI7P4zaSshH0oTAfpGXcFHQB0WBV7QXyHqMzG3yu2vZE9 Oo7wQ0oWaR9qFjDckY2bJIDBS5DUwjI2NiVqdiHq+yyqqkxqY0aHKXZDy7v29Uq/J/Gy f0W+g4PTnRi6R1HLap2/Uracl6izqMdpynTt+zkESFYEurk7sIaNClQH2O3K8yI2UDmG nM9A== X-Gm-Message-State: AO0yUKV855GSIDHcnWDOBDQisqBToMCtyzmn/EnUUjxn4LGUyO3WTK3R Sf5pXs2W/3GZzz5Bwynn82vjpkgnC0/MLzCqfrU= X-Google-Smtp-Source: AK7set9CJM56ZU+VRJIH2wiuvUN/gmVa3wRQ6zgsJuE5UW/LEvPxXaysZfFfUsD1OEDlKunWasNf5Eycrf+0CVJd388= X-Received: by 2002:a0c:f448:0:b0:56e:9f28:c667 with SMTP id h8-20020a0cf448000000b0056e9f28c667mr305661qvm.26.1676211168242; Sun, 12 Feb 2023 06:12:48 -0800 (PST) MIME-Version: 1.0 References: <20230210154947.4460-1-laoar.shao@gmail.com> <20230210140508.cfafb571dd44cf4fe776750b@linux-foundation.org> In-Reply-To: From: Yafang Shao Date: Sun, 12 Feb 2023 22:12:12 +0800 Message-ID: Subject: Re: [PATCH -mm] mm: percpu: fix incorrect size in pcpu_obj_full_size() To: Dennis Zhou Cc: Andrew Morton , linux-mm@kvack.org, Tejun Heo , Christoph Lameter , Roman Gushchin , Vasily Averin Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Queue-Id: 27699140020 X-Rspamd-Server: rspam01 X-Stat-Signature: 9fim6n9w6kzt6f8wax76okdpapoh6zeo X-HE-Tag: 1676211168-447899 X-HE-Meta: U2FsdGVkX18yxEOHMYx1+HMHpqgZGQ+BmhPeLRCePsOizqwJmdL0V9R4ySDRhXOWn62LEYA5JMVYGoK88h40HpuV/FQFIQ8Q+g0zB/tkhEVsZTnLoNZieJUSJtAvwEDmR5HjAbjz5p1VwgU3kx/RWvuaEOkxVd0h9iUvj+QO/ztCCa0KK8IKfUi2JbQml7lBM/LddmBQvH+vEgFQjWZC/Zm2fHwUEh0YTXlUcs7cSirMnz7bDAAiEnew7h1qzyG4QaAicg44klZbmIBJqjyMgPeXAtpX37JSutPIZ1U9SH+i/1QMeLSeKCBRwnF/nn73/Q4+mVArjoNrCOsPj51R1SUcuA2KxUY05MkKDPrAapT/Cj/m7lscN11xTc4t7Bo0DdatveFsCVpkB2BiggHOW5U+t6WoJyp3m84l+GsiafXHRBjLyd+gWBmVur72JdEvMz24nvyqRGwH/hYUVroBeKAGbZCWOCp/3WsUhw1kyxnbahJ284riafVEGYUp3nJz84lwB+wrBYSXySiHlUYb2QrVbJ2+FJjIbflZIohNzs1iXAKGc33yFLsoQ/KJIXtYr3Q8Ti1fsSDH70opvJhP1nVyNIKoI/FI4LMAcaJn/7pgAFi9G4siuqWIy2XhX4OSrq5PBBco7IbxQFH3NnEqVToZKugldqEZSxnHeMkxUeLkCED2dw9jNLtoDWbeGNujJDvKAjkVLSaYlFiBNK8yx6F4dBq78CaOFH9dWTPZwQxHT9xVcZrrfjgYAHWKCLYkWfv0WhTt83je+RaE5zD9FB9tYQh1HHadqPaJBZb9mKqb1A6fmGR5u0zL0JFY0YVWmCoPFyXkPgiQMhvej7IoUQxUeMyLIbb7o1zUbwbL7emI/qP8U6CTE2oFfCmIciiAoDz0p7yLGFfv4u6rc7PIkuWrN7lq4u3Hz5DTBusuJM7UrR6jkF99MPkr/rAXJwu4XUgTtiosjtwt6G4ke4m 5P9QhGrP hJNZ9hoWafY0pSqKyhBHRuWxow0cNXw501tY3NtXmAwnyM2pi5qsgZ/m2f4itr50n3p+wI1QgwoXzyskNTbYa3erBI/86GG+GqpclEs0hwnviES/UbFehi9nKAZ1GoZRcj970pHi1SFRVQhCKpPLUYB178S66ObRSmUrkfUrD3v5vau29FYr88SPAoPZ8nacGAvTeo2bgJ0n4mHuX0yWoh448mg6ni9DyU87+qDhT5aZSSjME+cJfg9aSuRVGqIDiFONPQ6w+Uk+0CbfBgGFKKY4nJuCxvVv4wpfAS/gFVfp9A1QWG8H1um0WK3DMx5R7xO9p2oV/05kuFnAizvg61ZDx7SEz32emSQC6UreGjWJvHS3XjgbHP24SAujgDt/IkQoNf268SRyiTqcs4OBoLxJOkK9TY3Tb6bcKtLyyQoqXmpunASnkVcL+afGry2845LNbhl1RQMO22DsEwqfvPb6YsDCzKpLLbYJ8nfG5APQs0SnHfPYm9SzYRa0yeeq3ZUNuB4t+GTxQHU+vANkRf9S5gw== 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: On Sat, Feb 11, 2023 at 6:39 AM Dennis Zhou wrote: > > Hello, > > On Fri, Feb 10, 2023 at 02:05:08PM -0800, Andrew Morton wrote: > > On Fri, 10 Feb 2023 15:49:47 +0000 Yafang Shao wrote: > > > > > The extra space which is used to store the obj_cgroup membership is only > > > valid when kmemcg is enabled. The kmemcg can be disabled via the kernel > > > parameter "cgroup.memory=nokmem" at runtime. > > > This helper is also used in non-memcg code, for example the tracepoint, > > > so we should fix it. > > > > > > It was found by code review when I was implementing bpf memory usage[1]. > > > No real issue happens in production environment. > > > > > > ... > > > > > > --- a/mm/percpu-internal.h > > > +++ b/mm/percpu-internal.h > > > @@ -4,6 +4,7 @@ > > > > > > #include > > > #include > > > +#include > > > > > > /* > > > * pcpu_block_md is the metadata block struct. > > > @@ -125,7 +126,8 @@ static inline size_t pcpu_obj_full_size(size_t size) > > > size_t extra_size = 0; > > > > > > #ifdef CONFIG_MEMCG_KMEM > > > - extra_size += size / PCPU_MIN_ALLOC_SIZE * sizeof(struct obj_cgroup *); > > > + if (!mem_cgroup_kmem_disabled()) > > > + extra_size += size / PCPU_MIN_ALLOC_SIZE * sizeof(struct obj_cgroup *); > > > #endif > > > > > > return size * num_possible_cpus() + extra_size; > > > > Sorry I've been a bit mia... > > > Seems risky at the first look - enabling kmemcg at runtime will make > > prior calculations based on pcpu_obj_full_size) incorrect. But as long > > as this is only used for accounting I guess that's OK. > > > > What happens if we do a bunch of allocations with kmemcg enabled, then > > disable kmemcg then free those allocations, or some such thing. Does > > the accounting end up wrong? > > > > For now it works correctly because of 2 things. 1 - the function is only > called by accounting. 2 - the free path doesn't consult > mem_cgroup_kmem_disabled() but consults if a memcg exists for a percpu > allocation. If accounting is enabled, we'd always account the additional > memory for the memcg accounting. If it's not enabled, then percpu is > well unaccounted for. > > This function probably needs to be renamed a bit more carefully so it > doesn't bleed outside of mm/percpu.c. > Do you have any suggestions on the new name ? > In short, I don't think this change is correct. Could you pls be more specific ? > > > The final sentence in the pcpu_obj_full_size() kerneldoc could do with > > an update - it still implies that the extra_size accounting is > > unconditional. > > > > Thanks, > Dennis -- Regards Yafang