[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140801210846.GA22308@port70.net>
Date: Fri, 1 Aug 2014 23:08:46 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: working C11 thread implementation
* Jens Gustedt <jens.gustedt@...ia.fr> [2014-08-01 11:55:31 +0200]:
> for anybody interested I attach a first working version, that is
> mildly tested with a relatively big private application that I have
> and that uses C11 threads quite intensively.
nice
> The choices that affect the ABI that I mentioned are values of some
> constants and the sizes of the types that are introduced. These may
> change depending on glibc's choices and also according to future
> developments.
>
> For the choices of the constants, for this version they are such that
> most wrapper calls result in being tail calls. I verified this by
> looking into the assembler that is produced. As they are now, most of
> these tail functions could also be just provided as weak aliases. We
> chose to implement them as functions such that in case of change of
> the constants we only have to recompile and no other maintenance is
> required.
>
i'd assume the constants to be right and fix up the
code only in case of later breakage
> + /* The following list of 9 integer constants makes up for the binary
> + compatibility of this C thread implementation. You must never
> + link code against versions of the C library that do not agree
> + upon these ABI parameters.
nice
> +int cnd_signal(cnd_t * cnd) {
> + /* In the best of all worlds this is a tail call. */
> + int ret = __pthread_cond_signal(cnd);
> + if (thrd_success)
> + return ret ? thrd_error : thrd_success;
> + return ret;
> +}
this is a bit weird
i think it's better to just assume thrd_success==0
and static assert it somewhere as a reminder
> +int mtx_init(mtx_t * m, int type)
> +{
> + *m = (pthread_mutex_t) {
> + ._m_type = ((type&mtx_recursive) ? PTHREAD_MUTEX_RECURSIVE : 0),
> + };
> + return 0;
> +}
return thrd_success
> +/* This is needed because not all arch have __pthread_self as a static
> + symbol. */
> pthread_t pthread_self()
> {
> return __pthread_self();
...
> +/* This is needed because not all arch have __pthread_self as a static
> + symbol. */
> +pthread_t thrd_current()
> +{
> + return __pthread_self();
> +}
i dont understand these comments
you mean that they could be weak aliases otherwise?
> +int thrd_join(thrd_t t, int *res)
> +{
> + int tmp;
> + while ((tmp = t->tid)) __timedwait(&t->tid, tmp, 0, 0, 0, 0, 0);
> + if (res) *res = (int)(intptr_t)t->result;
> + if (t->map_base) munmap(t->map_base, t->map_size);
> + return 0;
> +}
thrd_success
> +int tss_set(tss_t k, void *x)
> +{
> + struct pthread *self = __pthread_self();
> + /* Avoid unnecessary COW */
> + if (self->tsd[k] != x) {
> + self->tsd[k] = x;
> + self->tsd_used = 1;
> + }
> + return 0;
> +}
thrd_success
> +/* Roughly __syscall already returns the right thing: 0 if all went
> + well or a negative error indication, otherwise. Unfortunately the C
> + standard foresees the special case of an interrupted call and fixes
> + that error return to -1 (instead of introducing EINTR). */
> +int thrd_sleep(const struct timespec *req, struct timespec *rem)
> +{
> + int ret = __syscall(SYS_nanosleep, req, rem);
> + // compile time comparison, constant propagated
> + if (EINTR != 1) {
> + switch (ret) {
> + case -EINTR: return -1;
> + case -1: return -EINTR;
> + }
> + }
> + // potentially a tail call
> + return ret;
> +}
"The thrd_sleep function returns zero if the requested
time has elapsed, -1 if it has been interrupted by a
signal, or a negative value if it fails."
this is confusing
(either should not have the name thrd_ or follow
the error enum convention of other thrd_ functions)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
