From patchwork Mon Jan 26 23:35:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Metcalf X-Patchwork-Id: 4823 Received: (qmail 6238 invoked by alias); 26 Jan 2015 23:36:33 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 6206 invoked by uid 89); 26 Jan 2015 23:36:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: emea01-db3-obe.outbound.protection.outlook.com Message-ID: <54C6CF4B.1020600@ezchip.com> Date: Mon, 26 Jan 2015 18:35:39 -0500 From: Chris Metcalf User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Andreas Schwab , Torvald Riegel CC: Carlos O'Donell , GNU C Library , "H.J. Lu" , David Miller , Richard Henderson , Mike Frysinger , "Joseph S. Myers" , Kaz Kojima , Thomas Schwinge , Marcus Shawcroft , Chung-Lin Tang , Adhemerval Zanella , Andreas Krebbel Subject: [PATCH v2] handle sem_t with ILP32 and __HAVE_64B_ATOMICS References: <54C2BDD7.7000304@redhat.com> <54C3B6D5.3090308@ezchip.com> <1422119595.29655.42.camel@triegel.csb> <54C5094A.8060300@ezchip.com> <54C51D94.6030007@ezchip.com> <1422276280.29655.91.camel@triegel.csb> <54C6A1DD.4020004@ezchip.com> <1422305739.29655.144.camel@triegel.csb> <87a915b170.fsf@igel.home> In-Reply-To: <87a915b170.fsf@igel.home> X-ClientProxiedBy: BY1PR0201CA0026.namprd02.prod.outlook.com (25.160.191.164) To DB4PR02MB0543.eurprd02.prod.outlook.com (10.141.45.16) Authentication-Results: linux.vnet.ibm.com; dkim=none (message not signed) header.d=none;linux.vnet.ibm.com; dmarc=none action=none header.from=ezchip.com; X-DmarcAction-Test: None X-Microsoft-Antispam: UriScan:; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:(3005004);SRVR:DB4PR02MB0543; X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004); SRVR:DB4PR02MB0543; X-Forefront-PRVS: 0468FE4A2B X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6049001)(6009001)(377424004)(87976001)(86362001)(83506001)(15975445007)(575784001)(2950100001)(66066001)(42186005)(19580395003)(122386002)(19580405001)(80316001)(47776003)(50466002)(77156002)(62966003)(40100003)(64126003)(50986999)(117156001)(229853001)(59896002)(33656002)(76176999)(65816999)(54356999)(77096005)(92566002)(87266999)(36756003)(93886004)(65806001)(65956001)(46102003)(23746002)(21314002)(18886065003); DIR:OUT; SFP:1101; SCL:1; SRVR:DB4PR02MB0543; H:[192.168.1.160]; FPR:; SPF:None; MLV:nov; PTR:InfoNoRecords; LANG:en; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:; SRVR:DB4PR02MB0543; X-OriginatorOrg: ezchip.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jan 2015 23:35:59.7264 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB4PR02MB0543 This version reflects Torvald's recent comments. 2015-01-25 Chris Metcalf * sysdeps/nptl/internaltypes.h (to_new_sem): Define. Provides new behavior for [__HAVE_64B_ATOMICS && !defined (_LP64)]. * nptl/sem_getvalue.c (__new_sem_getvalue): Use to_new_sem. * nptl/sem_init.c (__new_sem_init): Likewise. * nptl/sem_open.c (sem_open): Likewise. * nptl/sem_post.c (__new_sem_post): Likewise. * nptl/sem_timedwait.c (sem_timedwait): Likewise. * nptl/sem_wait.c (__new_sem_wait): Likewise. (__new_sem_trywait): Likewise. diff --git a/nptl/sem_getvalue.c b/nptl/sem_getvalue.c index 1432cc7..08a0789 100644 --- a/nptl/sem_getvalue.c +++ b/nptl/sem_getvalue.c @@ -25,7 +25,7 @@ int __new_sem_getvalue (sem_t *sem, int *sval) { - struct new_sem *isem = (struct new_sem *) sem; + struct new_sem *isem = to_new_sem (sem); /* XXX Check for valid SEM parameter. */ /* FIXME This uses relaxed MO, even though POSIX specifies that this function diff --git a/nptl/sem_init.c b/nptl/sem_init.c index 575b661..aaa6af8 100644 --- a/nptl/sem_init.c +++ b/nptl/sem_init.c @@ -50,7 +50,7 @@ __new_sem_init (sem_t *sem, int pshared, unsigned int value) } /* Map to the internal type. */ - struct new_sem *isem = (struct new_sem *) sem; + struct new_sem *isem = to_new_sem (sem); /* Use the values the caller provided. */ #if __HAVE_64B_ATOMICS diff --git a/nptl/sem_open.c b/nptl/sem_open.c index bfd2dea..202769f 100644 --- 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)); #if __HAVE_64B_ATOMICS - sem.newsem.data = value; + newsem->data = value; #else - sem.newsem.value = value << SEM_VALUE_SHIFT; - sem.newsem.nwaiters = 0; + newsem->value = value << SEM_VALUE_SHIFT; + newsem->nwaiters = 0; #endif /* This always is a shared semaphore. */ - sem.newsem.private = LLL_SHARED; - - /* Initialize the remaining bytes as well. */ - memset ((char *) &sem.initsem + sizeof (struct new_sem), '\0', - sizeof (sem_t) - sizeof (struct new_sem)); + newsem->private = LLL_SHARED; tmpfname = __alloca (shm_dirlen + sizeof SEM_SHM_PREFIX + 6); char *xxxxxx = __mempcpy (tmpfname, shm_dir, shm_dirlen); @@ -242,7 +245,7 @@ sem_open (const char *name, int oflag, ...) break; } - if (TEMP_FAILURE_RETRY (__libc_write (fd, &sem.initsem, sizeof (sem_t))) + if (TEMP_FAILURE_RETRY (__libc_write (fd, &sem, sizeof (sem_t))) == sizeof (sem_t) /* Map the sem_t structure from the file. */ && (result = (sem_t *) mmap (NULL, sizeof (sem_t), diff --git a/nptl/sem_post.c b/nptl/sem_post.c index 6e495ed..71818ea 100644 --- a/nptl/sem_post.c +++ b/nptl/sem_post.c @@ -56,7 +56,7 @@ futex_wake (unsigned int* futex, int processes_to_wake, int private) int __new_sem_post (sem_t *sem) { - struct new_sem *isem = (struct new_sem *) sem; + struct new_sem *isem = to_new_sem (sem); int private = isem->private; #if __HAVE_64B_ATOMICS diff --git a/nptl/sem_timedwait.c b/nptl/sem_timedwait.c index 042b0ac..5e650e0 100644 --- a/nptl/sem_timedwait.c +++ b/nptl/sem_timedwait.c @@ -30,8 +30,8 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime) return -1; } - if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0) + if (__new_sem_wait_fast (to_new_sem (sem), 0) == 0) return 0; else - return __new_sem_wait_slow((struct new_sem *) sem, abstime); + return __new_sem_wait_slow(to_new_sem (sem), abstime); } diff --git a/nptl/sem_wait.c b/nptl/sem_wait.c index c1fd10c..73759f1 100644 --- a/nptl/sem_wait.c +++ b/nptl/sem_wait.c @@ -22,10 +22,10 @@ int __new_sem_wait (sem_t *sem) { - if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0) + if (__new_sem_wait_fast (to_new_sem (sem), 0) == 0) return 0; else - return __new_sem_wait_slow((struct new_sem *) sem, NULL); + return __new_sem_wait_slow(to_new_sem (sem), NULL); } versioned_symbol (libpthread, __new_sem_wait, sem_wait, GLIBC_2_1); @@ -65,7 +65,7 @@ __new_sem_trywait (sem_t *sem) { /* We must not fail spuriously, so require a definitive result even if this may lead to a long execution time. */ - if (__new_sem_wait_fast ((struct new_sem *) sem, 1) == 0) + if (__new_sem_wait_fast (to_new_sem (sem), 1) == 0) return 0; __set_errno (EAGAIN); return -1; diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h index 8f5cfa4..8dd4018 100644 --- a/sysdeps/nptl/internaltypes.h +++ b/sysdeps/nptl/internaltypes.h @@ -168,6 +168,22 @@ struct new_sem #endif }; +/* The sem_t alignment is typically just 4 bytes for ILP32, whereas + new_sem is 8 bytes with __HAVE_64B_ATOMICS. For better atomic + performance on platforms that tolerate unaligned atomics (and to + make it work at all on platforms that don't), round up the new_sem + pointer within the sem_t object to an 8-byte boundary. + + Note that in this case, the useful part of the new_sem structure + (up to the "pad" variable) is only 12 bytes, and sem_t is always + at least 16 bytes, so moving the start of new_sem to an offset + of 4 bytes within sem_t is okay. We must not read or write the + "pad" field within new_sem to maintain correctness. */ +#define to_new_sem(s) \ + ((__alignof__ (sem_t) == 4 && __alignof__ (struct new_sem) == 8) ? \ + (struct new_sem *) (((uintptr_t) (s) + 4) & -8) : \ + (struct new_sem *) (s)) + struct old_sem { unsigned int value;