[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1407170939.8274.271.camel@eris.loria.fr>
Date: Mon, 04 Aug 2014 18:48:59 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: C threads, v3.0
Am Montag, den 04.08.2014, 10:50 -0400 schrieb Rich Felker:
> On Mon, Aug 04, 2014 at 11:30:03AM +0200, Jens Gustedt wrote:
> > I'd like to discuss two issues before going further. The first is of
> > minor concern. In many places of that code _m_lock uses a flag, namely
> > 0x40000000. I only found places where this bit is read, but not where
> > it is set. Did I overlook something or is this a leftover?
>
> It's set by the kernel when a thread dies while owning a robust mutex.
ah, ok, good to know, this was not obvious at all to me.
so I will leave this in, although C mutexes are not supposed to be
robust.
> > The second issue concerns tsd...
> > I don't think that this policy is ideal for C threads, but it could
> > perhaps be revised for pthreads, too. My idea is the
> > following. (version for C threads with minimal impact on pthreads)
> >
> > - don't assign a tsd in thrd_create
> >
> > - change pthread_getspecific, pthread_setspecific, tss_get, and
> > __pthread_tsd_run_dtors such that they check for nullness of
> > tsd. this is a trivial and non-expensive change.
> > pthread_setspecific may return ENOMEM or EINVAL in such cases. The
> > getters should just return 0. __pthread_tsd_run_dtors obviously
> > would just do nothing, then.
>
> EINVAL is definitely not permitted here since ENOMEM is required by
> POSIX for this case, if it happens.
My idea is that ENOMEM doesn't fit too well here, since it is not this
call to pthread_setspecific that is out of memory, but that the key is
an invalid key for this specific thread. (It would only happen to C
threads that use pthread_setspecific.)
> > - change tss_set, to mmap the table if it is absent and if it is
> > called with a non-null pointer argument. (I.e if it really has to
> > store something). C11 allows this function to fail, so we could
> > return thrd_error if mmap fails.
> >
> > - change thrd_exit to check for tsd and to unmap it at the end if
> > necessary
> >
> > For thrd_create one of the consequences would be that main hasn't to
> > be treated special with that respect. The additional mmap and unmap in
> > tss_set should only concern entire pages. This should only have a
> > minimal runtime overhead, but which only would occur for threads that
> > call tss_set with a non-null pointer to be stored.
> >
> > Please let me know what you think.
>
> This is not an acceptable implementation (at least from the standpoint
> of musl's design principles); it sacrifices fail-safe behavior
> (guaranteeing non-failure of an interface that's usually impossible to
> deal with failure from, and for which there's no fundamental reason it
> should be able to fail) to safe a tiny amount of memory.
I see. (Also it is not *only* to save memory, it is also to simplify
the implementation and to put the burden on the real user of a
feature.)
I am not completely convinced of your analysis. pthread_key_create and
pthread_setspecific have failure paths that are defined by
POSIX.
pthread_getspecific would never fail. It would always return a null
pointer, which would be the correct result since no value had ever
been stored for that thread.
The tss functions also clearly have failure paths in their definition.
> For static linking, I think it's completely acceptable to assume that,
> if tsd functions are linked, they're going to be used.
As I said, I think this is a feature that is rarely used anyhow. For
the code that actually links pthread_key_create, it is not at all
clear to me that pthread_setspecific is then used by a lot of
threads. I don't have stats for this, obviously, but the tss stuff
will even be used less.
> Unfortunately
> this isn't possible for dynamic linking, and I had considered an
> alternate implementation for dynamic linking, based on having the
> first call to pthread_key_create simulate loading of a shared library
> containing a TLS array of PTHREAD_KEYS_MAX pointers. This would make
> the success/failure detectable early at a time where there's
> inherently a possibility of failure (too many keys) rather than where
> failure is just a consequence of a bad implementation.
I am not sure what you want to imply by /bad/ implementation :)
Just to clear that out, the way that I propose would activate a
potential failure path if the process ran out of memory, correct. But
the real problem on failure here would be the lack of memory and not
the failure of the pthread_setspecific call itself. In the current
model the corresponding call to pthread_create or thrd_create would
have failed for ENOMEM.
> But the
> complexity of doing this, and the added unnecessary failure case
> (failure before PTHREAD_KEYS_MAX is hit, which really shouldn't
> happen), seemed unjustified for a 0.5-1k savings.
I completely agree that such faking of a shared library load would be
an overkill.
For C threads, there would also be an option to avoid messing around
with the pthread key system and keep that intact. We don't know what
POSIX will decide on how they should interact, anyhow.
We could implement tss separately. That's quite simple to do.
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)
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.
