Message ID | fcd9fac8-f689-4f38-a53c-c2d09dd5230f@linaro.org |
---|---|
State | Dropped |
Headers |
Received: (qmail 33204 invoked by alias); 5 Jul 2017 13:48:45 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 26347 invoked by uid 89); 5 Jul 2017 13:48:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-16.4 required=5.0 tests=BAYES_00, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=validated, Hx-languages-length:2344 X-HELO: mail-qk0-f182.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Klkx/GKEOlqe43Rk8jAC150bTDBdVtCwTgS/OC3ALEo=; b=ayPOi2YlpKwzv92gybIMTulngnsI4t5vn49+J1ALgdsEuTOVXypgopjgMNryJK6p8B TQXA61if+dm8ifa3BOmo71tbrPClN5qexIxJ3ZM56tsfzAbQyardET+fqScuTGCpAxrb N4tGCCcCyGgw75FSIMxkAs5NcQQcwgxNeMgS7N1TBlCOPTWhnDM+Y3ec7gPKJLsTR/En wlI8gLe9Q9F7nAuqRupnBsluqrefAlJWPn2FqAfm5qS/q8GCQBNrPYOaT1DEfq7DPZeq rM7xqRCZZf6fJZrCi2J9sVAArUFTJSWOontuSU0rEFJzuFKFv0L7IrYZVBoNCMJZkVuz cIuA== X-Gm-Message-State: AIVw112WRdrM43fRWKOMz7WEy77QcR7E1JS0lwnIDHa+G+nZ0cRtmL4o a8JroA43jQQ0okS6myjxPg== X-Received: by 10.55.209.139 with SMTP id o11mr5944946qkl.144.1499262489515; Wed, 05 Jul 2017 06:48:09 -0700 (PDT) Subject: Re: 2.26 release blockers? To: Joseph Myers <joseph@codesourcery.com>, Siddhesh Poyarekar <siddhesh@gotplt.org> Cc: libc-alpha@sourceware.org References: <6ce02dfc-d8e8-bcd8-4ced-a09293cf1732@redhat.com> <8bcba445-524d-c0b2-cd11-03ef82f3f5a5@linaro.org> <1065eb07-b190-6edd-fc53-a717569a2273@gotplt.org> <alpine.DEB.2.20.1707041944400.14976@digraph.polyomino.org.uk> <7d896fb7-b83e-41e9-527d-01be5e299b89@gotplt.org> <alpine.DEB.2.20.1707051032390.28435@digraph.polyomino.org.uk> From: Adhemerval Zanella <adhemerval.zanella@linaro.org> Message-ID: <fcd9fac8-f689-4f38-a53c-c2d09dd5230f@linaro.org> Date: Wed, 5 Jul 2017 10:48:06 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <alpine.DEB.2.20.1707051032390.28435@digraph.polyomino.org.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit |
Commit Message
Adhemerval Zanella Netto
July 5, 2017, 1:48 p.m. UTC
On 05/07/2017 07:34, Joseph Myers wrote: > On Wed, 5 Jul 2017, Siddhesh Poyarekar wrote: > >>> there's also the possibility of interactions with the thread types >>> headers, if there's anything in the architecture-specific headers that >>> works for pthreads but not for C11 threads. >> >> From a quick look at the changes, I couldn't find any changes in the >> arch-specific sysdeps directories other than the abilist changes, i.e. >> there are no architecture-specific headers. Bugs in C11 should thus be > > The thread types headers refactoring, to make them usable for both C11 > threads and pthreads, went in some time ago. > > All that architecture-specific header content will get used in C11 threads > and it's far from obvious that any issues that appear with C11 threads > would also appear with pthreads. It was not clear to me if you still consider C11 threads patches a disruptive for arch-testing that can't not get validated in current cross-compiling testing. I think the only snippet that differs significantly from pthread is related to thread creation [1] where since different signatures between POSIX and C11 I had to add an explicit cast and recast: iff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 7a970ff..3efb76d 100644 Besides that, all other implementation uses pthread code directly and with a validation of pthread primitives it should not show any issues as you pointed out. [1] https://sourceware.org/ml/libc-alpha/2017-06/msg01418.html
Comments
On Wed, 5 Jul 2017, Adhemerval Zanella wrote: > > The thread types headers refactoring, to make them usable for both C11 > > threads and pthreads, went in some time ago. > > > > All that architecture-specific header content will get used in C11 threads > > and it's far from obvious that any issues that appear with C11 threads > > would also appear with pthreads. > > It was not clear to me if you still consider C11 threads patches a > disruptive for arch-testing that can't not get validated in current > cross-compiling testing. I think the ABI baselines are adequately validated by cross-compilation testing, but there's a risk of architecture-specific problems that only show up in C11 threads execution testing being missed.
On 05/07/2017 10:51, Joseph Myers wrote: > On Wed, 5 Jul 2017, Adhemerval Zanella wrote: > >>> The thread types headers refactoring, to make them usable for both C11 >>> threads and pthreads, went in some time ago. >>> >>> All that architecture-specific header content will get used in C11 threads >>> and it's far from obvious that any issues that appear with C11 threads >>> would also appear with pthreads. >> >> It was not clear to me if you still consider C11 threads patches a >> disruptive for arch-testing that can't not get validated in current >> cross-compiling testing. > > I think the ABI baselines are adequately validated by cross-compilation > testing, but there's a risk of architecture-specific problems that only > show up in C11 threads execution testing being missed. > I do see your rationale, but we are already pushing changes without much testing in master that are much more disruptive IMHO (maybe not for architecture cases, but on performance for instance) such as single thread FILE optimization and thread local cache. But I think we can postpone C11 threads for 2.27, although for the specific architecture where you already posted some testing results (arm hard-float, powerpc soft-float, and mips), I think it would be feasible to run the C11 threads tests on emulated qemu system to validate them.
On 07/05/2017 02:19 PM, Adhemerval Zanella wrote: > > > On 05/07/2017 10:51, Joseph Myers wrote: >> On Wed, 5 Jul 2017, Adhemerval Zanella wrote: >> >>>> The thread types headers refactoring, to make them usable for both C11 >>>> threads and pthreads, went in some time ago. >>>> >>>> All that architecture-specific header content will get used in C11 threads >>>> and it's far from obvious that any issues that appear with C11 threads >>>> would also appear with pthreads. >>> >>> It was not clear to me if you still consider C11 threads patches a >>> disruptive for arch-testing that can't not get validated in current >>> cross-compiling testing. >> >> I think the ABI baselines are adequately validated by cross-compilation >> testing, but there's a risk of architecture-specific problems that only >> show up in C11 threads execution testing being missed. >> > I do see your rationale, but we are already pushing changes without much > testing in master that are much more disruptive IMHO (maybe not for > architecture cases, but on performance for instance) such as single thread > FILE optimization and thread local cache. Each issue needs to be discussed independently. Each issue has their own risks and rewards. > But I think we can postpone C11 threads for 2.27, although for the specific > architecture where you already posted some testing results (arm hard-float, > powerpc soft-float, and mips), I think it would be feasible to run the C11 > threads tests on emulated qemu system to validate them. Can we commit to checking in C11 as soon as 2.27 opens? What else do we need to do to get there?
On 05/07/2017 17:16, Carlos O'Donell wrote: > On 07/05/2017 02:19 PM, Adhemerval Zanella wrote: >> >> >> On 05/07/2017 10:51, Joseph Myers wrote: >>> On Wed, 5 Jul 2017, Adhemerval Zanella wrote: >>> >>>>> The thread types headers refactoring, to make them usable for both C11 >>>>> threads and pthreads, went in some time ago. >>>>> >>>>> All that architecture-specific header content will get used in C11 threads >>>>> and it's far from obvious that any issues that appear with C11 threads >>>>> would also appear with pthreads. >>>> >>>> It was not clear to me if you still consider C11 threads patches a >>>> disruptive for arch-testing that can't not get validated in current >>>> cross-compiling testing. >>> >>> I think the ABI baselines are adequately validated by cross-compilation >>> testing, but there's a risk of architecture-specific problems that only >>> show up in C11 threads execution testing being missed. >>> >> I do see your rationale, but we are already pushing changes without much >> testing in master that are much more disruptive IMHO (maybe not for >> architecture cases, but on performance for instance) such as single thread >> FILE optimization and thread local cache. > > Each issue needs to be discussed independently. > > Each issue has their own risks and rewards. I understand that and I give you that both examples I gave had a more thoughtfully review before inclusion. And I also understand C11 threads seems to not be an urgent upcoming new feature for 2.26 (given relative lack of interest and review). > >> But I think we can postpone C11 threads for 2.27, although for the specific >> architecture where you already posted some testing results (arm hard-float, >> powerpc soft-float, and mips), I think it would be feasible to run the C11 >> threads tests on emulated qemu system to validate them. > > Can we commit to checking in C11 as soon as 2.27 opens? > > What else do we need to do to get there? Mostly some reviews of the patch themselves. Mostly are straightforward wrapper and code examples, maybe expect the thrd_* one that changes some internal pthread code for c11 call format. The documentation got some review, but I am not sure if it would be better to glue it to threads.texi or let in its own file.
--- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -461,7 +461,19 @@ START_THREAD_DEFN LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg); /* Run the code the user provided. */ - THREAD_SETMEM (pd, result, pd->start_routine (pd->arg)); + void *ret; + if (pd->c11) + { + /* The function pointer of the c11 thread start is cast to an incorrect + type on __pthread_create_2_1 call, however it is casted back to correct + one so the call behavior is well-defined (it is assumed that pointers + to void are able to represent all values of int. */ + int (*start)(void*) = (int (*) (void*)) pd->start_routine; + ret = (void*) (intptr_t) start (pd->arg); + } + else + ret = pd->start_routine (pd->arg); + THREAD_SETMEM (pd, result, ret); } /* Call destructors for the thread_local TLS variables. */