Re: (ITS#4943) tpool.c pause vs. finish

[h.b.furuseth@usit.uio.no]

hyc@symas.com writes:
>h.b.furuseth@usit.uio.no wrote:
>> I wonder if we've been waiting for each other...  Anyway, I've committed
>> some pending fixes now that HEAD passes all tests for me.  I've some
>> more cleanup, but if we are close to a freeze I don't know if that means
>> I should rush it in or wait until afterwards.
>
> Cleanup as in tidying the code, or as in fixing functionality?

One cleanup (move thread start before queuing task in pool_submit(),
one mini functionality: Make thread counts int instead of long, since
pool_query() wants that.  Oh, and make max_pending 0 or greater, since
negative numbers confuse pool_query().

>> Another time when tls.c code loses information is if pointers are wider
>> than long, which does happen.  (Don't know how it is nowadays, but I
>> remember plenty of postings about it some years ago.)
>
> Probably the OpenSSL API should have used a void * here. Too bad.

Yup.  I've seen mention it is coming in future version.

> OK, suppose we use a pthread thread-specific key here. What are you
> proposing for the unique value?

An unsigned long counter, maintained by libldap_r.  If the thread-local
variable is zero (thus uninitialized), increment the counter and grab
its value.

>>>>> 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.
>>
>> IIRC disallowing that would fix some other tpool problem too, but I
>> don't remember what.
>
> I'm not really keen on that since I like having the ability to both increase 
> and decrease the setting dynamically.

Me too, the only pro is it makes implementation simpler.

>> Which led me to notice that the setstacksize call now _decreases_ the
>> stack size on some hosts.  I don't suppose that was intentional, so I
>> guess it should be
>>     size_t cursize;
>>     if( pthread_attr_getstacksize( &attr, &cursize )
>>         || cursize < LDAP_PVT_THREAD_STACK_SIZE )
>>         pthread_attr_setstacksize( &attr, LDAP_PVT_THREAD_STACK_SIZE );
>> in thr_posix and I don't know what in other thr_* files.  A RE24 change
>> I guess, since reduces the max number of threads before slapd gets into
>> trouble.
>
> I don't consider this a bug. If someone wants a larger thread stack
> they can recompile to get it. Or perhaps we should make it a tunable
> variable and add a config keyword for it.

OK

-- 
Regards,
Hallvard