This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] New condvar implementation that provides stronger ordering guarantees.
- From: Florian Weimer <fweimer at redhat dot com>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: GLIBC Devel <libc-alpha at sourceware dot org>, "Carlos O'Donell" <carlos at redhat dot com>, David Miller <davem at davemloft dot net>, Darren Hart <dvhart at infradead dot org>
- Date: Tue, 14 Jun 2016 20:27:39 +0200
- Subject: Re: [PATCH] New condvar implementation that provides stronger ordering guarantees.
- Authentication-results: sourceware.org; auth=none
- References: <1464268895 dot 17104 dot 14 dot camel at localhost dot localdomain> <9ba4528e-6c48-f832-825a-bdc68c37eb47 at redhat dot com> <1465928107 dot 19633 dot 109 dot camel at localhost dot localdomain>
On 06/14/2016 08:15 PM, Torvald Riegel wrote:
Or alternatively, why is the s variable not reloaded?
It is. C11's compare_exchange updates the expected value if the it the
comparison failed (that's why we pass in the address of s).
Ahh. This explains it.
I haven't really reviewed this patch.
Thanks for taking a look nonetheless.
One small nit: GNU style does not
use a parenthesis at the end of a line, as in (among others):
+ uint64_t r = __condvar_fetch_add_64_relaxed (
+ return __condvar_load_64_relaxed (
What's the expected solution, parenthesis on the next line?
Exactly, like this:
return long_function name
(some_long expression_on_the_next_line_in_the_input_file);
There are some magic numbers 2/4/8, which might better use symbolic
constants.
I considered this, but kind of preferred the short numbers. If we
prefer constants with names, I'll change that.
I prefer them in the code I'm working on. But I understand that
preferences differ.
pthread_cond_common.c should be a header file (pthread_cond_common.h).
There are many cases of C files being included across all of glibc. Are
there precise rules for when to pick .h vs. .c? pthread_cond_common.c
doesn't have just declarations or small helper functions, so .c seemd to
be the right choice.
As far as I can tell, we use .c only if the file is *also* compiled as a
C source file. In such cases, there are some preprocessor macros you
can define to alter the behavior, before including the file. In other
cases, it's used to create test cases with different compiler/linker
flags (e.g., static linking). Or it's an override for the sysdeps
mechanism.
Thanks,
Florian