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 188E0C636CC for ; Mon, 13 Feb 2023 20:12:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 77EDE6B0072; Mon, 13 Feb 2023 15:12:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 72E746B0074; Mon, 13 Feb 2023 15:12:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5F5E46B0075; Mon, 13 Feb 2023 15:12:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 4BB106B0072 for ; Mon, 13 Feb 2023 15:12:22 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 193B8120508 for ; Mon, 13 Feb 2023 20:12:22 +0000 (UTC) X-FDA: 80463365724.13.F792302 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by imf10.hostedemail.com (Postfix) with ESMTP id 49799C001A for ; Mon, 13 Feb 2023 20:12:20 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=none; spf=pass (imf10.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1676319140; 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; bh=ZCSh0rOlnhq0jsvzmnls8jGEu2yOSFZpTDHY0cQzaGM=; b=Bpzxh10SUqTpFgvdFHuBMg8R6lJLaTX986hvVdHjYThbK0VQ0CaTGtiQb6f7OhZmQ4fhR7 fikpN3mYRZwkhWXdpK0t1o5BzzCujx89jNw/jmfwm3J+2VERBJNeCClgf3axFFxcB3YF3k XrMRhmwFz5qxHekqXuCp0f7lRTp/5rg= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=none; spf=pass (imf10.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1676319140; a=rsa-sha256; cv=none; b=FTwDD23t9zmNT1uhlhn2U0wihUmF9u/Z0klifruQzYk5zdZXB3tKOUYOIoZhBPgsW8KSTh Hq17zF9HuE4IaPZYcoQquXIWr0Y2xMg3mrp3p0+RaEsIMWTdn/9PLDEhcUAO0aEZBTXAmZ qzC+y9jp73H1LNwQAjusD6HBA3kVrek= Received: by mail-pl1-f173.google.com with SMTP id ja21so7140123plb.13 for ; Mon, 13 Feb 2023 12:12:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ZCSh0rOlnhq0jsvzmnls8jGEu2yOSFZpTDHY0cQzaGM=; b=VzfjbEJKaLdxX0wsgbzu2UtoCHdvBH4j8GMb6j02DucREhR+kL8GfO9Y/Ci/XsLEYN 2ZYyhxpRkLjfTFJGIDmF4qRSeF4wycbgr5Sb2SqrJQm/uLkAKWi+4CZKcrvv1t5GA65D DwPEXaicbNi8wEBaEcz+0iJncaPrwsosT1LmnGQ7KVsqGmDmARTEINSwwh+dFE2p9Qaa 6Kkde6HUh80OUk94/O3PppHIp6UTPxReuFT/O5C7+Omh09sEBxKcDdZL6VWu9mZe/4Rq 3sQX1EMdCh+MQPPfWv3S5dCXT78HYtBf/PNO6zJaL66JUErl+UDfj135/oLVvpH+0oMv OraQ== X-Gm-Message-State: AO0yUKXLMh+0nROvvKBjixMPVRHwXJsIL0d1RSM9+m9N1JOKC1I5mrOI KAsw2dxZHMWK1genBya4Bw0= X-Google-Smtp-Source: AK7set8W83xVwWpHHxrNovVxLQqzxOCaKgvCoW6UQXewBQ3SLyYcy9D0/2+NEx+ShZW3PbiTRkwbeA== X-Received: by 2002:a17:903:74b:b0:19a:95ab:6b2b with SMTP id kl11-20020a170903074b00b0019a95ab6b2bmr52851plb.69.1676319138998; Mon, 13 Feb 2023 12:12:18 -0800 (PST) Received: from snowbird (098-147-208-089.res.spectrum.com. [98.147.208.89]) by smtp.gmail.com with ESMTPSA id o7-20020a1709026b0700b00196519d8647sm8640782plk.4.2023.02.13.12.12.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Feb 2023 12:12:18 -0800 (PST) Date: Mon, 13 Feb 2023 12:12:15 -0800 From: Dennis Zhou To: Yafang Shao Cc: Andrew Morton , linux-mm@kvack.org, Tejun Heo , Christoph Lameter , Roman Gushchin , Vasily Averin Subject: Re: [PATCH -mm] mm: percpu: fix incorrect size in pcpu_obj_full_size() Message-ID: References: <20230210154947.4460-1-laoar.shao@gmail.com> <20230210140508.cfafb571dd44cf4fe776750b@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: 65njx4wfrkjkhorcb463kqxr4bd47qaj X-Rspam-User: X-Rspamd-Queue-Id: 49799C001A X-Rspamd-Server: rspam06 X-HE-Tag: 1676319140-942890 X-HE-Meta: U2FsdGVkX1/D9T3W1FCqhdV1EEMSSQIqOzpHhItsd5i2cEr76+MQNErrek83lZPhRPG4wJS2+2o9qezavIZa0UYBd3kDDYPZCLRy0bfHMFS1GjhhPr6rQneJzad+RiYGLb5Ooe+Qmy8fL+mn3Fu9+YLkhtDgGxHnUoXClax9zdAzFcJoTaNdpTI5EcakUuEDqUowHCSPPfvSbM7MPn3HnFu8nB9AEMOonhutL5QUSMwyIdyyIT5Cs8GLW8TFKHlaB5fFjFCExODiInG3shs0KcyL/Pe4pNw6s642+Mh6FF9tJh3smDMNz7JeQyzVwwQeDLSihDSHwvyVOKIsJfpv/3SCIG0fwXyXk3oGATqtisDfuHsAw1LAPB09AqG26MlBiIlqAUGx9HUrPIrIAnU+ghXQX50vIgT658NBiVaX79yVpbqzw8hiCtzSx2s2zKp9VgZikaOy4OSb26yeuIlKxOEDvznEpLp24SYrxuBBD6ybu32CwGwb7WNNP6oNwEZqhbpIjoM6jjv3cc42WDmeSAHSbhGgIC3iJy36q0kIPceEnLXkCdkqwLjfjdDQ0cS2rBqU4SvHC08qhIfDTYmiN7VHdsTfiiD14Lk/6V4xpxePtNfT1Ae5Tj1f9s+ViYL9MSRSoKxTQJMwy5iZgMS1fYMdvFpFzFu/SJNxefInF6bvLIOPLZlupSdoW9lEgvm0koIjjQ3estaYxvEyl3o8GoW5cEQC5rulS2iMjAGCr57VIMn9vrXoXP0ots9z43PK8TnpSvE37XK2o25tWTmmpWWhR/9gND1Sg65b1YxVwfehBiKbWJRcMZgp7BBaFOgAVtoi5DS3+I8F2IMVyzlLJSz8+aEeO+6bBmAWDnGg+GqnQUuvHxuD8P1u90+/idVwg1HiwugbPZZ50yQtL1eif/de47avho5i1lb7h5H4nj5bXUt6nwbqZf53R9QTJ+8W8hQrXvMWAarM5tG6Hk+ +M5j2cjY 1L8hVktJpgNvWe+Pq/knZKkVA5ZnaPzAgM1x7vlfbdG9feU0D0yBrTyakv+PxM0ASHUGUcO6yoZg8qmp5ZRWcaiI8BJrTd+t3YYcXFt+uM9L7fY+Ca+mu0Cap1IOa6dEbWjat2rXu1doAx8mH/sFMji6tw4UPx1clpmXsotT8ZTcfp3DbkNKcGjxj6Am6YnIhdnSIii5Bhz97tFGDLiWO4J1NRrvNDMucGzW7LRLEMWcRh1slOHjH+uT/md4A35M3X/ioOzV+1twgCM2U9TgxRvsqyc9T/BgLRXw5mq68HR/N1/5E9hcvOxJLU4N4RYZrlFyO69WWnYfdJmpgYJBsAOK+BqCSio1QxxT8GnJf0tn+vhfh1ehnfzmKblmyoU6hC5Kks3C9tAYZrXrP6zOQp2njjY8Xk1WFb9rVxBBXslvJH3M3nh+e9obLEF5n7Lee7TJLS1H0Led4F85RqnG/gdG23FllNFDEMoPzDT1yHiUZNmty/jzYcyQ90jQLZJHK0kyWzVIPimQptLVARgOy22PERvDddjzl9SoAPEJ7ddNiyUl9klHKtA7a3InRgr+Y2mKXTmFnl5QF/bk/ByR3O5yuNDfXf4L0ru/7zjoBLAkqEsCqWMuGZRKdZbJ1phbcIttiEw4J2gK3DgHWWobwkJoMHSjaTcycWlPCgYpbkAxPpyZXN1Vq9NZqYg== 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 Sun, Feb 12, 2023 at 10:12:12PM +0800, Yafang Shao wrote: > 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 ? > Hmmm I got ahead of myself. I misunderstood memcg_*_enabled() vs memcg_*_disabled(). Roman clarified it just now in [1]. I was imagining a world where we add disabled here and then eventually enabled would propagate here too. Anothing that was on my mind is, should a percpu object be charged for the memcg space even if it's not in use. I now think it's yes and then for general accounting outside of memcg, this function is correct. Acked-by: Dennis Zhou Andrew, I have nothing queued. Do you mind picking this up? [1] https://lore.kernel.org/linux-mm/20230213192922.1146370-1-roman.gushchin@linux.dev/T/#u Thanks, Dennis