hyc(a)symas.com writes:
>h.b.furuseth(a)usit.uio.no wrote:
>> It's called from pool_wrapper(). pool_wrapper() could go active so
>> there can be no pause while that context_reset() call is running.
>
> Oh, ok. This is pretty much a non-issue because threads in the pool stay
> live until slapd shuts down. The one exception is for a cn=config modify
> that decreases the size of the pool to below the number of currently
> active threads, in which case some number of threads will exit
> immediately. I don't see any potential for conflict here.
Eh? Yes, that's what I was talking about. But I'm not sure what you
mean with them exiting immediately - or how they could. Also they stay
around until there are no pending requests. A pause request could
arrive more or less at any time, I presume. (I don't know if there is a
pause while the max threads number is being modified.)
>>>>> How can a single thread ID be represented by multiple bit patterns?
>>>> A struct/union with padding bytes seems the least unlikely possibility.
>> (...)
>> So in that respect, my preference (not for RE23) is to cast to an
>> integer for the hash, and also to add pthread_<set/get>specific() so
>> thread_keys[] and its hash will only be a fallback solution.
>
> Casting to an int only works with simple types, the point of using the
> hash was to provide a solution that works for both simple and structured
> thread IDs.
Yes, except it's not guaranteed to work. Which is why I'm thinking it
may be better to fail to compile and provide a fix if/when someone
complains. We did have code which compared thread IDs with '==' for
quite some time. Do you remember if anyone complained, or if the
switch to using ldap_pvt_thread_equal() was a "just in case" fix?
However note that we do
#define ldap_int_thread_equal(a, b) ((a) == (b))
for all our supported thread implementations except pthreads, and
for pthreads we can test pthread_<set/get>specific() just fine
(as a replacement for thread_keys[]).
OTOH, '==' also works for pointers that are wider than any integer type
(and for floating-point thread IDs:-) so I guess I'm arguing against my
own suggestion on that one if we add pthread_<set/get>specific()
support.
> If you want to #ifdef the code so the two cases are handled
> differently, then just get rid of TID_HASH for the integral case.
No, sould still hash to avoid clustering.
> I'm still leary of the pthread TLS support,
Me too. But the old code was broken on real-life systems I have access
to, not just on theoretical ones. Still waiting to hear from Kurt if my
fix to the first tls.c change fixed it for FreeBSD.
> and making that change will
> require us to update all of the other thread package support, some of
> which I don't have access to build/test any more (e.g. SunOS4 LWP).
What do you mean? The TLS problem is to extract an unsigned long from a
ldap_pvt_thread_t. That needs no support in the thr_*.c files, only to
know what kind of type ldap_pvt_thread_t is. Once configure knows what
ldap_pvt_thread_t will be typedeffed to it can test if that's a pointer,
integer or something worse - and whether it's too wide for a long.
BTW I don't know if the unsigned long needs to be the thread ID or just
_some_ unique integer. A question to openssl-users(a)openssl.org got no
answer.
>>>>> The best fix may be to just use the address of the thread's stack as the
>>>>> thread ID. That's actually perfect for our purpose.
>> (...)
>> I'm not sure how to determine _which_ multiple of 1MB - start two
>> threads and compare their stack addresses, maybe? In any case, I
>> agree it sounds rather more machine-dependent than the current code.
>
> That's what the LDAP_PVT_THREAD_STACK_SIZE macro tells you. It's really
> not hard. If we were to disallow increasing the number of threads at
> runtime, we could simply allocate a block of thread stacks all at once,
> which would then allow our thread IDs to be simple integers from 0-N.
Hmm. I asked the wrong question, should have read some man pages first.
Still, thr_cthreads.c & thr_lwp.c don't use LDAP_PVT_THREAD_STACK_SIZE.
man pthread_attr_getstacksize says it sets the _minimum_ stack size, but
perhaps several M is large enough that one can expect it won't get
multiples of that.
--
Regards,
Hallvard