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 X-Spam-Level: X-Spam-Status: No, score=-16.6 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6D9A3C432BE for ; Tue, 24 Aug 2021 04:52:19 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A208F6044F for ; Tue, 24 Aug 2021 04:52:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A208F6044F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:40414 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mIOQ5-0002SU-DD for qemu-devel@archiver.kernel.org; Tue, 24 Aug 2021 00:52:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:35426) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mIOPG-0001mo-I9 for qemu-devel@nongnu.org; Tue, 24 Aug 2021 00:51:26 -0400 Received: from szxga01-in.huawei.com ([45.249.212.187]:2473) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mIOPB-00059b-Go for qemu-devel@nongnu.org; Tue, 24 Aug 2021 00:51:25 -0400 Received: from dggemv703-chm.china.huawei.com (unknown [172.30.72.55]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4GtxRc5qjRzbgqt; Tue, 24 Aug 2021 12:47:24 +0800 (CST) Received: from dggpemm500023.china.huawei.com (7.185.36.83) by dggemv703-chm.china.huawei.com (10.3.19.46) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Tue, 24 Aug 2021 12:51:13 +0800 Received: from [10.174.187.128] (10.174.187.128) by dggpemm500023.china.huawei.com (7.185.36.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Tue, 24 Aug 2021 12:51:12 +0800 Subject: Re: [PATCH v7 05/15] machine: Improve the error reporting of smp parsing To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , , Eduardo Habkost , Igor Mammedov References: <20210823122804.7692-1-wangyanan55@huawei.com> <20210823122804.7692-6-wangyanan55@huawei.com> From: "wangyanan (Y)" Message-ID: <4b49fb0c-ec73-d8ca-f622-cc8e21ed0140@huawei.com> Date: Tue, 24 Aug 2021 12:51:12 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.174.187.128] X-ClientProxiedBy: dggeme709-chm.china.huawei.com (10.1.199.105) To dggpemm500023.china.huawei.com (7.185.36.83) X-CFilter-Loop: Reflected Received-SPF: pass client-ip=45.249.212.187; envelope-from=wangyanan55@huawei.com; helo=szxga01-in.huawei.com X-Spam_score_int: -61 X-Spam_score: -6.2 X-Spam_bar: ------ X-Spam_report: (-6.2 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-2.023, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Andrew Jones , =?UTF-8?Q?Daniel_P_=2e_Berrang=c3=a9?= , Pierre Morel , "Michael S . Tsirkin" , Cornelia Huck , Richard Henderson , Greg Kurz , Halil Pasic , Paolo Bonzini , Pankaj Gupta , Thomas Huth , wanghaibin.wang@huawei.com, David Gibson Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 2021/8/23 21:17, Philippe Mathieu-Daudé wrote: > On 8/23/21 2:27 PM, Yanan Wang wrote: >> We have two requirements for a valid SMP configuration: >> the product of "sockets * cores * threads" must represent all the >> possible cpus, i.e., max_cpus, and then must include the initially >> present cpus, i.e., smp_cpus. >> >> So we only need to ensure 1) "sockets * cores * threads == maxcpus" >> at first and then ensure 2) "maxcpus >= cpus". With a reasonable >> order of the sanity check, we can simplify the error reporting code. >> When reporting an error message we also report the exact value of >> each topology member to make users easily see what's going on. >> >> Signed-off-by: Yanan Wang >> Reviewed-by: Andrew Jones >> Reviewed-by: Pankaj Gupta >> --- >> hw/core/machine.c | 22 +++++++++------------- >> hw/i386/pc.c | 24 ++++++++++-------------- >> 2 files changed, 19 insertions(+), 27 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 85908abc77..093c0d382d 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -779,25 +779,21 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) >> maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads; >> cpus = cpus > 0 ? cpus : maxcpus; >> >> - if (sockets * cores * threads < cpus) { >> - error_setg(errp, "cpu topology: " >> - "sockets (%u) * cores (%u) * threads (%u) < " >> - "smp_cpus (%u)", >> - sockets, cores, threads, cpus); >> + if (sockets * cores * threads != maxcpus) { >> + error_setg(errp, "Invalid CPU topology: " >> + "product of the hierarchy must match maxcpus: " >> + "sockets (%u) * cores (%u) * threads (%u) " >> + "!= maxcpus (%u)", >> + sockets, cores, threads, maxcpus); >> return; >> } > Thinking about scalability, MachineClass could have a > parse_cpu_topology() handler, and this would be the > generic one. Principally because architectures don't > use the same terms, and die/socket/core/thread arrangement > is machine specific (besides being arch-spec). > Not a problem as of today, but the way we try to handle > this generically seems over-engineered to me. Hi Philippe, The reason for introducing a generic implementation and avoiding specific ones is that we thought there is little difference in parsing logic between the specific parsers. Most part of the parsing is the automatic calculation of missing values and the related error reporting, in which the only difference between parsers is the handling of specific (no matter of arch-specific or machine-specifc) parameters. So it may be better to keep the parsing logic unified if we can easily realize that. And actually we can use compat stuff to handle specific topology parameters well. See implementation in patch #10. There have been patches on list introducing new specific members (s390 related in [1] and ARM related in [2]), and in each of them there is a specific parser needed. However, based on generic one we can extend without the increasing code duplication. There is also some discussion about generic/specific parser in [1], which can be a reference. [1] https://lore.kernel.org/qemu-devel/1626281596-31061-2-git-send-email-pmorel@linux.ibm.com/ [2] https://lore.kernel.org/qemu-devel/20210516103228.37792-1-wangyanan55@huawei.com/ Thanks, Yanan . > [unrelated to this particular patch] > > .