This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] handle sem_t with ILP32 and __HAVE_64B_ATOMICS
- From: Torvald Riegel <triegel at redhat dot com>
- To: Chris Metcalf <cmetcalf at ezchip dot com>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>, Andreas Schwab <schwab at suse dot de>, "Carlos O'Donell" <carlos at redhat dot com>, GNU C Library <libc-alpha at sourceware dot org>, David Miller <davem at davemloft dot net>, Richard Henderson <rth at redhat dot com>, Mike Frysinger <vapier at gentoo dot org>, "Joseph S. Myers" <joseph at codesourcery dot com>, Kaz Kojima <kkojima at rr dot iij4u dot or dot jp>, Thomas Schwinge <thomas at codesourcery dot com>, Marcus Shawcroft <marcus dot shawcroft at linaro dot org>, Chung-Lin Tang <chunglin_tang at mentor dot com>, Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- Date: Wed, 28 Jan 2015 10:04:11 +0100
- Subject: Re: [PATCH v2] handle sem_t with ILP32 and __HAVE_64B_ATOMICS
- Authentication-results: sourceware.org; auth=none
- References: <54C2BDD7 dot 7000304 at redhat dot com> <54C3B6D5 dot 3090308 at ezchip dot com> <1422119595 dot 29655 dot 42 dot camel at triegel dot csb> <54C5094A dot 8060300 at ezchip dot com> <54C51D94 dot 6030007 at ezchip dot com> <1422276280 dot 29655 dot 91 dot camel at triegel dot csb> <54C6A1DD dot 4020004 at ezchip dot com> <1422305739 dot 29655 dot 144 dot camel at triegel dot csb> <87a915b170 dot fsf at igel dot home> <54C6CF4B dot 1020600 at ezchip dot com> <CAMe9rOqAx=6Jjjj4V7Rb3o+jfz+ibOZxYy1x3AWfRrLjSBfauA at mail dot gmail dot com> <54C8009A dot 2000709 at ezchip dot com>
On Tue, 2015-01-27 at 16:18 -0500, Chris Metcalf wrote:
> On 1/27/2015 6:40 AM, H.J. Lu wrote:
> > On Mon, Jan 26, 2015 at 3:35 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> >> --- a/nptl/sem_open.c
> >> +++ b/nptl/sem_open.c
> >> @@ -186,25 +186,28 @@ sem_open (const char *name, int oflag, ...)
> >> return SEM_FAILED;
> >> }
> >>
> >> - /* Create the initial file content. */
> >> - union
> >> - {
> >> - sem_t initsem;
> >> - struct new_sem newsem;
> >> - } sem;
> >> + /* Create the initial file content. We force the alignment of
> >> + the sem_t to match the alignment of struct new_sem since we
> >> + will copy this stack structure to a file and then mmap it,
> >> + so we must ensure it is aligned to zero here so that the
> >> + behavior of to_new_sem () is the same as when we later mmap
> >> + it into memory and have it be page-aligned. */
> >> + sem_t sem __attribute__ ((aligned (__alignof__ (struct new_sem))));;
> >> + struct new_sem *newsem = to_new_sem (&sem);
> >> +
> >> + /* Initialize the unused parts of sem_t to zero.
> >> + The compiler will notice most of this memset is dead based on
> >> + the assignments through the struct new_sem pointer. */
> >> + memset (&sem, '\0', sizeof (sem_t));
> > Why is this change needed? Since union sem has the largest alignment of
> > sem_t and struct new_sem, sem is properly aligned here.
>
> It is not needed. Torvald's claim was that it was more stylistically
> consistent to use to_new_sem() in all cases, even here. I am personally
> on the fence about this; I think the new code and old code are both
> plausible ways to represent the issue with the required alignment.
>
> If Torvald prefers the new code and you prefer the old code, perhaps
> we will need to ask for a tie-breaker vote. :-)
>
I thing using to_new_sem in all cases is cleaner, but my comment on the
union wasn't quite right. I agree the union should give the proper
alignment, because it includes struct new_sem directly, so will pick up
the uint64_t alignment of it.