[{"id":1759346,"web_url":"http://patchwork.ozlabs.org/comment/1759346/","msgid":"","list_archive_url":null,"date":"2017-08-29T13:38:29","subject":"Re: [PATCH] ia64: Fix thread stack allocation permission set (BZ\n\t#21672)","submitter":{"id":22438,"url":"http://patchwork.ozlabs.org/api/people/22438/","name":"Carlos O'Donell","email":"carlos@redhat.com"},"content":"On 08/29/2017 09:34 AM, Adhemerval Zanella wrote:\n> This patch fixes ia64 failures on thread exit by madvise the required\n> area taking in consideration its disjoing stacks\n> (NEED_SEPARATE_REGISTER_STACK). Also the snippet that setup the\n> madvise call to advertise kernel the area won't be used anymore in\n> near future is reallocated in allocatestack.c (for consistency to\n> put all stack management function in one place).\n> \n> Checked on x86_64-linux-gnu and i686-linux-gnu for sanity (since\n> it is not expected code changes for architecture that do not\n> define NEED_SEPARATE_REGISTER_STACK) and also got a report that\n> it fixes ia64-linux-gnu failures from Sergei Trofimovich\n> .\n> \n> \t[BZ #21672]\n> \t* nptl/allocatestack.c [_STACK_GROWS_DOWN] (setup_stack_prot):\n> \tSet to use !NEED_SEPARATE_REGISTER_STACK as well.\n> \t(advise_stack_range): New function.\n> \t* nptl/pthread_create.c (START_THREAD_DEFN): Move logic to mark\n> \tstack non required to advise_stack_range at allocatestack.c\n\nLooks good to me.\n\n> ---\n> ChangeLog | 9 +++++++++\n> nptl/allocatestack.c | 29 ++++++++++++++++++++++++++++-\n> nptl/pthread_create.c | 27 ++-------------------------\n> 3 files changed, 39 insertions(+), 26 deletions(-)\n> \n> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c\n> index 6d1bcaa..8766deb 100644\n> --- a/nptl/allocatestack.c\n> +++ b/nptl/allocatestack.c\n> @@ -356,7 +356,7 @@ setup_stack_prot (char *mem, size_t size, char *guard, size_t guardsize,\n> \t\t const int prot)\n> {\n> char *guardend = guard + guardsize;\n> -#if _STACK_GROWS_DOWN\n> +#if _STACK_GROWS_DOWN && !defined(NEED_SEPARATE_REGISTER_STACK)\n> /* As defined at guard_position, for architectures with downward stack\n> the guard page is always at start of the allocated area. */\n> if (__mprotect (guardend, size - guardsize, prot) != 0)\n> @@ -372,6 +372,33 @@ setup_stack_prot (char *mem, size_t size, char *guard, size_t guardsize,\n> return 0;\n> }\n> \n> +/* Mark the memory of the stack as usable to the kernel. It frees everything\n> + except for the space used for the TCB itself. */\n> +static inline void\n> +__always_inline\n> +advise_stack_range (void *mem, size_t size, uintptr_t pd, size_t guardsize)\n> +{\n> + uintptr_t sp = (uintptr_t) CURRENT_STACK_FRAME;\n> + size_t pagesize_m1 = __getpagesize () - 1;\n> +#if _STACK_GROWS_DOWN && !defined(NEED_SEPARATE_REGISTER_STACK)\n> + size_t freesize = (sp - (uintptr_t) mem) & ~pagesize_m1;\n> + assert (freesize < size);\n> + if (freesize > PTHREAD_STACK_MIN)\n> + __madvise (mem, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);\n> +#else\n> + /* Page aligned start of memory to free (higher than or equal\n> + to current sp plus the minimum stack size). */\n> + uintptr_t freeblock = (sp + PTHREAD_STACK_MIN + pagesize_m1) & ~pagesize_m1;\n> + uintptr_t free_end = (pd - guardsize) & ~pagesize_m1;\n> + if (free_end > freeblock)\n> + {\n> + size_t freesize = free_end - freeblock;\n> + assert (freesize < size);\n> + __madvise ((void*) freeblock, freesize, MADV_DONTNEED);\n> + }\n> +#endif\n> +}\n> +\n> /* Returns a usable stack for a new thread either by allocating a\n> new stack or reusing a cached stack of sufficient size.\n> ATTR must be non-NULL and point to a valid pthread_attr.\n> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c\n> index 2f8ada3..83b88bf 100644\n> --- a/nptl/pthread_create.c\n> +++ b/nptl/pthread_create.c\n> @@ -551,31 +551,8 @@ START_THREAD_DEFN\n> }\n> #endif\n> \n> - /* Mark the memory of the stack as usable to the kernel. We free\n> - everything except for the space used for the TCB itself. */\n> - size_t pagesize_m1 = __getpagesize () - 1;\n> -#ifdef _STACK_GROWS_DOWN\n> - char *sp = CURRENT_STACK_FRAME;\n> - size_t freesize = (sp - (char *) pd->stackblock) & ~pagesize_m1;\n> - assert (freesize < pd->stackblock_size);\n> - if (freesize > PTHREAD_STACK_MIN)\n> - __madvise (pd->stackblock, freesize - PTHREAD_STACK_MIN, MADV_DONTNEED);\n> -#else\n> - /* Page aligned start of memory to free (higher than or equal\n> - to current sp plus the minimum stack size). */\n> - void *freeblock = (void*)((size_t)(CURRENT_STACK_FRAME\n> -\t\t\t\t + PTHREAD_STACK_MIN\n> -\t\t\t\t + pagesize_m1)\n> -\t\t\t\t & ~pagesize_m1);\n> - char *free_end = (char *) (((uintptr_t) pd - pd->guardsize) & ~pagesize_m1);\n> - /* Is there any space to free? */\n> - if (free_end > (char *)freeblock)\n> - {\n> - size_t freesize = (size_t)(free_end - (char *)freeblock);\n> - assert (freesize < pd->stackblock_size);\n> - __madvise (freeblock, freesize, MADV_DONTNEED);\n> - }\n> -#endif\n> + advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,\n> +\t\t pd->guardsize);\n> \n> /* If the thread is detached free the TCB. */\n> if (IS_DETACHED (pd))\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list libc-alpha@sourceware.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=sourceware.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=libc-alpha-return-83815-incoming=patchwork.ozlabs.org@sourceware.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tsecure) header.d=sourceware.org header.i=@sourceware.org\n\theader.b=\"BNEDNAib\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xhVB813RPz9sRV\n\tfor ;\n\tTue, 29 Aug 2017 23:39:59 +1000 (AEST)","(qmail 122390 invoked by alias); 29 Aug 2017 13:39:53 -0000","(qmail 122360 invoked by uid 89); 29 Aug 2017 13:39:52 -0000"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id\n\t:list-unsubscribe:list-subscribe:list-archive:list-post\n\t:list-help:sender:subject:to:cc:references:from:message-id:date\n\t:mime-version:in-reply-to:content-type\n\t:content-transfer-encoding; q=dns; s=default; b=sRiNozb869BeuDwZ\n\tt99hSMxJ9wPDSmMIGlQfExrzoa5sx5dTYmhfo28zjVoY8VhTed+n2CluHJwl6NHw\n\tFfRnx8f92ptQVgrYCyyryDUQhrx7SKPJYTdrymedyMWRaTnseq/fTYvqSnQlV1B/\n\tCQnhSf4LGBgyADNdOiGVpgAmZrQ=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id\n\t:list-unsubscribe:list-subscribe:list-archive:list-post\n\t:list-help:sender:subject:to:cc:references:from:message-id:date\n\t:mime-version:in-reply-to:content-type\n\t:content-transfer-encoding; s=default; bh=Jp70KJoosPtfVEsf6dO8h+\n\tuYrnM=; b=BNEDNAibXcy/wSnVxdb+WCXDEhl0oHJlmpr+TFT6uEF5UqpQma1pOp\n\tID9Q3uUV2oASytMl7EJuTePxIWck3XSRgcwi6HNjGoJee8TLrSdhFeIijNMOxSJP\n\tYcTYzmccI2db+x9gWWHIwbLKdi0ecdTFjb5t4VhWjygMO+A1PJT5U=","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":",\n\t","Sender":"libc-alpha-owner@sourceware.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-27.1 required=5.0 tests=BAYES_00, GIT_PATCH_0,\n\tGIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW,\n\tRCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=","X-HELO":"mail-qt0-f182.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=ntRnB6WGozpyHX6hWn7/iSTRT4ZLDOAvLzUJZndmalg=;\n\tb=LLwdb7gLXhchUvWb1jjyx4dSgneGjkBVHnTJymX4p2SxiYliu0ZfHLBVukoUNm0YqS\n\t+LsxSyZ7JXKjMWUaqm8jU4AsD+6w6ERib8j7p/6wjXLM9ZKHzpVlPvwUAQ61vw42fz5d\n\tet1Q8jkCCyk0HIj+n8f7IM8JmyvFMamt8rT8N3p5V8GZqDmL/FFRWz0A+8GLqqyvFIFt\n\tUJVxxGw2bKjT+edlx/QCtf6RoAlwB1UqsFyrzwvZRSY/30E16E4KKuiaLUpdcBd4Emds\n\tJ8BAqyzM9sNh/NK+fsX4Ac3URIAQfk/yhsArGba/1dvGKTueL/X+zQWQ68sY0GeUeRFU\n\t/Q7g==","X-Gm-Message-State":"AHYfb5hfbRPDPmzWcwRR1DH7ElKo9wgeVO38NR8NiqpmjFKMyGSDsf73\n\tc1l0Q/3NYS6IPkRQ67jBXw==","X-Google-Smtp-Source":"ADKCNb63VgC1BxYYrrgJ9/AQo1pvKOnIIqzRGNWapF+J7m90ALwJaPq0jL9V+zkQtyHwkU4PQZm+FQ==","X-Received":"by 10.200.46.78 with SMTP id s14mr5565898qta.14.1504013980006;\n\tTue, 29 Aug 2017 06:39:40 -0700 (PDT)","Subject":"Re: [PATCH] ia64: Fix thread stack allocation permission set (BZ\n\t#21672)","To":"Adhemerval Zanella ,\n\tlibc-alpha@sourceware.org","Cc":"Sergei Trofimovich ","References":"<1504013662-1367-1-git-send-email-adhemerval.zanella@linaro.org>","From":"Carlos O'Donell ","Message-ID":"","Date":"Tue, 29 Aug 2017 09:38:29 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504013662-1367-1-git-send-email-adhemerval.zanella@linaro.org>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"7bit"}}]