[dpdk-dev] [PATCH v4 06/17] eal: add eal_common_thread.c for common thread API

Liang, Cunming cunming.liang at intel.com
Tue Feb 10 03:46:23 CET 2015



> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Tuesday, February 10, 2015 1:30 AM
> To: Liang, Cunming; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 06/17] eal: add eal_common_thread.c for
> common thread API
> 
> 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.
[LCM] Ok, I see. Will update it.
> 
> 
> >>
> >>> +
> >>> +	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


More information about the dev mailing list