From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 3FF8C58E8 for ; Mon, 9 Feb 2015 18:30:20 +0100 (CET) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1YKsDc-0006mX-RV; Mon, 09 Feb 2015 18:34:07 +0100 Message-ID: <54D8EE9B.3060000@6wind.com> Date: Mon, 09 Feb 2015 18:30:03 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.3.0 MIME-Version: 1.0 To: "Liang, Cunming" , "dev@dpdk.org" References: <1422491072-5114-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-7-git-send-email-cunming.liang@intel.com> <54D7C04C.70308@6wind.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 06/17] eal: add eal_common_thread.c for common thread API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Feb 2015 17:30:20 -0000 Hi, On 02/09/2015 02:12 PM, Liang, Cunming wrote: >>> +int >>> +rte_thread_get_affinity(rte_cpuset_t *cpusetp) >>> +{ >>> + if (!cpusetp) >>> + return -1; >> >> Same here. This is the only reason why rte_thread_get_affinity() could >> fail. Removing this test would allow to change the API to return void >> instead. It will avoid a useless test below in >> eal_thread_dump_affinity(). > [LCM] The cpusetp is used as destination of memcpy and the function suppose an EAL API. > I don't think it's a good idea to remove the check, do you ? I know we often have debate on this subject on the list. My personal opinion is that checking a NULL pointer in these cases is useless because the user is suppose to give a non-NULL pointer. Returning an error will result in managing an error for something that cannot happen. On the other hand, adding an assert() (or the dpdk equivalent) would be a good idea. >> >>> + >>> + rte_memcpy(cpusetp, &RTE_PER_LCORE(_cpuset), >>> + sizeof(rte_cpuset_t)); >>> + >>> + return 0; >>> +} >>> + >>> +void >>> +eal_thread_dump_affinity(char str[], unsigned size) >>> +{ >>> + rte_cpuset_t cpuset; >>> + unsigned cpu; >>> + int ret; >>> + unsigned int out = 0; >>> + >>> + if (rte_thread_get_affinity(&cpuset) < 0) { >>> + str[0] = '\0'; >>> + return; >>> + } >> >> This one could be removed it the (== NULL) test is removed. >> >>> + >>> + for (cpu = 0; cpu < RTE_MAX_LCORE; cpu++) { >>> + if (!CPU_ISSET(cpu, &cpuset)) >>> + continue; >>> + >>> + ret = snprintf(str + out, >>> + size - out, "%u,", cpu); >>> + if (ret < 0 || (unsigned)ret >= size - out) >>> + break; >> >> On the contrary, I think here returning an error to the user >> would be useful so he can knows that the dump is not complete. > [LCM] accept. >> >> >> Regards, >> Olivier