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 1A090C04FF8 for ; Fri, 19 Apr 2024 02:56:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 76A4A6B0085; Thu, 18 Apr 2024 22:56:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 71A406B0087; Thu, 18 Apr 2024 22:56:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 608DB6B0088; Thu, 18 Apr 2024 22:56:06 -0400 (EDT) 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 3F9CB6B0085 for ; Thu, 18 Apr 2024 22:56:06 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id D0CA814019D for ; Fri, 19 Apr 2024 02:56:05 +0000 (UTC) X-FDA: 82024767090.17.3B78E84 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by imf25.hostedemail.com (Postfix) with ESMTP id 51A71A0003 for ; Fri, 19 Apr 2024 02:56:01 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf25.hostedemail.com: domain of zhangpeng362@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=zhangpeng362@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713495363; 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; bh=Cs2ngKBUMj/UFkpqHyZSccDREtz2Ctc6Nsmws9WDCCE=; b=z2wO8KMi8onhW/dhiATSh4NlaJ0uzbUhIIzgS8gL6ySP4LJrleSalQyHoeuaLJG9C7JjUk HzOvqRWew1+e7YQQmSRx0BOr22svKdsIn0AMoE6Yj1zFVAtLXxzyBYjS8RxmkhSPqcgO2F eMKQRCvJpKrhwUMxk8q4wXG1MnCFqpg= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf25.hostedemail.com: domain of zhangpeng362@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=zhangpeng362@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713495363; a=rsa-sha256; cv=none; b=Pr8q5iTOTdhy+ZF81qRIv4oUJnkecnOMyzGClBCTGUeGcMYJ5zXU6Yb8gJL6wgewjoWuer ygRoAftaqP4obtpsU7Sn8sUiH/hHxXp8sHC2KC55igC8AfJDMXDIvVVcCan9KRJlKigs2H eGDBrrJWBiHUO4sJ7on2LKL+TA1TA3I= Received: from mail.maildlp.com (unknown [172.19.88.194]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4VLK2R5wQDz1N7rs; Fri, 19 Apr 2024 10:53:03 +0800 (CST) Received: from kwepemm600020.china.huawei.com (unknown [7.193.23.147]) by mail.maildlp.com (Postfix) with ESMTPS id 61C631400D5; Fri, 19 Apr 2024 10:55:58 +0800 (CST) Received: from [10.174.179.160] (10.174.179.160) by kwepemm600020.china.huawei.com (7.193.23.147) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Fri, 19 Apr 2024 10:55:57 +0800 Message-ID: Date: Fri, 19 Apr 2024 10:55:56 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [RFC PATCH v2 1/2] percpu_counter: introduce atomic mode for percpu_counter Content-Language: en-US To: Andrew Morton CC: , , , , , , , , , , , , References: <20240418142008.2775308-1-zhangpeng362@huawei.com> <20240418142008.2775308-2-zhangpeng362@huawei.com> <20240418124000.ce4606dad982d7f31fc0d117@linux-foundation.org> From: "zhangpeng (AS)" In-Reply-To: <20240418124000.ce4606dad982d7f31fc0d117@linux-foundation.org> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.179.160] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemm600020.china.huawei.com (7.193.23.147) X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 51A71A0003 X-Rspam-User: X-Stat-Signature: 9zrawu1bgnoss31ssco8845ixwr5hwu3 X-HE-Tag: 1713495361-729695 X-HE-Meta: U2FsdGVkX1+Rbt5YA2kZRChggyEVgD5geHNeFjQ+NrN/8KiF7zwYNK+icMTE80yztjlCpp3LYMjQncJqylN/nuhdFBR0j+eUArUBYP/7Bccn9d+QVi67ZXi98hwlFCYgYhDRftZSXKKcKQhizbEhBTAYXli5uPDYkRuaudQ1Zh0kM5C2HsOXCehfEB2wv2L4DyoJItMQnfNvn0wLl40qhGXiCGWPsy+eh4JkcDmrBMPGv1OE9tQ8WMcyGiDEEA282D07TaF+ijhJz2C/NoQ6jr75pvTeJS/r1q0WcFIeRh8144Xmz+5DhD8w38fLb/LOuFveIOWkTd6TcfPP6+/IQMSftDw4Br7/stJGRrk3GlEFeGG+OOnCZxnXAJsmW3xLSOQt3qm/IxX2Kq/1W1vWy0OuSIVcVO1Egc4xgMxJIyqubWSvZdDD/ljQsg6andFmJ/t9ooxBMCLYCGiYE6ekfZHwvEhdrkEaD1f10PgRtojN6ESuGZAON4Lmuin6A5UMXA3sS88z4EgOn23seexAN5BVygUXissriz4GhnbeMCGzOpBzSY6mC9ySc4APva6ZpgoK61p7EfGqhgXPzLnvfqV3FlZ6FaodJRIUHg2heE1VEB+a8dJI59GuJvBHRsA0ZVSjbAz1L/nP7y8nRRDZv89aC6dC1f5wGp5GkZZzwDgAgMcvPyz2RBCZRzYzvdrFHSNeb6AfwNBCWkQRjO0RB8I9bEJfRMdfU4HFuLCyfqUN9BQ69OEBg/pOBTkkMFgCs7kFynsv3nBd5bJ3j4tuZqXftoxVFaRAV5wAuc/0Qh6/G20WvE67NTAVuwc9thdlbt6sdUhUNpWKDOFMqlxRQOf+q28Mttc0uEf3ONjh3b7SyKOmjhUv5czNOMoF4A6ZkpcbkRZ0bhS6HhvOUQYEOIuvY4nyEXhdBSiAW7Wn+EBACQuONaj+FnQC6UHSDS1K9l7itAjeK2s+AQ/g5u+ rqkTJIOo 769rhvEwJLOghRmzBJwnlxN5ai5fe8ZWCVPsOfKsAOP5tUf797oXKnlJTDYc1dCE4ZYMPjDIhkqdAqkLMdS91CT63/G3oDJAbM3pakTGPtc10Q3+xpl8WMOWGGdJwAqq/3CKNNBuMmWyCmeFj2i12MGNNgKvaEBY8tbTzmT36fz0mEzhSz6a3crOM7HyJ28D3yzSe09PCfqUgOrQ= 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 2024/4/19 3:40, Andrew Morton wrote: > On Thu, 18 Apr 2024 22:20:07 +0800 Peng Zhang wrote: > >> From: ZhangPeng >> >> Depending on whether counters is NULL, we can support two modes: >> atomic mode and perpcu mode. We implement both modes by grouping >> the s64 count and atomic64_t count_atomic in a union. At the same time, >> we create the interface for adding and reading in atomic mode and for >> switching atomic mode to percpu mode. >> > I think it would be better if we had a detailed code comment in an > appropriate place which fully describes the tradeoffs here. Tell > people when they would benefit from using one mode versus the other. > Thanks for your reply! Yes, of course, I would add before the union: /* * Depending on whether counters is NULL, we can support two modes, * atomic mode using count_atomic and perpcu mode using count. * The single-thread processes should use atomic mode to reduce the * memory consumption and performance regression. * The multiple-thread processes should use percpu mode to reduce the * error margin. */ union { s64 count; atomic64_t count_atomic; }; >> --- a/lib/percpu_counter.c >> +++ b/lib/percpu_counter.c >> @@ -153,7 +153,7 @@ EXPORT_SYMBOL(__percpu_counter_sum); >> >> int __percpu_counter_init_many(struct percpu_counter *fbc, s64 amount, >> gfp_t gfp, u32 nr_counters, >> - struct lock_class_key *key) >> + struct lock_class_key *key, bool switch_mode) >> { >> unsigned long flags __maybe_unused; >> size_t counter_size; >> @@ -174,7 +174,8 @@ int __percpu_counter_init_many(struct percpu_counter *fbc, s64 amount, >> #ifdef CONFIG_HOTPLUG_CPU >> INIT_LIST_HEAD(&fbc[i].list); >> #endif >> - fbc[i].count = amount; >> + if (likely(!switch_mode)) >> + fbc[i].count = amount; >> fbc[i].counters = (void *)counters + (i * counter_size); >> >> debug_percpu_counter_activate(&fbc[i]); >> @@ -357,6 +358,32 @@ bool __percpu_counter_limited_add(struct percpu_counter *fbc, >> return good; >> } >> >> +/* >> + * percpu_counter_switch_to_pcpu_many: Converts struct percpu_counters from >> + * atomic mode to percpu mode. > Describe what happens if that GFP_ATOMIC allocation fails. We remain > in atomic mode, yes? Yes. I'll add comments like: * Return: 0 if percpu_counter is already in atomic mode or successfully * switched to atomic mode; -ENOMEM if perpcu memory allocation fails, * perpcu_counter is still in atomic mode. >> + */ >> +int percpu_counter_switch_to_pcpu_many(struct percpu_counter *fbc, >> + u32 nr_counters) >> +{ >> + static struct lock_class_key __key; >> + unsigned long flags; >> + bool ret = 0; >> + >> + if (percpu_counter_initialized(fbc)) >> + return 0; >> + >> + preempt_disable(); >> + local_irq_save(flags); > Do we need both? Does local_irq_save() always disable preemption? > This might not be the case for RT kernels, I always forget. Yes, we need both. For RT kernels, local_irq_save() doesn't mean that preemption is disabled. >> + if (likely(!percpu_counter_initialized(fbc))) >> + ret = __percpu_counter_init_many(fbc, 0, >> + GFP_ATOMIC|__GFP_NOWARN|__GFP_ZERO, >> + nr_counters, &__key, true); >> + local_irq_restore(flags); >> + preempt_enable(); >> + >> + return ret; >> +} >> + > Why is there no API for switching back to atomic mode? At least for the current test scenario, we don't need to switch back to atomic mode. The scenario for switching back to atomic mode may be that the multi-threaded process destroys the last second thread. I could add this API if needed later. -- Best Regards, Peng