[16/19] nptl: Make pthread_attr_t dynamically extensible
Commit Message
This introduces the function __pthread_attr_extension to allocate the
extension space, which is freed by pthread_attr_destroy.
---
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
Comments
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:
>
@@ -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 \
@@ -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
@@ -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)
{
@@ -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;
}
new file mode 100644
@@ -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;
+}
@@ -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. */
@@ -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;
@@ -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;
@@ -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
{
@@ -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: