From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id B040C58F4 for ; Tue, 10 Feb 2015 03:46:28 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 09 Feb 2015 18:46:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,547,1418112000"; d="scan'208";a="664028302" Received: from pgsmsx106.gar.corp.intel.com ([10.221.44.98]) by fmsmga001.fm.intel.com with ESMTP; 09 Feb 2015 18:46:25 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by PGSMSX106.gar.corp.intel.com (10.221.44.98) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 10 Feb 2015 10:46:23 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.62]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.192]) with mapi id 14.03.0195.001; Tue, 10 Feb 2015 10:46:23 +0800 From: "Liang, Cunming" To: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 06/17] eal: add eal_common_thread.c for common thread API Thread-Index: AQHQPoxkCcxjkTcYs0q2zE+79mvOm5zmsHwAgAGf7sD//8h0gIABIXmg Date: Tue, 10 Feb 2015 02:46:23 +0000 Message-ID: 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> <54D8EE9B.3060000@6wind.com> In-Reply-To: <54D8EE9B.3060000@6wind.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Tue, 10 Feb 2015 02:46:29 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Tuesday, February 10, 2015 1:30 AM > To: Liang, Cunming; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 06/17] eal: add eal_common_thread.c for > common thread API >=20 > Hi, >=20 > 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 sup= pose > an EAL API. > > I don't think it's a good idea to remove the check, do you ? >=20 > 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. >=20 > On the other hand, adding an assert() (or the dpdk equivalent) would > be a good idea. [LCM] Ok, I see. Will update it. >=20 >=20 > >> > >>> + > >>> + 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 =3D 0; > >>> + > >>> + if (rte_thread_get_affinity(&cpuset) < 0) { > >>> + str[0] =3D '\0'; > >>> + return; > >>> + } > >> > >> This one could be removed it the (=3D=3D NULL) test is removed. > >> > >>> + > >>> + for (cpu =3D 0; cpu < RTE_MAX_LCORE; cpu++) { > >>> + if (!CPU_ISSET(cpu, &cpuset)) > >>> + continue; > >>> + > >>> + ret =3D snprintf(str + out, > >>> + size - out, "%u,", cpu); > >>> + if (ret < 0 || (unsigned)ret >=3D 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