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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DBE76E7718E for ; Mon, 23 Dec 2024 16:33:29 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [127.0.0.1]) by lists.ozlabs.org (Postfix) with ESMTP id 4YH3Vb6WkKz2yZ4; Tue, 24 Dec 2024 03:33:27 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; arc=none smtp.remote-ip=185.176.79.56 ARC-Seal: i=1; a=rsa-sha256; d=lists.ozlabs.org; s=201707; t=1734971607; cv=none; b=GCQBAVgi3TQtdKodqxKRn2+kw97diVDXxfXAm8yppVnrcVGlRk9v11ubXvGkdmZK5CG6+Ud8/Y7DRxWQ+Or1A1CqG0NvFey3gHx9o5MyDkDoxtueDeuTbv9OPe46GoP212AbqRPNh6eD8tSUSTEMls6Tu6lPXlXC0/lbW1cw7YMoqMzKWLPniueQDdYI/7kAcJYciTGp0pFkgYiZ/jWhIr0TWcJMbh69y7Q4f1J3+7BnT6P3jXJQSd+K/jtEyiW99kWNvYmLNxqg3KBObbQ0szJTTuKhAQ7u9a7ueZBYsmiFBGYAO9T+BuaDTetYsGomObTBSI2OEKS3SSvlvHeQrA== ARC-Message-Signature: i=1; a=rsa-sha256; d=lists.ozlabs.org; s=201707; t=1734971607; c=relaxed/relaxed; bh=UlwxU9yU5UoX8moS94KBcUEK1tmdXrMY1Z1yZrD4Gl4=; h=Date:From:To:CC:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=RErItgyf/rW72vbRoKlmXBPeXMKFEYtUyWS2PUB8W6pufGo4FOB7XYwjMS+vKlCygLaa9nkraoaQ5Vl+253dDHIRbs3DzwiNVA9j+3xZe6IEhxz1YpydjxrhXtX1ffrz5jXMjQ/fsQul1z/0zWtYz1Ei3DxhLz4HiN8CarB3KrkQPRnGdmo4LHURIJcrxSuZKy6WTdGttq+g2/kFuBgMuL7lKat+LyMGNSJTFo/RnaSASCRlCSfoRL7SVn6YBtcrHfogoIh1ByG3xaDui5hxNTRBzL4T5XnIGpP7HEbbOJ+uROt2VLQPCqUzD8olc1WkTgfKa+Xt/1wuWy77wG+rAQ== ARC-Authentication-Results: i=1; lists.ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass (client-ip=185.176.79.56; helo=frasgout.his.huawei.com; envelope-from=jonathan.cameron@huawei.com; receiver=lists.ozlabs.org) smtp.mailfrom=huawei.com Authentication-Results: lists.ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=huawei.com (client-ip=185.176.79.56; helo=frasgout.his.huawei.com; envelope-from=jonathan.cameron@huawei.com; receiver=lists.ozlabs.org) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4YH3VZ1dCdz2xxw for ; Tue, 24 Dec 2024 03:33:22 +1100 (AEDT) Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4YH3V50KhNz6K75k; Tue, 24 Dec 2024 00:33:01 +0800 (CST) Received: from frapeml500008.china.huawei.com (unknown [7.182.85.71]) by mail.maildlp.com (Postfix) with ESMTPS id E7597140517; Tue, 24 Dec 2024 00:33:17 +0800 (CST) Received: from localhost (10.47.75.118) by frapeml500008.china.huawei.com (7.182.85.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Mon, 23 Dec 2024 17:33:16 +0100 Date: Mon, 23 Dec 2024 16:33:14 +0000 From: Jonathan Cameron To: Yicong Yang CC: , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH v10 2/4] arch_topology: Support SMT control for OF based system Message-ID: <20241223163314.00006fed@huawei.com> In-Reply-To: <20241220075313.51502-3-yangyicong@huawei.com> References: <20241220075313.51502-1-yangyicong@huawei.com> <20241220075313.51502-3-yangyicong@huawei.com> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.42; x86_64-w64-mingw32) X-Mailing-List: linuxppc-dev@lists.ozlabs.org List-Id: List-Help: List-Owner: List-Post: List-Archive: , List-Subscribe: , , List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.47.75.118] X-ClientProxiedBy: lhrpeml100002.china.huawei.com (7.191.160.241) To frapeml500008.china.huawei.com (7.182.85.71) On Fri, 20 Dec 2024 15:53:11 +0800 Yicong Yang wrote: > From: Yicong Yang > > On building the topology from the devicetree, we've already > gotten the SMT thread number of each core. Update the largest > SMT thread number and enable the SMT control by the end of > topology parsing. > > The core's SMT control provides two interface to the users [1]: > 1) enable/disable SMT by writing on/off > 2) enable/disable SMT by writing thread number 1/max_thread_number > > If a system have more than one SMT thread number the 2) may > not handle it well, since there're multiple thread numbers in the > system and 2) only accept 1/max_thread_number. So issue a warning > to notify the users if such system detected. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-devices-system-cpu#n542 > Signed-off-by: Yicong Yang Hi Yicong, Apologies that I'm late to the game on this one. A few comments inline. Only important one is whether to bail out early on error from parse_cluster() Thanks, Jonathan > --- > drivers/base/arch_topology.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index 3ebe77566788..9e81060144c7 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -506,6 +507,10 @@ core_initcall(free_raw_capacity); > #endif > > #if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > + > +/* Maximum SMT thread number detected used to enable the SMT control */ > +static unsigned int max_smt_thread_num; > + > /* > * This function returns the logic cpu number of the node. > * There are basically three kinds of return values: > @@ -565,6 +570,17 @@ static int __init parse_core(struct device_node *core, int package_id, > i++; > } while (1); > > + /* > + * If max_smt_thread_num has been initialized and doesn't match > + * the thread number of this entry, then the system has > + * heterogeneous SMT topology. > + */ > + if (max_smt_thread_num && max_smt_thread_num != i) > + pr_warn_once("Heterogeneous SMT topology is partly supported by SMT control\n"); > + > + if (max_smt_thread_num < i) > + max_smt_thread_num = i; Maybe more self documenting if you use min()? I'm not sure... max_smt_thread_num = min(max_smt_thread_num, i); > + > cpu = get_cpu_for_node(core); > if (cpu >= 0) { > if (!leaf) { > @@ -677,6 +693,16 @@ static int __init parse_socket(struct device_node *socket) > if (!has_socket) > ret = parse_cluster(socket, 0, -1, 0); Is it appropriate to check ret before setting num threads? if (!has_socket) { ret = parse_cluster(socket, 0, -1, 0); if (ret) return ret; } ... return 0; > > + /* > + * Notify the CPU framework of the SMT support. Initialize the > + * max_smt_thread_num to 1 if no SMT support detected. A thread > + * number of 1 can be handled by the framework so we don't need > + * to check max_smt_thread_num to see we support SMT or not. > + */ > + if (!max_smt_thread_num) > + max_smt_thread_num = 1; > + > + cpu_smt_set_num_threads(max_smt_thread_num, max_smt_thread_num); Trivial but I'd put a blank line here. > return ret; > } >