Re: [RFC] pthread support for FUTEX_WAIT_MULTIPLE

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 31/07/2019 05:14, Florian Weimer wrote:
> * Pierre-Loup A. Griffais:
> 
>> The gist of it is that it adds a new function,
>> pthread_mutex_timedlock_any(), that can take a list of mutexes. It
>> returns when one of the mutex has been locked (and tells you which
>> one), or if the timeout is met. We would use this to reduce
>> unnecessary wakeups and spinning in our thread pool synchronization
>> code, compared to the current eventfd hack we rely on.
> 
> This explains why the mutexes have to be contiguous in memory.  For
> other applications, this looks like an unnecessary restriction.
> 
>>  - I assume whichever name it might end up as should end with _np? Is
>> there any specific process to follow for this sort of non-standard
>> inclusion, other than just writing the code and documentation?
> 
> It seems unlikely that this is ever going to be standardized, so I think
> we'd need the _np suffix, yes.
> 
>>  - What is the standard way for an application to discover whether it
>> can use an entrypoint dependent on a certain Linux kernel version?
>> With our proposed use, we'd be fine running the function once at
>> startup to pick which path to chose, eg. pthread_mutex_lock_any( NULL,
>> 0, NULL, NULL ). If it returns 0, we'd enable the new path, otherwise
>> we'd fall back to eventfd(). I have a TODO in the code where we could
>> do that, but it's probably not the right way to do things.
> 
> I think you would have to probe on first use inside
> pthread_mutex_lock_any, using a dummy call.
> 
>>  - I assume the way I'm exposing it as a 2.2.5-versioned symbol for
>> local testing is wrong; what is the right way to do this?
> 
> This patch could be targeted at glibc 2.31, then you would have to use
> GLIBC_2.31.
> 
>>  - In my tree I have a placeholder test application that should be
>> replaced by a new mutex test. However, it would also be a good idea to
>> leverage other mutex tests to test this new thing, since it's a
>> superset of many other mutex calls. Could we make the normal mutex
>> test suite run a second time, with a macro wrapping the normal
>> pthread_lock with this implementation instead?
> 
> Due to the new ENOMEM error, the new function is not a strict superset.
> 
> I'm wondering if the current design is really the right one,
> particularly for thread pools.  The implementation performs multiple
> scans of the mutex lists, which look rather costly for large pools.
> That's probably unavoidable if the list is dynamic and potentially
> different for every call, but given the array-based interface, I don't
> think this is the intended use.  Something that use pre-registration of
> the participating futexes could avoid that.  I also find it difficult to
> believe that this approach beats something that involves queues, where a
> worker thread that becomes available identifies itself directly to a
> submitter thread, or can directly consume the submitted work item.

Also, I am trying to understand exactly what the new futex operation tries
to optimize.  The kernel patch adds some information about a wine specific
implementation, but it not clear what exactly was tested on "Wine fsync 
implementation and executing multi-threaded applications" neither by
"example code that exercises the FUTEX_WAIT_MULTIPLE path on a
multi-threaded".

Could you provide some actual source code for your 'thread pool synchronization 
code' or at least point to the wine code that you are aiming to optimize?