This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add and use new glibc-internal futex API.
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Torvald Riegel <triegel at redhat dot com>, GLIBC Devel <libc-alpha at sourceware dot org>
- Cc: Roland McGrath <roland at hack dot frob dot com>
- Date: Tue, 09 Jun 2015 11:01:12 -0300
- Subject: Re: [PATCH] Add and use new glibc-internal futex API.
- Authentication-results: sourceware.org; auth=none
- References: <1433796158 dot 21461 dot 438 dot camel at triegel dot csb>
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 =)