From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AB6BE41C45; Thu, 9 Feb 2023 03:19:02 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 46CB340DDA; Thu, 9 Feb 2023 03:19:02 +0100 (CET) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id A22884067B for ; Thu, 9 Feb 2023 03:19:00 +0100 (CET) Received: from kwepemm600004.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4PC0mY0nr3zJr08; Thu, 9 Feb 2023 10:14:21 +0800 (CST) Received: from [10.67.103.231] (10.67.103.231) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Thu, 9 Feb 2023 10:18:58 +0800 Message-ID: <373cbd89-3efe-fc4c-2300-edead9c0808c@huawei.com> Date: Thu, 9 Feb 2023 10:18:57 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH v9 1/5] eal: add lcore info in telemetry To: Robin Jarry , References: <20221123102612.1688865-1-rjarry@redhat.com> <20230207193731.1242505-2-rjarry@redhat.com> <3cd7617f-dbb5-9626-2db1-e6a8ce5f6376@huawei.com> From: "lihuisong (C)" CC: =?UTF-8?Q?Morten_Br=c3=b8rup?= , Kevin Laatz , Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600004.china.huawei.com (7.193.23.242) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 在 2023/2/9 1:04, Robin Jarry 写道: > Hi lihuisong, > > lihuisong (C), Feb 08, 2023 at 03:24: >>> static int >>> lcore_dump_cb(unsigned int lcore_id, void *arg) >>> { >>> struct rte_config *cfg = rte_eal_get_configuration(); >>> char cpuset[RTE_CPU_AFFINITY_STR_LEN]; >>> - const char *role; >>> FILE *f = arg; >>> int ret; >>> >>> - switch (cfg->lcore_role[lcore_id]) { >>> - case ROLE_RTE: >>> - role = "RTE"; >>> - break; >>> - case ROLE_SERVICE: >>> - role = "SERVICE"; >>> - break; >>> - case ROLE_NON_EAL: >>> - role = "NON_EAL"; >>> - break; >>> - default: >>> - role = "UNKNOWN"; >>> - break; >>> - } >>> - >>> ret = eal_thread_dump_affinity(&lcore_config[lcore_id].cpuset, cpuset, >>> sizeof(cpuset)); >>> fprintf(f, "lcore %u, socket %u, role %s, cpuset %s%s\n", lcore_id, >>> - rte_lcore_to_socket_id(lcore_id), role, cpuset, >>> - ret == 0 ? "" : "..."); >>> + rte_lcore_to_socket_id(lcore_id), >>> + lcore_role_str(cfg->lcore_role[lcore_id]), >>> + cpuset, ret == 0 ? "" : "..."); >>> return 0; >>> } >> The above modification doesn't seem to be related to this patch. >> Suggest remove or delete it from this patch. > I was asked in an earlier review to factorize this into an helper to > avoid code duplication. ok, this patch also use lcore_role_str function. please ignore this comment. > >>> + if (info->lcore_id != lcore_id) >> Suggest: info->lcore_id != lcore_id -> lcore_id != info->lcore_id >> Here, info->lcore_id is a target and lcore_id is the variable to be >> judged, right? > Yeah that looks better. I didn't pay too much attention since this > principle is not well respected in the current code base. That's not a very good reason. It's similar to "ret != 0" and "p != NULL" in DPDK coding style. > > .