Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1

2020-04-22 Thread Steve McIntyre
Hi folks!

I'm adding a CC to Steve Capper, a colleague in Arm who's our expert
here for this kind of question. He's also a DM in Debian... :-)

On Tue, Apr 21, 2020 at 06:37:07PM -0400, Noah Meyerhans wrote:
>On Sun, Apr 12, 2020 at 12:18:35PM +0200, Aurelien Jarno wrote:
>
>> It would also be nice to have numbers to see the impact on non-ARMv8.1
>> CPU on real workloads. As pointed out by Florian, and if the impact is
>> negligible, it might be a good idea to enable -moutline-atomics
>> globally at the GCC level so that all software can benefit from it, and
>> instead of only glibc. That could be either upstream or only in Debian,
>> that's probably a separate discussion. Otherwise we will likely end up
>> using this non-default GCC option on all packages that runs faster with
>> it.
>
>Agreed.

I think the -moutline-atomics is probably good to enable by default
once we've got it (gcc 10). that's the suggestion I've heard from gcc
folks in Arm.

>> Also note that the mechanism allowing a safe upgrade *does* incur a 
>> runtime overhead as every binary now has to test for the presence of
>> /etc/ld.so.nohwcap to detect a possible upgrade of the glibc in
>> progress. That's why we have disabled it on architecture not providing
>> an optimized library [1].

Oh, ick. :-/

>Thanks for the pointer, it's interesting to see data on that.  This also
>suggests that it might be worthwhile to investigate a better mechanism
>for identifying the availability of hardware features.
>
>> > I've tested both options and found them to be acceptable on v8.1a (Neoverse
>> > N1) and v8a (Cortex A72) CPUs.  I can provide bulk test run data of the
>> > various different configuration permutations if you'd like to see 
>> > additional
>> > data.
>> 
>> As said above I think we would need more numbers on real workload to
>> take a decision. Don't get me wrong I do not oppose on improving atomics
>> on ARMv8.1, but I would like that we chose the best option. Also if we
>> go with the -moutline-atomics option, I believe it rather has to be a
>> ARM porters decision than a glibc maintainers decision (hence the Cc:).
>
>I'll see what I can come up with.
>
>Do the arm porters have any opinions on this matter?

It's a good question, and thanks for asking! I definitely think it's
worth doing -moutline-atomics, and I'm hoping Steve can share some
performance numbers to help convince. :-)

-- 
Steve McIntyre, Cambridge, UK.st...@einval.com
Who needs computer imagery when you've got Brian Blessed?



Bug#798955: [PATCH] libstdc++: don't use #include_next in c_global headers

2020-04-22 Thread Helmut Grohne
Hi,

On Mon, Apr 20, 2020 at 10:12:37AM +0100, Jonathan Wakely wrote:
> > Now you are probably going to say that "-isystem /usr/include" is a bad
> > idea and that you shouldn't do that.
> 
> Right.
> 
> > I'm inclined to agree. This isn't a
> > problem just yet. Debian wants to move /usr/include/stdlib.h to
> > /usr/include//stdlib.h. After that move, the problematic flag
> > becomes "-isystem /usr/include/". Unfortunately, around 30
> > Debian packages[1] do pass exactly that flag. Regardless whether doing
> > so is a bad idea, I guess we will have to support that.
> 
> Or Debian should fix what they're going to break.

This is not quite precise. The offending -isystem
/usr/include/ flag is already being passed. According to what
you write later, doing so is broken today. It just happens to work by
accident. So all we do is making the present breakage visible.

> > I am proposing to replace those two #include_next with plain #include.
> > That'll solve the problem described above, but it is not entirely
> > obvious that doing so doesn't break something else.
> > 
> > After switching those #include_next to #include,
> > libstdc++-v3/include/c_global/cstdlib will continue to temporarily
> > will #include . Now, it'll search all include directories. It
> > may find libstdc++-v3/include/c_comaptibility/stdlib.h or the libc's
> > version. We cannot tell which. If it finds the one from libstdc++-v3,
> > the header will notice the _GLIBCXX_INCLUDE_NEXT_C_HEADERS macro and
> > immediately #include_next  skipping the rest of the header.
> > That in turn will find the libc version. So in both cases, it ends up
> > using the right one. Precisely what we wanted.
> 
> As Marc said, this doesn't work.

That is not very precise either. Marc said that it won't fix all cases.
In practice, it would make those work that don't #include  but
use #include  instead.

Marc also indicated that using include_next for a header of a different
name is wrong. So this is a bug in libstdc++ regardless of whether it
breaks or unbreaks other pieces of software.

> If a program tries to include  it needs to get the libstdc++
> version, otherwise only the libc versions of certain functions are
> defined. That means the additional C++ overloads such as ::abs(long)
> and ::abs(long long) won't be defined. That is the reason why
> libstdc++ provides its own .
> 
> And if you do -isystem /usr/include (or any other option that causes
> libstdc++'s  to be skipped) that doesn't work. Only
> ::abs(int) gets defined.
> 
> So -isystem /usr/include breaks code, with or without your patch.

It is very difficult to disagree with -isystem /usr/include or -isystem
/usr/include/ being broken and unsupported. Having you state it
that clearly does help with communicating to other upstreams. For this
reason, I've looked into the remaining cases. It turns out that there
aren't that many left. In particular chromium, opencv and vtk got fixed
in the mean time. Basically all remaining failures could be attributed
to qmake, which passes all directories below /usr/include (including
/usr/include and /usr/include/ if a .pc file mentions them)
using -isystem. I've sent a patch https://bugs.debian.org/958479 to make
qmake stop doing that.

I therefore agree with you that the patch I sent for libstdc++ is not
necessary to make packages build on Debian. Removing the offending
-isystem flags from the respective builds is a manageable option and has
already happened to a large extend.

We can conclude that the motivation for my patch is not a good one,
because it embraces broken behaviour. However, the use of include_next
remains a bug, because the name of the including and the name of the
included header differ, and it should be fixed on that ground.

Helmut



Bug#91815: Patch to add memusage and memusagestat

2020-04-22 Thread Stephen Kitt
Hi,

On Wed, 22 Apr 2020 00:04:32 +0200, Aurelien Jarno  wrote:
> On 2020-04-21 20:58, Stephen Kitt wrote:
> Thanks for trying to fix one of the oldest glibc bugs ;-)

You’re welcome!

> > The attached patch adds memusage and memusagestat to the libc-bin package.
> > This does mean that the latter becomes dependent on libgd3, so it might be
> > better to add a new memusage package; I can take care of that if the
> > maintainers think it’s better.  
> 
> I am not sure we want to add a new dependency for libc-bin, I am sure
> people running embedded systems won't appreciate. Any reason for not
> shipping it in libc-dev-bin instead? For me that looks more like a tool
> to be used for "development". At least the memusagestat is similar to
> the mtrace one that is in libc-dev-bin.

Indeed, especially since libgd3 is a rather heavy-weight dependency. I’m
attaching a patch which adds the tools to libc-dev-bin instead.

> We also have to make sure that this new build-dependency doesn't break
> bootstrapping. I have added Helmut in Cc so that he can have a look.

Isn’t non-cross bootstrapping handled by the stage1 build profile? If this
causes issues with cross bootstrapping then shipping a separate memusage
package ( ) should take care of things.

Regards,

Stephen
From bd3f09edb6296b8a8e2987aefd3169d48903625f Mon Sep 17 00:00:00 2001
From: Stephen Kitt 
Date: Tue, 21 Apr 2020 20:55:53 +0200
Subject: [PATCH] Build and package memusage*

This builds memusage and memusagestat in the libc pass, and ships them
in libc-dev-bin. This involves adding a build-dependency on
libgd-dev (outside stage1) and libc-dev-bin acquires a runtime
dependency on the corresponding library package.

Closes: #91815
Signed-off-by: Stephen Kitt 
---
 debian/control.in/main | 3 ++-
 debian/debhelper.in/libc-dev-bin.install   | 2 ++
 debian/debhelper.in/libc-dev-bin.lintian-overrides | 2 ++
 debian/rules.d/build.mk| 6 +-
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/debian/control.in/main b/debian/control.in/main
index 659267bd..3537b751 100644
--- a/debian/control.in/main
+++ b/debian/control.in/main
@@ -12,7 +12,8 @@ Build-Depends: gettext, dpkg (>= 1.18.7), dpkg-dev (>= 1.17.14), xz-utils, file,
  g++-9, g++-9-multilib [amd64 i386 kfreebsd-amd64 mips mipsel mipsn32 mipsn32el mips64 mips64el mipsr6 mipsr6el mipsn32r6 mipsn32r6el mips64r6 mips64r6el powerpc ppc64 s390x sparc sparc64 x32] ,
  python3:native,
  libidn2-0 (>= 2.0.5~) ,
- libc-bin (>= @GLIBC_VERSION@) 
+ libc-bin (>= @GLIBC_VERSION@) ,
+ libgd-dev 
 Build-Depends-Indep: perl, po-debconf (>= 1.0)
 Maintainer: GNU Libc Maintainers 
 Uploaders: Clint Adams , Aurelien Jarno , Adam Conrad , Samuel Thibault 
diff --git a/debian/debhelper.in/libc-dev-bin.install b/debian/debhelper.in/libc-dev-bin.install
index 0d760a7e..8ced4378 100644
--- a/debian/debhelper.in/libc-dev-bin.install
+++ b/debian/debhelper.in/libc-dev-bin.install
@@ -1,4 +1,6 @@
 debian/tmp-libc/usr/bin/gencat usr/bin
+debian/tmp-libc/usr/bin/memusage usr/bin
+debian/tmp-libc/usr/bin/memusagestat usr/bin
 debian/tmp-libc/usr/bin/mtrace usr/bin
 debian/tmp-libc/usr/bin/rpcgen usr/bin
 debian/tmp-libc/usr/bin/sotruss usr/bin
diff --git a/debian/debhelper.in/libc-dev-bin.lintian-overrides b/debian/debhelper.in/libc-dev-bin.lintian-overrides
index eedd7cbd..1031449a 100644
--- a/debian/debhelper.in/libc-dev-bin.lintian-overrides
+++ b/debian/debhelper.in/libc-dev-bin.lintian-overrides
@@ -1,3 +1,5 @@
 # these manpages are provided by the manpages package
+libc-dev-bin: binary-without-manpage usr/bin/memusage
+libc-dev-bin: binary-without-manpage usr/bin/memusagestat
 libc-dev-bin: binary-without-manpage usr/bin/mtrace
 libc-dev-bin: binary-without-manpage usr/bin/sprof
diff --git a/debian/rules.d/build.mk b/debian/rules.d/build.mk
index 0d03116a..3f664316 100644
--- a/debian/rules.d/build.mk
+++ b/debian/rules.d/build.mk
@@ -37,7 +37,11 @@ $(stamp)configure_%: $(stamp)config_sub_guess $(stamp)patch $(KERNEL_HEADER_DIR)
 	echo "BASH := /bin/bash"  >> $(DEB_BUILDDIR)/configparms
 	echo "KSH := /bin/bash"   >> $(DEB_BUILDDIR)/configparms
 	echo "SHELL := /bin/bash" >> $(DEB_BUILDDIR)/configparms
-	echo "LIBGD = no" >> $(DEB_BUILDDIR)/configparms
+	if [ "$(curpass)" = "libc" ]; then \
+	  echo "LIBGD = yes"  >> $(DEB_BUILDDIR)/configparms; \
+	else \
+	  echo "LIBGD = no"   >> $(DEB_BUILDDIR)/configparms; \
+	fi
 	echo "bindir = $(bindir)" >> $(DEB_BUILDDIR)/configparms
 	echo "datadir = $(datadir)"   >> $(DEB_BUILDDIR)/configparms
 	echo "complocaledir = $(complocaledir)"   >> $(DEB_BUILDDIR)/configparms
-- 
2.20.1



pgpEALW2r4ByK.pgp
Description: OpenPGP digital signature


Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1

2020-04-22 Thread Noah Meyerhans
On Wed, Apr 22, 2020 at 05:48:27PM +0100, Steve McIntyre wrote:
> I think the -moutline-atomics is probably good to enable by default
> once we've got it (gcc 10). that's the suggestion I've heard from gcc
> folks in Arm.

JFTR, it's been backported to gcc 9 and is available in Debian's gcc-9
as of 9.3.0-9. See
https://salsa.debian.org/toolchain-team/gcc/-/blob/gcc-9-debian/debian/patches/git-updates.diff

noah



Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1

2020-04-22 Thread Steve Capper
On Wed, Apr 22, 2020 at 05:48:27PM +0100, Steve McIntyre wrote:
> Hi folks!

Hiya,

> 
> I'm adding a CC to Steve Capper, a colleague in Arm who's our expert
> here for this kind of question. He's also a DM in Debian... :-)

Now I feel guilty about not doing enough Debian :-).

> 
> On Tue, Apr 21, 2020 at 06:37:07PM -0400, Noah Meyerhans wrote:
> >On Sun, Apr 12, 2020 at 12:18:35PM +0200, Aurelien Jarno wrote:
> >
> >> It would also be nice to have numbers to see the impact on non-ARMv8.1
> >> CPU on real workloads. As pointed out by Florian, and if the impact is
> >> negligible, it might be a good idea to enable -moutline-atomics
> >> globally at the GCC level so that all software can benefit from it, and
> >> instead of only glibc. That could be either upstream or only in Debian,
> >> that's probably a separate discussion. Otherwise we will likely end up
> >> using this non-default GCC option on all packages that runs faster with
> >> it.
> >
> >Agreed.
> 
> I think the -moutline-atomics is probably good to enable by default
> once we've got it (gcc 10). that's the suggestion I've heard from gcc
> folks in Arm.
> 
> >> Also note that the mechanism allowing a safe upgrade *does* incur a 
> >> runtime overhead as every binary now has to test for the presence of
> >> /etc/ld.so.nohwcap to detect a possible upgrade of the glibc in
> >> progress. That's why we have disabled it on architecture not providing
> >> an optimized library [1].
> 
> Oh, ick. :-/
> 
> >Thanks for the pointer, it's interesting to see data on that.  This also
> >suggests that it might be worthwhile to investigate a better mechanism
> >for identifying the availability of hardware features.
> >
> >> > I've tested both options and found them to be acceptable on v8.1a 
> >> > (Neoverse
> >> > N1) and v8a (Cortex A72) CPUs.  I can provide bulk test run data of the
> >> > various different configuration permutations if you'd like to see 
> >> > additional
> >> > data.

That's good to hear!

> >> 
> >> As said above I think we would need more numbers on real workload to
> >> take a decision. Don't get me wrong I do not oppose on improving atomics
> >> on ARMv8.1, but I would like that we chose the best option. Also if we
> >> go with the -moutline-atomics option, I believe it rather has to be a
> >> ARM porters decision than a glibc maintainers decision (hence the Cc:).
> >
> >I'll see what I can come up with.
> >
> >Do the arm porters have any opinions on this matter?
> 
> It's a good question, and thanks for asking! I definitely think it's
> worth doing -moutline-atomics, and I'm hoping Steve can share some
> performance numbers to help convince. :-)
> 

We ran -moutline-atomics on a mixture of development hardware running,
IIRC some DPDK lock tests that employed C11-style atomics. As expected
there was a performance penalty, but it was order of magnitude of 1%.
The perf boost from moving to LSE was a lot larger (and we noticed the
variance dropping a lot with LSE too).

FWIW, I'd recommend the -moutline-atomics for the general case. (I used
to be a fan of the multi-lib approach; but the way the runtime selection
is implemented in gcc with a direct branch changed my mind :-) ).

Cheers,
-- 
Steve



Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1

2020-04-22 Thread Steve McIntyre
On Wed, Apr 22, 2020 at 01:08:46PM -0400, Noah Meyerhans wrote:
>On Wed, Apr 22, 2020 at 05:48:27PM +0100, Steve McIntyre wrote:
>> I think the -moutline-atomics is probably good to enable by default
>> once we've got it (gcc 10). that's the suggestion I've heard from gcc
>> folks in Arm.
>
>JFTR, it's been backported to gcc 9 and is available in Debian's gcc-9
>as of 9.3.0-9. See
>https://salsa.debian.org/toolchain-team/gcc/-/blob/gcc-9-debian/debian/patches/git-updates.diff

Ah, cool. I knew it *was* being backported, but I wasn't aware it was
already with us. Woot!

-- 
Steve McIntyre, Cambridge, UK.st...@einval.com
  Getting a SCSI chain working is perfectly simple if you remember that there
  must be exactly three terminations: one on one end of the cable, one on the
  far end, and the goat, terminated over the SCSI chain with a silver-handled
  knife whilst burning *black* candles. --- Anthony DeBoer



Bug#956418: src:glibc: Please provide optimized builds for ARMv8.1

2020-04-22 Thread Florian Weimer
* Noah Meyerhans:

> On Sun, Apr 12, 2020 at 12:18:35PM +0200, Aurelien Jarno wrote:
>> > Significant performance impact has also been observed in less contrived
>> > cases (MariaDB and Postgres), but I don't have a repro to share.
>> 
>> But indeed what counts is number on real workloads. It would be nice to
>> get numbers when those software are run against a rebuilt glibc. As
>> those software are using a lot of atomics directly, it would be also
>> interesting to have numbers with those software also rebuilt to use
>> those new instructions.
>
> Agreed.  I don't have specific examples of real world impact at the
> moment.  AIUI, the most significant impact comes in the usage of atomics
> in pthread_mutex_lock().  When there are multiple threads contending for
> a lock, one thread will (approximately) always obtain the lock, while
> the others will starve.  With atomics support in place, the probability
> of obtaining the lock is roughly evenly distributed among all the
> threads.  So any workload in which multiple threads may contend for a
> lock should be a candidate to demonstrate this problem in the real
> world.

Does this behavior affect just one implementation with LSE, or also
implementations without LSE?

If the latter, we might need a different mutex implementation for
AArch64. 8-(



Bug#91815: Patch to add memusage and memusagestat

2020-04-22 Thread Helmut Grohne
Hi Aurelien and Stephen,

On Wed, Apr 22, 2020 at 12:04:32AM +0200, Aurelien Jarno wrote:
> > The attached patch adds memusage and memusagestat to the libc-bin package.
> > This does mean that the latter becomes dependent on libgd3, so it might be
> > better to add a new memusage package; I can take care of that if the
> > maintainers think it’s better.
> 
> I am not sure we want to add a new dependency for libc-bin, I am sure
> people running embedded systems won't appreciate. Any reason for not
> shipping it in libc-dev-bin instead? For me that looks more like a tool
> to be used for "development". At least the memusagestat is similar to
> the mtrace one that is in libc-dev-bin.
> 
> We also have to make sure that this new build-dependency doesn't break
> bootstrapping. I have added Helmut in Cc so that he can have a look.

Thank you for notifying me. Indeed, the patch doesn't work for
bootstrapping as is. A stage2 build of glibc would start requiring
libgd-dev -> libgd3 -> libc6, but the stage1 libc6 will not be
sufficient to build src:libgd2. It must be possible to build stage2
without that dependency.

(Note that this is going to become more confusing as there is ongoing
work on removing stage1 and maybe renaming stage2, but let's stick to
the current names for now.)

>From a bootstrapping pov, the libgd-dev dependency is similar to the
libselinux-dev dependency, but I notice just now that it is not
correctly annotated with  in Build-Depends!

>From a build profile pov, it is very undesirable to have package
contents change with profiles. Doing so makes it impossible to validate
them using reproducible builds. However, since we need libc6-dev from
stage2 and it depends on libc-dev-bin we cannot skip generating
libc-dev-bin in stage2. I wonder whether it would make sense to add
another binary package for development tools that are not normally
needed for building software. Then we could skip generating that
package. mtrace would be a good candidate to join that package indeed.

Failing that, we'll have to cope with changing package contents for
stage2 and disable the new dependency for stage2 (or some other
profile).

Helmut



Processed: qmake passes unsupported -isystem to gcc

2020-04-22 Thread Debian Bug Tracking System
Processing control commands:

> affects -1 + src:deepin-music
Bug #958479 [qt5-qmake] qmake passes unsupported -isystem to gcc
Added indication that 958479 affects src:deepin-music
> block 798955 by -1
Bug #798955 [src:glibc] Moving glibc headers from /usr/include to 
/usr/include/$(DEB_HOST_MULTIARCH)
798955 was blocked by: 911860 911456 875921 909967 911556 909965 909964 911359 
909969 911491 909966 910055 911358 912422 911393 910322 911455 910097 911489 
912605 910235 911394 911683 857535 910299 911574 910127 857708
798955 was not blocking any bugs.
Added blocking bug(s) of 798955: 958479

-- 
798955: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=798955
958479: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=958479
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems