This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add and use new glibc-internal futex API.


Hi

On 08-06-2015 17:42, Torvald Riegel wrote:
> This adds new functions for futex operations, starting with wait,
> abstimed_wait, reltimed_wait, wake.  They add documentation and error
> checking according to the current draft of the Linux kernel futex
> manpage.
> 
> Waiting with absolute or relative timeouts is split into separate
> functions.  This allows for removing a few cases of code duplication in
> pthreads code, which uses absolute timeouts; also, it allows us to put
> platform-specific code to go from an absolute to a relative timeout into
> the platform-specific futex abstractions.  The latter is done by adding
> lll_futex_abstimed_wait.  I expect that we will refactor this later on,
> depending on how we do the lll_ parts.
> 
> Futex operations that can be canceled are also split out into separate
> functions suffixed by "_cancelable".
> 
> This is a revision of
> https://sourceware.org/ml/libc-alpha/2015-01/msg00215.html
> 
> I have transformed all the lll_futex_* uses that I'm aware of except the
> following:
> * Anything related to lowlevellock or mutexes.
> * sparc-specific files: I'll send a follow-up patch so this can be
> reviewed and tested separately.
> * pthread condvar: I'll send a follow-up patch on top of my revised
> condvar implementation.
> * All tls.h files: Siddhesh wants to look at this area in more detail,
> so I'll work with him to see how to best move this to the new API.
> 
> Interacting with futex words requires atomic accesses, which isn't done
> by most of glibc's current futex callers.  I did not fix these in this
> patch to keep the patch easier to review: Using the new futex API is in
> most cases a pretty mechanical change, which I didn't want to obfuscate
> by lots of other changes to atomics.  The core motivation behind this
> patch is to add error handling and improve the internal futex API, not
> to change any synchronization.
> Nonetheless, using atomics where they are needed is on my list of things
> to do, so don't worry :)
> Specifically, this is already done in the my new condvar implementation
> and in the new semaphore.  It will get done for rwlock in the new
> implementation I'm working on.  I also plan to update the barrier
> implementation.  For TLS, this is something Siddhesh has on his radar,
> AFAIK.  Adhemerval is working on a new cancellation scheme.
> 
> I kept the old semaphore code unchanged for now because I don't have a
> testing setup for this ready.
> 
> Adhemerval, is this API compatible with the new cancellation scheme you
> are working on?

Yes I think so, some comments below.

> +/* Like futex_wait but cancelable.  */
> +static __always_inline int
> +futex_wait_cancelable (unsigned int *futex_word, unsigned int expected,
> +		       int private)
> +{
> +  int oldtype;
> +  oldtype = __pthread_enable_asynccancel ();
> +  int err = lll_futex_timed_wait (futex_word, expected, NULL, private);
> +  __pthread_disable_asynccancel (oldtype);
> +  switch (err)

I would prefer to follow c6bb095eb and just create a lll_futex_timed_wait_cancel
that call SYSCALL_CANCEL instead (since the idea of my patch is just to remove
all the *_{enable,disable}_asynccancel calls). However I can also add this change
on my upcoming patches.

The patch looks good, nice to see some glibc refactor =)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]