| Message ID | 0372d63243979fb93d566609b913cf1ee3c15097.1589884403.git.fweimer@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Signal mask for timer helper thread | expand |
On 5/19/20 6:44 AM, Florian Weimer via Libc-alpha wrote: > This introduces the function __pthread_attr_extension to allocate the > extension space, which is freed by pthread_attr_destroy. OK for master with one typo fixed. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > --- > nptl/Makefile | 1 + > nptl/pthreadP.h | 6 +++++ > nptl/pthread_attr_copy.c | 16 +++++++------ > nptl/pthread_attr_destroy.c | 12 ++++++---- > nptl/pthread_attr_extension.c | 32 ++++++++++++++++++++++++++ > nptl/pthread_attr_getaffinity.c | 14 +++++------ > nptl/pthread_attr_setaffinity.c | 23 +++++++++++------- > nptl/pthread_create.c | 2 +- > sysdeps/nptl/internaltypes.h | 16 ++++++++++--- > sysdeps/unix/sysv/linux/createthread.c | 9 +++++--- > 10 files changed, 98 insertions(+), 33 deletions(-) > create mode 100644 nptl/pthread_attr_extension.c > > diff --git a/nptl/Makefile b/nptl/Makefile > index e5686b20ac..d6f12d0371 100644 > --- a/nptl/Makefile > +++ b/nptl/Makefile > @@ -41,6 +41,7 @@ routines = \ > pthread_atfork \ > pthread_attr_copy \ > pthread_attr_destroy \ > + pthread_attr_extension \ OK. > pthread_attr_getdetachstate \ > pthread_attr_getinheritsched \ > pthread_attr_getschedparam \ > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > index ed30b72923..7b3153594e 100644 > --- a/nptl/pthreadP.h > +++ b/nptl/pthreadP.h > @@ -578,6 +578,12 @@ extern void __shm_directory_freeres (void) attribute_hidden; > > extern void __wait_lookup_done (void) attribute_hidden; > > +/* Allocates the extension space for ATTR. Returns an error code on > + memory allocation failure, zero on success. If ATTR already has an > + extension space, this function does nothing. */ > +int __pthread_attr_extension (struct pthread_attr *attr) attribute_hidden > + __attribute_warn_unused_result__; OK. Good comment. > + > #ifdef SHARED > # define PTHREAD_STATIC_FN_REQUIRE(name) > #else > diff --git a/nptl/pthread_attr_copy.c b/nptl/pthread_attr_copy.c > index 77a1a43eeb..eb29557505 100644 > --- a/nptl/pthread_attr_copy.c > +++ b/nptl/pthread_attr_copy.c > @@ -29,18 +29,20 @@ __pthread_attr_copy (pthread_attr_t *target, const pthread_attr_t *source) > temp.external = *source; > > /* Force new allocation. This function has full ownership of temp. */ > - temp.internal.cpuset = NULL; > - temp.internal.cpusetsize = 0; > + temp.internal.extension = NULL; OK. Init. > > int ret = 0; > > struct pthread_attr *isource = (struct pthread_attr *) source; > > - /* Propagate affinity mask information. */ > - if (isource->cpusetsize > 0) > - ret = __pthread_attr_setaffinity_np (&temp.external, > - isource->cpusetsize, > - isource->cpuset); > + if (isource->extension != NULL) > + { > + /* Propagate affinity mask information. */ > + if (isource->extension->cpusetsize > 0) > + ret = __pthread_attr_setaffinity_np (&temp.external, > + isource->extension->cpusetsize, > + isource->extension->cpuset); > + } OK. Use extension space. > > if (ret != 0) > { > diff --git a/nptl/pthread_attr_destroy.c b/nptl/pthread_attr_destroy.c > index 21f8026a2c..b6a3cca657 100644 > --- a/nptl/pthread_attr_destroy.c > +++ b/nptl/pthread_attr_destroy.c > @@ -30,12 +30,16 @@ __pthread_attr_destroy (pthread_attr_t *attr) > iattr = (struct pthread_attr *) attr; > > #if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_1) > - /* In old struct pthread_attr, neither next nor cpuset are > - present. */ > + /* In old struct pthread_attr, the extension member is missing. */ > if (__builtin_expect ((iattr->flags & ATTR_FLAG_OLDATTR), 0) == 0) > #endif > - /* The affinity CPU set might be allocated dynamically. */ > - free (iattr->cpuset); > + { > + if (iattr->extension != NULL) > + { > + free (iattr->extension->cpuset); > + free (iattr->extension); OK. > + } > + } > > return 0; > } > diff --git a/nptl/pthread_attr_extension.c b/nptl/pthread_attr_extension.c > new file mode 100644 > index 0000000000..e8521b1556 > --- /dev/null > +++ b/nptl/pthread_attr_extension.c > @@ -0,0 +1,32 @@ > +/* Allocate the extension space for strucht pthread_attr. Typo: s/strucht/struct/g > + Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <pthreadP.h> > +#include <stdlib.h> > + > +int > + __pthread_attr_extension (struct pthread_attr *attr) > +{ > + if (attr->extension != NULL) > + return 0; > + attr->extension = calloc (sizeof (*attr->extension), 1); > + if (attr->extension == NULL) > + return errno; > + return 0; > +} OK. Dynamically allocate space for the extension via calloc. > diff --git a/nptl/pthread_attr_getaffinity.c b/nptl/pthread_attr_getaffinity.c > index 212c1f7c0a..9483f69ddc 100644 > --- a/nptl/pthread_attr_getaffinity.c > +++ b/nptl/pthread_attr_getaffinity.c > @@ -33,22 +33,22 @@ __pthread_attr_getaffinity_new (const pthread_attr_t *attr, size_t cpusetsize, > > iattr = (const struct pthread_attr *) attr; > > - if (iattr->cpuset != NULL) > + if (iattr->extension != NULL && iattr->extension->cpuset != NULL) OK. > { > /* Check whether there are any bits set beyond the limits > the user requested. */ > - for (size_t cnt = cpusetsize; cnt < iattr->cpusetsize; ++cnt) > - if (((char *) iattr->cpuset)[cnt] != 0) > + for (size_t cnt = cpusetsize; cnt < iattr->extension->cpusetsize; ++cnt) > + if (((char *) iattr->extension->cpuset)[cnt] != 0) OK. > return EINVAL; > > /* Copy over the cpuset from the thread attribute object. Limit the copy > to the minimum of the source and destination sizes to prevent a buffer > overrun. If the destination is larger, fill the remaining space with > zeroes. */ > - void *p = mempcpy (cpuset, iattr->cpuset, > - MIN (iattr->cpusetsize, cpusetsize)); > - if (cpusetsize > iattr->cpusetsize) > - memset (p, '\0', cpusetsize - iattr->cpusetsize); > + void *p = mempcpy (cpuset, iattr->extension->cpuset, > + MIN (iattr->extension->cpusetsize, cpusetsize)); > + if (cpusetsize > iattr->extension->cpusetsize) > + memset (p, '\0', cpusetsize - iattr->extension->cpusetsize); OK. > } > else > /* We have no information. */ > diff --git a/nptl/pthread_attr_setaffinity.c b/nptl/pthread_attr_setaffinity.c > index a42ffd92f4..9f9a70dee0 100644 > --- a/nptl/pthread_attr_setaffinity.c > +++ b/nptl/pthread_attr_setaffinity.c > @@ -34,23 +34,30 @@ __pthread_attr_setaffinity_np (pthread_attr_t *attr, size_t cpusetsize, > > if (cpuset == NULL || cpusetsize == 0) > { > - free (iattr->cpuset); > - iattr->cpuset = NULL; > - iattr->cpusetsize = 0; > + if (iattr->extension != NULL) > + { > + free (iattr->extension->cpuset); > + iattr->extension->cpuset = NULL; > + iattr->extension->cpusetsize = 0; OK. > + } > } > else > { > - if (iattr->cpusetsize != cpusetsize) > + int ret = __pthread_attr_extension (iattr); > + if (ret != 0) > + return ret; > + > + if (iattr->extension->cpusetsize != cpusetsize) > { > - void *newp = (cpu_set_t *) realloc (iattr->cpuset, cpusetsize); > + void *newp = realloc (iattr->extension->cpuset, cpusetsize); > if (newp == NULL) > return ENOMEM; > > - iattr->cpuset = newp; > - iattr->cpusetsize = cpusetsize; > + iattr->extension->cpuset = newp; > + iattr->extension->cpusetsize = cpusetsize; > } > > - memcpy (iattr->cpuset, cpuset, cpusetsize); > + memcpy (iattr->extension->cpuset, cpuset, cpusetsize); OK. > } > > return 0; > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index 86fbeb5218..f6418eb5ed 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -884,7 +884,7 @@ __pthread_create_2_0 (pthread_t *newthread, const pthread_attr_t *attr, > new_attr.guardsize = ps; > new_attr.stackaddr = NULL; > new_attr.stacksize = 0; > - new_attr.cpuset = NULL; > + new_attr.extension = NULL; OK. > > /* We will pass this value on to the real implementation. */ > attr = (pthread_attr_t *) &new_attr; > diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h > index 6d06a76baf..ca57c315d4 100644 > --- a/sysdeps/nptl/internaltypes.h > +++ b/sysdeps/nptl/internaltypes.h > @@ -36,9 +36,10 @@ struct pthread_attr > /* Stack handling. */ > void *stackaddr; > size_t stacksize; > - /* Affinity map. */ > - cpu_set_t *cpuset; > - size_t cpusetsize; > + > + /* Allocated via a call to __pthread_attr_extension once needed. */ > + struct pthread_attr_extension *extension; > + void *unused; OK. Keep the same size, but shuffle the usage around to get an extension pointer and an unused pointer. > }; > > #define ATTR_FLAG_DETACHSTATE 0x0001 > @@ -57,6 +58,15 @@ union pthread_attr_transparent > struct pthread_attr internal; > }; > > +/* Extension space for pthread attributes. Referenced by the > + extension member of struct pthread_attr. */ > +struct pthread_attr_extension > +{ > + /* Affinity map. */ > + cpu_set_t *cpuset; > + size_t cpusetsize; OK. At worst an applicaiton that is copying the attribute around is going to just get a copied pointer to the same affinity map. Such an application was not correct anyway. > +}; > + > /* Mutex attribute data structure. */ > struct pthread_mutexattr > { > diff --git a/sysdeps/unix/sysv/linux/createthread.c b/sysdeps/unix/sysv/linux/createthread.c > index 21f9d24f2d..6588893ba5 100644 > --- a/sysdeps/unix/sysv/linux/createthread.c > +++ b/sysdeps/unix/sysv/linux/createthread.c > @@ -52,8 +52,10 @@ create_thread (struct pthread *pd, const struct pthread_attr *attr, > /* Determine whether the newly created threads has to be started > stopped since we have to set the scheduling parameters or set the > affinity. */ > + bool need_setaffinity = (attr != NULL && attr->extension != NULL > + && attr->extension->cpuset != 0); OK. > if (attr != NULL > - && (__glibc_unlikely (attr->cpuset != NULL) > + && (__glibc_unlikely (need_setaffinity) > || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))) > *stopped_start = true; > > @@ -113,12 +115,13 @@ create_thread (struct pthread *pd, const struct pthread_attr *attr, > int res; > > /* Set the affinity mask if necessary. */ > - if (attr->cpuset != NULL) > + if (need_setaffinity) > { > assert (*stopped_start); > > res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, > - attr->cpusetsize, attr->cpuset); > + attr->extension->cpusetsize, > + attr->extension->cpuset); OK. > > if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) > err_out: >
diff --git a/nptl/Makefile b/nptl/Makefile index e5686b20ac..d6f12d0371 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -41,6 +41,7 @@ routines = \ pthread_atfork \ pthread_attr_copy \ pthread_attr_destroy \ + pthread_attr_extension \ pthread_attr_getdetachstate \ pthread_attr_getinheritsched \ pthread_attr_getschedparam \ diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index ed30b72923..7b3153594e 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -578,6 +578,12 @@ extern void __shm_directory_freeres (void) attribute_hidden; extern void __wait_lookup_done (void) attribute_hidden; +/* Allocates the extension space for ATTR. Returns an error code on + memory allocation failure, zero on success. If ATTR already has an + extension space, this function does nothing. */ +int __pthread_attr_extension (struct pthread_attr *attr) attribute_hidden + __attribute_warn_unused_result__; + #ifdef SHARED # define PTHREAD_STATIC_FN_REQUIRE(name) #else diff --git a/nptl/pthread_attr_copy.c b/nptl/pthread_attr_copy.c index 77a1a43eeb..eb29557505 100644 --- a/nptl/pthread_attr_copy.c +++ b/nptl/pthread_attr_copy.c @@ -29,18 +29,20 @@ __pthread_attr_copy (pthread_attr_t *target, const pthread_attr_t *source) temp.external = *source; /* Force new allocation. This function has full ownership of temp. */ - temp.internal.cpuset = NULL; - temp.internal.cpusetsize = 0; + temp.internal.extension = NULL; int ret = 0; struct pthread_attr *isource = (struct pthread_attr *) source; - /* Propagate affinity mask information. */ - if (isource->cpusetsize > 0) - ret = __pthread_attr_setaffinity_np (&temp.external, - isource->cpusetsize, - isource->cpuset); + if (isource->extension != NULL) + { + /* Propagate affinity mask information. */ + if (isource->extension->cpusetsize > 0) + ret = __pthread_attr_setaffinity_np (&temp.external, + isource->extension->cpusetsize, + isource->extension->cpuset); + } if (ret != 0) { diff --git a/nptl/pthread_attr_destroy.c b/nptl/pthread_attr_destroy.c index 21f8026a2c..b6a3cca657 100644 --- a/nptl/pthread_attr_destroy.c +++ b/nptl/pthread_attr_destroy.c @@ -30,12 +30,16 @@ __pthread_attr_destroy (pthread_attr_t *attr) iattr = (struct pthread_attr *) attr; #if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_1) - /* In old struct pthread_attr, neither next nor cpuset are - present. */ + /* In old struct pthread_attr, the extension member is missing. */ if (__builtin_expect ((iattr->flags & ATTR_FLAG_OLDATTR), 0) == 0) #endif - /* The affinity CPU set might be allocated dynamically. */ - free (iattr->cpuset); + { + if (iattr->extension != NULL) + { + free (iattr->extension->cpuset); + free (iattr->extension); + } + } return 0; } diff --git a/nptl/pthread_attr_extension.c b/nptl/pthread_attr_extension.c new file mode 100644 index 0000000000..e8521b1556 --- /dev/null +++ b/nptl/pthread_attr_extension.c @@ -0,0 +1,32 @@ +/* Allocate the extension space for strucht pthread_attr. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <pthreadP.h> +#include <stdlib.h> + +int + __pthread_attr_extension (struct pthread_attr *attr) +{ + if (attr->extension != NULL) + return 0; + attr->extension = calloc (sizeof (*attr->extension), 1); + if (attr->extension == NULL) + return errno; + return 0; +} diff --git a/nptl/pthread_attr_getaffinity.c b/nptl/pthread_attr_getaffinity.c index 212c1f7c0a..9483f69ddc 100644 --- a/nptl/pthread_attr_getaffinity.c +++ b/nptl/pthread_attr_getaffinity.c @@ -33,22 +33,22 @@ __pthread_attr_getaffinity_new (const pthread_attr_t *attr, size_t cpusetsize, iattr = (const struct pthread_attr *) attr; - if (iattr->cpuset != NULL) + if (iattr->extension != NULL && iattr->extension->cpuset != NULL) { /* Check whether there are any bits set beyond the limits the user requested. */ - for (size_t cnt = cpusetsize; cnt < iattr->cpusetsize; ++cnt) - if (((char *) iattr->cpuset)[cnt] != 0) + for (size_t cnt = cpusetsize; cnt < iattr->extension->cpusetsize; ++cnt) + if (((char *) iattr->extension->cpuset)[cnt] != 0) return EINVAL; /* Copy over the cpuset from the thread attribute object. Limit the copy to the minimum of the source and destination sizes to prevent a buffer overrun. If the destination is larger, fill the remaining space with zeroes. */ - void *p = mempcpy (cpuset, iattr->cpuset, - MIN (iattr->cpusetsize, cpusetsize)); - if (cpusetsize > iattr->cpusetsize) - memset (p, '\0', cpusetsize - iattr->cpusetsize); + void *p = mempcpy (cpuset, iattr->extension->cpuset, + MIN (iattr->extension->cpusetsize, cpusetsize)); + if (cpusetsize > iattr->extension->cpusetsize) + memset (p, '\0', cpusetsize - iattr->extension->cpusetsize); } else /* We have no information. */ diff --git a/nptl/pthread_attr_setaffinity.c b/nptl/pthread_attr_setaffinity.c index a42ffd92f4..9f9a70dee0 100644 --- a/nptl/pthread_attr_setaffinity.c +++ b/nptl/pthread_attr_setaffinity.c @@ -34,23 +34,30 @@ __pthread_attr_setaffinity_np (pthread_attr_t *attr, size_t cpusetsize, if (cpuset == NULL || cpusetsize == 0) { - free (iattr->cpuset); - iattr->cpuset = NULL; - iattr->cpusetsize = 0; + if (iattr->extension != NULL) + { + free (iattr->extension->cpuset); + iattr->extension->cpuset = NULL; + iattr->extension->cpusetsize = 0; + } } else { - if (iattr->cpusetsize != cpusetsize) + int ret = __pthread_attr_extension (iattr); + if (ret != 0) + return ret; + + if (iattr->extension->cpusetsize != cpusetsize) { - void *newp = (cpu_set_t *) realloc (iattr->cpuset, cpusetsize); + void *newp = realloc (iattr->extension->cpuset, cpusetsize); if (newp == NULL) return ENOMEM; - iattr->cpuset = newp; - iattr->cpusetsize = cpusetsize; + iattr->extension->cpuset = newp; + iattr->extension->cpusetsize = cpusetsize; } - memcpy (iattr->cpuset, cpuset, cpusetsize); + memcpy (iattr->extension->cpuset, cpuset, cpusetsize); } return 0; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 86fbeb5218..f6418eb5ed 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -884,7 +884,7 @@ __pthread_create_2_0 (pthread_t *newthread, const pthread_attr_t *attr, new_attr.guardsize = ps; new_attr.stackaddr = NULL; new_attr.stacksize = 0; - new_attr.cpuset = NULL; + new_attr.extension = NULL; /* We will pass this value on to the real implementation. */ attr = (pthread_attr_t *) &new_attr; diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h index 6d06a76baf..ca57c315d4 100644 --- a/sysdeps/nptl/internaltypes.h +++ b/sysdeps/nptl/internaltypes.h @@ -36,9 +36,10 @@ struct pthread_attr /* Stack handling. */ void *stackaddr; size_t stacksize; - /* Affinity map. */ - cpu_set_t *cpuset; - size_t cpusetsize; + + /* Allocated via a call to __pthread_attr_extension once needed. */ + struct pthread_attr_extension *extension; + void *unused; }; #define ATTR_FLAG_DETACHSTATE 0x0001 @@ -57,6 +58,15 @@ union pthread_attr_transparent struct pthread_attr internal; }; +/* Extension space for pthread attributes. Referenced by the + extension member of struct pthread_attr. */ +struct pthread_attr_extension +{ + /* Affinity map. */ + cpu_set_t *cpuset; + size_t cpusetsize; +}; + /* Mutex attribute data structure. */ struct pthread_mutexattr { diff --git a/sysdeps/unix/sysv/linux/createthread.c b/sysdeps/unix/sysv/linux/createthread.c index 21f9d24f2d..6588893ba5 100644 --- a/sysdeps/unix/sysv/linux/createthread.c +++ b/sysdeps/unix/sysv/linux/createthread.c @@ -52,8 +52,10 @@ create_thread (struct pthread *pd, const struct pthread_attr *attr, /* Determine whether the newly created threads has to be started stopped since we have to set the scheduling parameters or set the affinity. */ + bool need_setaffinity = (attr != NULL && attr->extension != NULL + && attr->extension->cpuset != 0); if (attr != NULL - && (__glibc_unlikely (attr->cpuset != NULL) + && (__glibc_unlikely (need_setaffinity) || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))) *stopped_start = true; @@ -113,12 +115,13 @@ create_thread (struct pthread *pd, const struct pthread_attr *attr, int res; /* Set the affinity mask if necessary. */ - if (attr->cpuset != NULL) + if (need_setaffinity) { assert (*stopped_start); res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, - attr->cpusetsize, attr->cpuset); + attr->extension->cpusetsize, + attr->extension->cpuset); if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) err_out: