From patchwork Fri Feb 23 19:13:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 26040 Received: (qmail 68061 invoked by alias); 23 Feb 2018 19:13:46 -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 68050 invoked by uid 89); 23 Feb 2018 19:13:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-oi0-f66.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=DTtsnxcGk2v2dVdYgTWPTyCQbrbtXB5FigKvcCCutOU=; b=iJl26RJ1PQsqMLus2kCmNEoq9GnGC9wRhflIvv+iKCKb0wXhbE2XYgVtB4hNNYjjaX BZ2H1QzWS6Es3OSnpQgSQpnKU/VqJ8RiAUuHsIpDBHIVV/PZxyLTclhj8j0FuVi8AihW a96GFeUJvlzKypqcg5gYZojf6ZjG70i84ojVwEwLAqe+GMLvg6U0IZ62HhEU0P0yyH9E SwhMxKYN0e6eX26+6+iafPd64stsb54aXHYcqGSdsHhoNRLUBNkIpbOVAyS8Y/6fXeWV niIuHvQi7bh3rD5qjqFmSyLnTH3/EKJxoN1CAQMx2qdSK3msam6Tn+bUR8OA6yRSPX1Z r0xw== X-Gm-Message-State: APf1xPCdgMg0R2tp/P4WGat1rE0D7q3UBhDV7wheNH8Slw02Lmc43sCa F52yqTynMqCELeLKo6DVS2Pkw0HLNSpW9BJ1eiw= X-Google-Smtp-Source: AG47ELscspAf4WWjLBcLr4nyXzugN5e67akUHVn5a0SwvSGlUbgA57R30111xjkF1rSBWXlpAfNI/0/shWrSll/HioQ= X-Received: by 10.202.221.131 with SMTP id u125mr1626696oig.104.1519413221521; Fri, 23 Feb 2018 11:13:41 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <0991cb4f-178e-664d-874b-7c30a0787211@redhat.com> References: <20180208175818.GA8519@gmail.com> <7a406aa7-36f8-cdeb-c021-4857e398ca39@redhat.com> <0991cb4f-178e-664d-874b-7c30a0787211@redhat.com> From: "H.J. Lu" Date: Fri, 23 Feb 2018 11:13:40 -0800 Message-ID: Subject: Re: [PATCH] Define GEN_AS_CONST_HEADERS when generating header files [BZ #22792] To: "Carlos O'Donell" Cc: Florian Weimer , GNU C Library On Thu, Feb 22, 2018 at 8:46 AM, Carlos O'Donell wrote: > On 02/22/2018 07:11 AM, Florian Weimer wrote: >> On 02/21/2018 06:19 PM, H.J. Lu wrote: >>> On Wed, Feb 21, 2018 at 7:40 AM, H.J. Lu wrote: >>>> On Tue, Feb 20, 2018 at 11:09 PM, Florian Weimer wrote: >>>>> On 02/08/2018 06:58 PM, H.J. Lu wrote: >>>>>> >>>>>> [BZ #22792] >>>>>> * Makerules ($(common-objpfx)%.h): Pass -DGEN_AS_CONST_HEADERS >>>>>> to $(CC). >>>>>> * sysdeps/unix/sysv/linux/i386/lowlevellock.h: Include >>>>>> only if GEN_AS_CONST_HEADERS isn't defined. >>>>>> * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Don't include >>>>>> . >>>>> >>>>> >>>>> It looks to me that a cleaner fix would be to move the definition of the TCB >>>>> types to a separate header. >>>> >>>> I will take a look. >>> >>> sysdeps/i386/nptl/tcb-offsets.sym has >>> >>> #include >>> #include >>> #include >>> >>> RESULT offsetof (struct pthread, result) >>> TID offsetof (struct pthread, tid) >>> CANCELHANDLING offsetof (struct pthread, cancelhandling) >>> CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf) >>> MULTIPLE_THREADS_OFFSET offsetof (tcbhead_t, multiple_threads) >>> SYSINFO_OFFSET offsetof (tcbhead_t, sysinfo) >>> CLEANUP offsetof (struct pthread, cleanup) >>> CLEANUP_PREV offsetof (struct _pthread_cleanup_buffer, __prev) >>> MUTEX_FUTEX offsetof (pthread_mutex_t, __data.__lock) >>> POINTER_GUARD offsetof (tcbhead_t, pointer_guard) >>> #ifndef __ASSUME_PRIVATE_FUTEX >>> PRIVATE_FUTEX offsetof (tcbhead_t, private_futex) >>> #endif >>> >>> It covers more than just tcbhead_t. A separate header file for tcbhead_t >>> won't help here. >> >> Yes, it's more involved. >> >>>>> The circular dependency is just a cosmetic warning from make, right? >>>> >>>> It depends on how unlucky you are. In one case on a many-core machine >>>> with make -j60, I had to kill glibc build since it never stopped. >> >> If it fixes an actual build issue, then I think your patch is acceptable, although I consider it quite hackish. >> >> But please do add a comment explaining the issue before: >> >> +# ifndef GEN_AS_CONST_HEADERS > > Agreed, I think the hack is OK, but needs a multi-line comment > explaining *why* the simple route of header disentanglement > is not easily possible and why this is needed to bootstrap > the automatic header building without a circular dependency. > This is the patch I am checking in. Thanks. From 55a426ef8c54eb1d6e48b5858eb43496b1742797 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 8 Feb 2018 08:37:54 -0800 Subject: [PATCH] Define GEN_AS_CONST_HEADERS when generating header files [BZ #22792] Glibc build generates header files to define constants from special .sym files. If a .sym file includes the same header file which it generates, it leads to circular dependency which may lead to build hang on a many-core machine. Define GEN_AS_CONST_HEADERS when generating header files to avoid circular dependency. is needed for i686 and it isn't needed for x86-64 at least since glibc 2.23. Tested on i686 and x86-64. [BZ #22792] * Makerules ($(common-objpfx)%.h): Pass -DGEN_AS_CONST_HEADERS to $(CC). * sysdeps/unix/sysv/linux/i386/lowlevellock.h: Include only if GEN_AS_CONST_HEADERS isn't defined. * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Don't include . --- Makerules | 9 ++++++++- sysdeps/unix/sysv/linux/i386/lowlevellock.h | 9 ++++++++- sysdeps/unix/sysv/linux/x86_64/lowlevellock.h | 1 - 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Makerules b/Makerules index ef6abeac6d..b2c2724fcb 100644 --- a/Makerules +++ b/Makerules @@ -276,10 +276,17 @@ ifdef gen-as-const-headers # Generating headers for assembly constants. # We need this defined early to get into before-compile before # it's used in sysd-rules, below. +# Define GEN_AS_CONST_HEADERS to avoid circular dependency [BZ #22792]. +# NB: is generated from tcb-offsets.sym to define +# offsets and sizes of types in and maybe which +# may include . Target header files can check if +# GEN_AS_CONST_HEADERS is defined to avoid circular dependency which +# may lead to build hang on a many-core machine. $(common-objpfx)%.h $(common-objpfx)%.h.d: $(..)scripts/gen-as-const.awk \ %.sym $(common-before-compile) $(AWK) -f $< $(filter %.sym,$^) \ - | $(CC) -S -o $(@:.h.d=.h)T3 $(CFLAGS) $(CPPFLAGS) -x c - \ + | $(CC) -S -o $(@:.h.d=.h)T3 $(CFLAGS) $(CPPFLAGS) \ + -DGEN_AS_CONST_HEADERS -x c - \ -MD -MP -MF $(@:.h=.h.d)T -MT '$(@:.h=.h.d) $(@:.h.d=.h)' sed -n 's/^.*@@@name@@@\([^@]*\)@@@value@@@[^0-9Xxa-fA-F-]*\([0-9Xxa-fA-F-][0-9Xxa-fA-F-]*\).*@@@end@@@.*$$/#define \1 \2/p' \ $(@:.h.d=.h)T3 > $(@:.h.d=.h)T diff --git a/sysdeps/unix/sysv/linux/i386/lowlevellock.h b/sysdeps/unix/sysv/linux/i386/lowlevellock.h index fb59b57934..38fbc2556f 100644 --- a/sysdeps/unix/sysv/linux/i386/lowlevellock.h +++ b/sysdeps/unix/sysv/linux/i386/lowlevellock.h @@ -26,7 +26,14 @@ # include # include # include -# include +/* is generated from tcb-offsets.sym to define offsets + and sizes of types in as well as which includes + via nptl/descr.h. Don't include + when generating to avoid circular dependency which + may lead to build hang on a many-core machine. */ +# ifndef GEN_AS_CONST_HEADERS +# include +# endif # ifndef LOCK_INSTR # ifdef UP diff --git a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h index a8bcfbe4a3..eedb6fc990 100644 --- a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h +++ b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h @@ -26,7 +26,6 @@ # include # include # include -# include # ifndef LOCK_INSTR # ifdef UP -- 2.14.3