Date: Thu, 24 Sep 2015 13:52:55 -0700 From: Jason Schulz <schulz.j@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-threads@freebsd.org, jhb@freebsd.org Subject: Re: pthread_getaffinity_np issue Message-ID: <20150924205255.GI3104@gmail.com> In-Reply-To: <20150924192623.GL67105@kib.kiev.ua> References: <20150924181127.GC3104@gmail.com> <20150924183246.GK67105@kib.kiev.ua> <20150924191121.GG3104@gmail.com> <20150924192623.GL67105@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 24, 2015 at 10:26:23PM +0300, Konstantin Belousov wrote:
> On Thu, Sep 24, 2015 at 12:11:21PM -0700, Jason Schulz wrote:
> > Hey Konstantin,
> >
> > Thanks for the quick response. The version I'm developing with is the latest release.
> >
> > [jason@fb ~/development/git/uxcn/yafd]$ uname -a
> > FreeBSD fb 10.2-RELEASE FreeBSD 10.2-RELEASE #0 r286666: Wed Aug 12 15:26:37 UTC 2015 root@releng1.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64
> >
> > I was able to narrow it down to the following,
> >
> > #include <stdio.h>
> >
> > #include <pthread.h>
> > #include <pthread_np.h>
> >
> > int main(int argc, char* argv[argc]) {
> >
> > long cs = 0;
> >
> > pthread_t this = pthread_self();
> >
> > cpuset_t cpus;
> >
> > CPU_ZERO(&cpus);
> >
> > int err = pthread_getaffinity_np(this, sizeof(cpus), &cpus);
> >
> > if (err)
> > return err;
> >
> > cs = (long) CPU_COUNT(&cpus);
> >
> > printf("num cpus... %li\n", cs);
> >
> > return 0;
> > }
> >
> > Compiling with '-D_XOPEN_SOURCE=700',
> >
> > [jason@fb /tmp]$ clang -D_XOPEN_SOURCE=700 -pthread -o num_cpus num_cpus.c
> > num_cpus.c:21:15: warning: implicit declaration of function '__bitcountl' is invalid in C99 [-Wimplicit-function-declaration]
> > cs = (long) CPU_COUNT(&cpus);
> > ^
> > /usr/include/sys/cpuset.h:63:24: note: expanded from macro 'CPU_COUNT'
> > #define CPU_COUNT(p) BIT_COUNT(CPU_SETSIZE, p)
> > ^
> > /usr/include/sys/bitset.h:185:14: note: expanded from macro 'BIT_COUNT'
> > __count += __bitcountl((p)->__bits[__i]); \
> > ^
> > 1 warning generated.
> > /tmp/num_cpus-560a33.o: In function `main':
> > num_cpus.c:(.text+0xbd): undefined reference to `__bitcountl'
> > clang: error: linker command failed with exit code 1 (use -v to see invocation)
> >
> > However compiling without '-D_XOPEN_SOURCE=700' works,
> >
> > [jason@fb /tmp]$ clang -pthread -o num_cpus num_cpus.c
> > [jason@fb /tmp]$
> >
> > Should I file a bug report?
>
> So your issue only appears when you specify _XOPEN_SOURCE. This is reasonable,
> because __bitcount stuff in sys/types.h was put under _BSD_VISIBLE protection.
> I am not sure why and I Cc:ed the author of the change.
>
> IMO all symbols from __bitcount* are in the private namespace and could
> be made unconditionally accessible, to avoid surprises. I put the patch
> at the end of the message which does this.
>
> That said, pthread_np.h is not specified by POSIX/X/Open, so your use of
> the header and affinity functionality is not covered by _XOPEN_SOURCE
> contract. The consequences of mixing implementation-specific headers
> and request for the standard-compliance can be arbitrary.
>
> >
> > -J
> >
> > On Thu, Sep 24, 2015 at 09:32:46PM +0300, Konstantin Belousov wrote:
> > > On Thu, Sep 24, 2015 at 11:11:27AM -0700, Jason Schulz wrote:
> > > > I'm having an issue using the non-posix thread affinity methods. Specifically, I'm having trouble using CPU_COUNT. I'm trying to use the following code to get the current number of processors in a processes affinity...
> > > >
> > > >
> > > >
> > > > long cs = 0;
> > > >
> > > > #if defined(HAVE_LINUX) || defined(HAVE_FREEBSD)
> > > >
> > > > pthread_t this = pthread_self();
> > > >
> > > > cpu_set_t cpus;
> > > >
> > > > CPU_ZERO(&cpus);
> > > >
> > > > int err = pthread_getaffinity_np(this, sizeof(cpus), &cpus);
> > > >
> > > > if (err)
> > > > return err;
> > > >
> > > > cs = (long) CPU_COUNT(&cpus);
> > > >
> > > >
> > > >
> > > > However, I get the compiler error...
> > > >
> > > >
> > > >
> > > > clang -DHAVE_CONFIG_H -I. -g -O2 -D_THREAD_SAFE -pthread -pipe -std=c99 -D_XOPEN_SOURCE=700 -pedantic -Wall -Wextra -Wsign-conversion -Wconversion -Werror -MT thread.o -MD -MP -MF .deps/thread.Tpo -c -o thread.o thread.c
> > > > thread.c:85:15: error: implicit declaration of function '__bitcountl' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
> > > > cs = (long) CPU_COUNT(&cpus);
> > > > ^
> > > > /usr/include/sys/cpuset.h:63:24: note: expanded from macro 'CPU_COUNT'
> > > > #define CPU_COUNT(p) BIT_COUNT(CPU_SETSIZE, p)
> > > > ^
> > > > /usr/include/sys/bitset.h:185:14: note: expanded from macro 'BIT_COUNT'
> > > > __count += __bitcountl((p)->__bits[__i]); \
> > > > ^
> > > > 1 error generated.
> > > >
> > > >
> > > >
> > > > I'm able to use CPU_ISSET to the same effect, but obviously I'd rather use CPU_COUNT. Suggestions?
> > > >
> > >
> > > Show the minimal example demonstrating the issue, specify the version
> > > of the system and which you use. My guess is that you did not added
> > > #include <sys/types.h> before including sys/cpuset.h.
>
> diff --git a/sys/sys/types.h b/sys/sys/types.h
> index 4a66a4e..4dfd8ef 100644
> --- a/sys/sys/types.h
> +++ b/sys/sys/types.h
> @@ -290,9 +290,6 @@ typedef _Bool bool;
> * The following are all things that really shouldn't exist in this header,
> * since its purpose is to provide typedefs, not miscellaneous doodads.
> */
> -#if __BSD_VISIBLE
> -
> -#include <sys/select.h>
>
> #ifdef __POPCNT__
> #define __bitcount64(x) __builtin_popcountll((__uint64_t)(x))
> @@ -356,6 +353,10 @@ __bitcount64(__uint64_t _x)
> #define __bitcount(x) __bitcount32((unsigned int)(x))
> #endif
>
> +#if __BSD_VISIBLE
> +
> +#include <sys/select.h>
> +
> /*
> * minor() gives a cookie instead of an index since we don't want to
> * change the meanings of bits 0-15 or waste time and space shifting
Affinity functionality (cpusets, etc...) definitely seems to be outside the
scope of POSIX. sysconf isn't, but my opinion is that it's more considerate to
query cpus within the affinity group rather than just the raw number of
processors.
Honestly I want the software to be able run as many versions of FreeBSD as
possible anyway, so I'll probably be using CPU_ISSET, but using CPU_COUNT would
probably be a little cleaner though. For simple stuff at least, I'm trying to
keep it as platform agnostic as possible.
-J
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150924205255.GI3104>