android: util/u_thread: fix building error with bionic pthread

Submitted by Mauro Rossi on Sept. 10, 2018, 7:52 a.m.

Details

Message ID 20180910075208.5176-1-issor.oruam@gmail.com
State New
Headers show
Series "android: util/u_thread: fix building error with bionic pthread" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mauro Rossi Sept. 10, 2018, 7:52 a.m.
This patch is to tackle with shortcomings in Android bionic libc.
Even if setting cflag -D__USE__GNU and adding include of <sched.h> cpuset macros 
may become available, bionic libc does not support pthread_{g,s}etaffinity_np()

Wrappers to sched_{g,s]etaffinity() were found to here:
www.spinics.net/lists/linux-rt-users/msg16928.html
but I'm not sure that passing 0 as pid (the means "for the current process")
would works to pin the thread and if sched_{g,s]etaffinity() makes sense.

Feedback about opportunity to use wrappers implementation is appreciated,
I have tested also the build with wrappers, but I need to know if it may work.

For now preprocessor conditional is added to avoid following building errors:

In file included from external/mesa/src/gallium/auxiliary/os/os_thread.h:42:
external/mesa/src/util/u_thread.h:93:4: error: use of undeclared identifier 'pthread_setaffinity_np'
   pthread_setaffinity_np(thread, sizeof(cpuset), &cpuset);
   ^~~~~~~~~~~~~~~~~~~~~~
...
In file included from external/mesa/src/gallium/auxiliary/os/os_thread.h:42:
external/mesa/src/util/u_thread.h:110:8: error: use of undeclared identifier 'pthread_getaffinity_np'
   if (pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset) == 0) {
       ^
2 errors generated.

Fixes: 8d473f555a ("st/mesa: pin driver threads to a specific L3 cache on AMD Zen (v2)")
---
 src/util/u_thread.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/util/u_thread.h b/src/util/u_thread.h
index ec0d9a7f36..7419dd7e2c 100644
--- a/src/util/u_thread.h
+++ b/src/util/u_thread.h
@@ -83,7 +83,7 @@  static inline void u_thread_setname( const char *name )
 static inline void
 util_pin_thread_to_L3(thrd_t thread, unsigned L3_index, unsigned cores_per_L3)
 {
-#if defined(HAVE_PTHREAD)
+#if defined(HAVE_PTHREAD) && !defined(ANDROID)
    cpu_set_t cpuset;
 
    CPU_ZERO(&cpuset);
@@ -103,7 +103,7 @@  util_pin_thread_to_L3(thrd_t thread, unsigned L3_index, unsigned cores_per_L3)
 static inline int
 util_get_L3_for_pinned_thread(thrd_t thread, unsigned cores_per_L3)
 {
-#if defined(HAVE_PTHREAD)
+#if defined(HAVE_PTHREAD) && !defined(ANDROID)
    cpu_set_t cpuset;
 
    if (pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset) == 0) {

Comments

Marek sent a similar fix here:
https://lists.freedesktop.org/archives/mesa-dev/2018-September/204797.html

On 09/10/2018 10:52 AM, Mauro Rossi wrote:
> This patch is to tackle with shortcomings in Android bionic libc.
> Even if setting cflag -D__USE__GNU and adding include of <sched.h> cpuset macros
> may become available, bionic libc does not support pthread_{g,s}etaffinity_np()
> 
> Wrappers to sched_{g,s]etaffinity() were found to here:
> www.spinics.net/lists/linux-rt-users/msg16928.html
> but I'm not sure that passing 0 as pid (the means "for the current process")
> would works to pin the thread and if sched_{g,s]etaffinity() makes sense.
> 
> Feedback about opportunity to use wrappers implementation is appreciated,
> I have tested also the build with wrappers, but I need to know if it may work.
> 
> For now preprocessor conditional is added to avoid following building errors:
> 
> In file included from external/mesa/src/gallium/auxiliary/os/os_thread.h:42:
> external/mesa/src/util/u_thread.h:93:4: error: use of undeclared identifier 'pthread_setaffinity_np'
>     pthread_setaffinity_np(thread, sizeof(cpuset), &cpuset);
>     ^~~~~~~~~~~~~~~~~~~~~~
> ...
> In file included from external/mesa/src/gallium/auxiliary/os/os_thread.h:42:
> external/mesa/src/util/u_thread.h:110:8: error: use of undeclared identifier 'pthread_getaffinity_np'
>     if (pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset) == 0) {
>         ^
> 2 errors generated.
> 
> Fixes: 8d473f555a ("st/mesa: pin driver threads to a specific L3 cache on AMD Zen (v2)")
> ---
>   src/util/u_thread.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/u_thread.h b/src/util/u_thread.h
> index ec0d9a7f36..7419dd7e2c 100644
> --- a/src/util/u_thread.h
> +++ b/src/util/u_thread.h
> @@ -83,7 +83,7 @@ static inline void u_thread_setname( const char *name )
>   static inline void
>   util_pin_thread_to_L3(thrd_t thread, unsigned L3_index, unsigned cores_per_L3)
>   {
> -#if defined(HAVE_PTHREAD)
> +#if defined(HAVE_PTHREAD) && !defined(ANDROID)
>      cpu_set_t cpuset;
>   
>      CPU_ZERO(&cpuset);
> @@ -103,7 +103,7 @@ util_pin_thread_to_L3(thrd_t thread, unsigned L3_index, unsigned cores_per_L3)
>   static inline int
>   util_get_L3_for_pinned_thread(thrd_t thread, unsigned cores_per_L3)
>   {
> -#if defined(HAVE_PTHREAD)
> +#if defined(HAVE_PTHREAD) && !defined(ANDROID)
>      cpu_set_t cpuset;
>   
>      if (pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset) == 0) {
>
Hi,

Il giorno lun 10 set 2018 alle ore 09:58 Tapani Pälli
<tapani.palli@intel.com> ha scritto:
>
> Marek sent a similar fix here:
> https://lists.freedesktop.org/archives/mesa-dev/2018-September/204797.html

Thanks a lot!
So the APU L3 cache optimization will be lost for Android

>
> On 09/10/2018 10:52 AM, Mauro Rossi wrote:
> > This patch is to tackle with shortcomings in Android bionic libc.
> > Even if setting cflag -D__USE__GNU and adding include of <sched.h> cpuset macros
> > may become available, bionic libc does not support pthread_{g,s}etaffinity_np()
> >
> > Wrappers to sched_{g,s]etaffinity() were found to here:
> > www.spinics.net/lists/linux-rt-users/msg16928.html
> > but I'm not sure that passing 0 as pid (the means "for the current process")
> > would works to pin the thread and if sched_{g,s]etaffinity() makes sense.
> >
> > Feedback about opportunity to use wrappers implementation is appreciated,
> > I have tested also the build with wrappers, but I need to know if it may work.

Provided that Android bionic libc should not have been chopped,
as now it has part of sched.h and does not have thread affinity,
going back to my doubt, would those wrappers work or not?

Mauro

> >
> > For now preprocessor conditional is added to avoid following building errors:
> >
> > In file included from external/mesa/src/gallium/auxiliary/os/os_thread.h:42:
> > external/mesa/src/util/u_thread.h:93:4: error: use of undeclared identifier 'pthread_setaffinity_np'
> >     pthread_setaffinity_np(thread, sizeof(cpuset), &cpuset);
> >     ^~~~~~~~~~~~~~~~~~~~~~
> > ...
> > In file included from external/mesa/src/gallium/auxiliary/os/os_thread.h:42:
> > external/mesa/src/util/u_thread.h:110:8: error: use of undeclared identifier 'pthread_getaffinity_np'
> >     if (pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset) == 0) {
> >         ^
> > 2 errors generated.
> >
> > Fixes: 8d473f555a ("st/mesa: pin driver threads to a specific L3 cache on AMD Zen (v2)")
> > ---
> >   src/util/u_thread.h | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/util/u_thread.h b/src/util/u_thread.h
> > index ec0d9a7f36..7419dd7e2c 100644
> > --- a/src/util/u_thread.h
> > +++ b/src/util/u_thread.h
> > @@ -83,7 +83,7 @@ static inline void u_thread_setname( const char *name )
> >   static inline void
> >   util_pin_thread_to_L3(thrd_t thread, unsigned L3_index, unsigned cores_per_L3)
> >   {
> > -#if defined(HAVE_PTHREAD)
> > +#if defined(HAVE_PTHREAD) && !defined(ANDROID)
> >      cpu_set_t cpuset;
> >
> >      CPU_ZERO(&cpuset);
> > @@ -103,7 +103,7 @@ util_pin_thread_to_L3(thrd_t thread, unsigned L3_index, unsigned cores_per_L3)
> >   static inline int
> >   util_get_L3_for_pinned_thread(thrd_t thread, unsigned cores_per_L3)
> >   {
> > -#if defined(HAVE_PTHREAD)
> > +#if defined(HAVE_PTHREAD) && !defined(ANDROID)
> >      cpu_set_t cpuset;
> >
> >      if (pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset) == 0) {
> >
On Mon, Sep 10, 2018 at 4:07 AM, Mauro Rossi <issor.oruam@gmail.com> wrote:
> Hi,
>
> Il giorno lun 10 set 2018 alle ore 09:58 Tapani Pälli
> <tapani.palli@intel.com> ha scritto:
>>
>> Marek sent a similar fix here:
>> https://lists.freedesktop.org/archives/mesa-dev/2018-September/204797.html
>
> Thanks a lot!
> So the APU L3 cache optimization will be lost for Android

Hopefully Android won't be used on Ryzens with multiple CCXs. CPUs
without graphics have at least 2 CCXs. APUs have only 1.

Marek