| Message ID | AM5PR0801MB1668EAA428F59B3F89184FDC83A49@AM5PR0801MB1668.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve performance of IO locks | expand |
ping
Improve performance of recursive IO locks by adding a fast path for
the single-threaded case. To reduce the number of memory accesses for
locking/unlocking, only increment the recursion counter if the lock
is already taken.
On Neoverse V1, a microbenchmark with many small freads improved by
2.9 times. Multithreaded performance improved by 2%.
Passes GLIBC testsuite, OK for commit?
---
diff --git a/sysdeps/nptl/stdio-lock.h b/sysdeps/nptl/stdio-lock.h
index afa0b779c81d7dd915f8edb6c0974e4f231d4e0a..45823cd1629d3e3efecc64a7d07706a6e6de9af1 100644
--- a/sysdeps/nptl/stdio-lock.h
+++ b/sysdeps/nptl/stdio-lock.h
@@ -37,12 +37,18 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t;
#define _IO_lock_lock(_name) \
do { \
void *__self = THREAD_SELF; \
- if ((_name).owner != __self) \
+ if (SINGLE_THREAD_P && (_name).owner == NULL) \
+ { \
+ (_name).lock = LLL_LOCK_INITIALIZER_LOCKED; \
+ (_name).owner = __self; \
+ } \
+ else if ((_name).owner != __self) \
{ \
lll_lock ((_name).lock, LLL_PRIVATE); \
- (_name).owner = __self; \
+ (_name).owner = __self; \
} \
- ++(_name).cnt; \
+ else \
+ ++(_name).cnt; \
} while (0)
#define _IO_lock_trylock(_name) \
@@ -52,10 +58,7 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t;
if ((_name).owner != __self) \
{ \
if (lll_trylock ((_name).lock) == 0) \
- { \
- (_name).owner = __self; \
- (_name).cnt = 1; \
- } \
+ (_name).owner = __self; \
else \
__result = EBUSY; \
} \
@@ -66,11 +69,18 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t;
#define _IO_lock_unlock(_name) \
do { \
- if (--(_name).cnt == 0) \
+ if (SINGLE_THREAD_P && (_name).cnt == 0) \
+ { \
+ (_name).owner = NULL; \
+ (_name).lock = 0; \
+ } \
+ else if ((_name).cnt == 0) \
{ \
- (_name).owner = NULL; \
+ (_name).owner = NULL; \
lll_unlock ((_name).lock, LLL_PRIVATE); \
} \
+ else \
+ --(_name).cnt; \
} while (0)
Sounds good, tested on x86_64. On Mon, Aug 1, 2022 at 7:06 AM Wilco Dijkstra via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > ping > > > Improve performance of recursive IO locks by adding a fast path for > the single-threaded case. To reduce the number of memory accesses for > locking/unlocking, only increment the recursion counter if the lock > is already taken. > > On Neoverse V1, a microbenchmark with many small freads improved by > 2.9 times. Multithreaded performance improved by 2%. > > Passes GLIBC testsuite, OK for commit? > > --- > > diff --git a/sysdeps/nptl/stdio-lock.h b/sysdeps/nptl/stdio-lock.h > index afa0b779c81d7dd915f8edb6c0974e4f231d4e0a..45823cd1629d3e3efecc64a7d07706a6e6de9af1 100644 > --- a/sysdeps/nptl/stdio-lock.h > +++ b/sysdeps/nptl/stdio-lock.h > @@ -37,12 +37,18 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; > #define _IO_lock_lock(_name) \ > do { \ > void *__self = THREAD_SELF; \ > - if ((_name).owner != __self) \ > + if (SINGLE_THREAD_P && (_name).owner == NULL) \ > + { \ > + (_name).lock = LLL_LOCK_INITIALIZER_LOCKED; \ > + (_name).owner = __self; \ > + } \ > + else if ((_name).owner != __self) \ > { \ > lll_lock ((_name).lock, LLL_PRIVATE); \ > - (_name).owner = __self; \ > + (_name).owner = __self; \ > } \ > - ++(_name).cnt; \ > + else \ > + ++(_name).cnt; \ > } while (0) > > #define _IO_lock_trylock(_name) \ > @@ -52,10 +58,7 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; > if ((_name).owner != __self) \ > { \ > if (lll_trylock ((_name).lock) == 0) \ > - { \ > - (_name).owner = __self; \ > - (_name).cnt = 1; \ > - } \ > + (_name).owner = __self; \ > else \ > __result = EBUSY; \ > } \ > @@ -66,11 +69,18 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; > > #define _IO_lock_unlock(_name) \ > do { \ > - if (--(_name).cnt == 0) \ > + if (SINGLE_THREAD_P && (_name).cnt == 0) \ > + { \ > + (_name).owner = NULL; \ > + (_name).lock = 0; \ > + } \ > + else if ((_name).cnt == 0) \ > { \ > - (_name).owner = NULL; \ > + (_name).owner = NULL; \ > lll_unlock ((_name).lock, LLL_PRIVATE); \ > } \ > + else \ > + --(_name).cnt; \ > } while (0) > >
Hi, On Mon, Aug 01, 2022 at 11:06:07AM +0000, Wilco Dijkstra via Libc-alpha wrote: > Improve performance of recursive IO locks by adding a fast path for > the single-threaded case. To reduce the number of memory accesses for > locking/unlocking, only increment the recursion counter if the lock > is already taken. > > On Neoverse V1, a microbenchmark with many small freads improved by > 2.9 times. Multithreaded performance improved by 2%. Strangely this seems to have broken the glibc-debian-ppc64 buildbot: https://builder.sourceware.org/buildbot/#/builders/glibc-debian-ppc64 I don't see how this commit can cause a failure just on debian-ppc64 (all other distro/arches are fine). And the corresponding bunsen test results don't really show why. https://builder.sourceware.org/testrun/498f51f327afdd7030516455b709a31a0038b432 Most of the .out files are empty, but some indicate an segmentation fault. Comparing to the build before only shows test result diffs, no configuration differences. https://builder.sourceware.org/testrun-diffs?commitish=498f51f327afdd7030516455b709a31a0038b432&commitish=58fd9d63b078b6bbfdba45135c4021038f33534e I don't have access to the buildbot, so cannot easily investigate more. Tom, could you have a look and see if you can find out more? Does just reverting commit c51c483d2b8ae66fe31a12509aedae02a6982ced make things OK again? Or did something else on the buildbot worker change? Note that the buildbot only does a make subdirs=elf check, not a full make check. Thanks, Mark
Hi Mark, Mark Wielaard <mark@klomp.org> writes: > On Mon, Aug 01, 2022 at 11:06:07AM +0000, Wilco Dijkstra via Libc-alpha wrote: >> Improve performance of recursive IO locks by adding a fast path for >> the single-threaded case. To reduce the number of memory accesses for >> locking/unlocking, only increment the recursion counter if the lock >> is already taken. >> >> On Neoverse V1, a microbenchmark with many small freads improved by >> 2.9 times. Multithreaded performance improved by 2%. > > Strangely this seems to have broken the glibc-debian-ppc64 buildbot: > https://builder.sourceware.org/buildbot/#/builders/glibc-debian-ppc64 > > I don't see how this commit can cause a failure just on debian-ppc64 > (all other distro/arches are fine). > > And the corresponding bunsen test results don't really show why. > https://builder.sourceware.org/testrun/498f51f327afdd7030516455b709a31a0038b432 > Most of the .out files are empty, but some indicate an segmentation fault. > > Comparing to the build before only shows test result diffs, no > configuration differences. > https://builder.sourceware.org/testrun-diffs?commitish=498f51f327afdd7030516455b709a31a0038b432&commitish=58fd9d63b078b6bbfdba45135c4021038f33534e > > I don't have access to the buildbot, so cannot easily investigate more. > > Tom, could you have a look and see if you can find out more? Does just > reverting commit c51c483d2b8ae66fe31a12509aedae02a6982ced make things > OK again? Yes reverting that commit, the result is: Summary of test results: 317 PASS 10 UNSUPPORTED 2 XPASS Without the reversion: Summary of test results: 256 FAIL 68 PASS 3 UNSUPPORTED 2 XFAIL I looked at elf/unload as an example; it's segfaulting in _dl_relocate_object, backtrace attached; not sure what else to check. > Or did something else on the buildbot worker change? I didn't change anything. > Note that the buildbot only does a make subdirs=elf check, not a full > make check. Thanks, Thomas buildbot@debian-ppc64-builder:~/workers/wildebeest/glibc-debian-ppc64/build$ env GCONV_PATH=/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/iconvdata LOCPATH=/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/localedata LC_ALL=C gdb --args /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf/ld64.so.1 --library-path /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/math:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/dlfcn:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/nss:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/nis:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/rt:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/resolv:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/mathvec:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/support:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/crypt:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/nptl /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf/unload GNU gdb (Debian 10.1-2+b1) 10.1.90.20210103-git Copyright (C) 2021 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "powerpc64-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <https://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf/ld64.so.1... (gdb) r Starting program: /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf/ld64.so.1 --library-path /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/math:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/dlfcn:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/nss:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/nis:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/rt:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/resolv:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/mathvec:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/support:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/crypt:/var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/nptl /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf/unload Program received signal SIGSEGV, Segmentation fault. 0x00007ffff7fabad8 in _dl_relocate_object (l=0x7ffff7ff2eb0, scope=0x7ffff7ff3250, reloc_mode=<optimized out>, consider_profiling=<optimized out>) at dl-reloc.c:301 301 ELF_DYNAMIC_RELOCATE (l, scope, lazy, consider_profiling, skip_ifunc); (gdb) bt #0 0x00007ffff7fabad8 in _dl_relocate_object (l=0x7ffff7ff2eb0, scope=0x7ffff7ff3250, reloc_mode=<optimized out>, consider_profiling=<optimized out>) at dl-reloc.c:301 #1 0x00007ffff7fc3c1c in dl_main (phdr=<optimized out>, phnum=<optimized out>, user_entry=<optimized out>, auxv=<optimized out>) at rtld.c:2314 #2 0x00007ffff7fbef40 in _dl_sysdep_start (start_argptr=<optimized out>, dl_main=@0x7ffff7fef840: 0x7ffff7fc1ba0 <dl_main>) at ../sysdeps/unix/sysv/linux/dl-sysdep.c:140 #3 0x00007ffff7fc0a30 in _dl_start_final (arg=arg@entry=0x7ffffffff6c0, info=info@entry=0x7ffffffff0a0) at rtld.c:497 #4 0x00007ffff7fc16f4 in _dl_start (arg=0x7ffffffff6c0) at rtld.c:586 #5 0x00007ffff7fbfcb0 in ._start () from /var/lib/buildbot/workers/wildebeest/glibc-debian-ppc64/build/elf/ld64.so.1 (gdb)
* Thomas Fitzsimmons: > Hi Mark, > > Mark Wielaard <mark@klomp.org> writes: > >> On Mon, Aug 01, 2022 at 11:06:07AM +0000, Wilco Dijkstra via Libc-alpha wrote: >>> Improve performance of recursive IO locks by adding a fast path for >>> the single-threaded case. To reduce the number of memory accesses for >>> locking/unlocking, only increment the recursion counter if the lock >>> is already taken. >>> >>> On Neoverse V1, a microbenchmark with many small freads improved by >>> 2.9 times. Multithreaded performance improved by 2%. >> >> Strangely this seems to have broken the glibc-debian-ppc64 buildbot: >> https://builder.sourceware.org/buildbot/#/builders/glibc-debian-ppc64 >> >> I don't see how this commit can cause a failure just on debian-ppc64 >> (all other distro/arches are fine). >> >> And the corresponding bunsen test results don't really show why. >> https://builder.sourceware.org/testrun/498f51f327afdd7030516455b709a31a0038b432 >> Most of the .out files are empty, but some indicate an segmentation fault. >> >> Comparing to the build before only shows test result diffs, no >> configuration differences. >> https://builder.sourceware.org/testrun-diffs?commitish=498f51f327afdd7030516455b709a31a0038b432&commitish=58fd9d63b078b6bbfdba45135c4021038f33534e >> >> I don't have access to the buildbot, so cannot easily investigate more. >> >> Tom, could you have a look and see if you can find out more? Does just >> reverting commit c51c483d2b8ae66fe31a12509aedae02a6982ced make things >> OK again? > > Yes reverting that commit, the result is: > > Summary of test results: > 317 PASS > 10 UNSUPPORTED > 2 XPASS > > Without the reversion: > > Summary of test results: > 256 FAIL > 68 PASS > 3 UNSUPPORTED > 2 XFAIL > > I looked at elf/unload as an example; it's segfaulting in > _dl_relocate_object, backtrace attached; not sure what else to check. I don't see this on powerpc64, with a toolchain based on GCC 8.2 and binutils 2.30. I'm at a loss how these things could be related. Thanks, Florian
Hi, On Tue, Aug 16, 2022 at 09:31:49AM +0200, Florian Weimer wrote: > > Mark Wielaard <mark@klomp.org> writes: > >> On Mon, Aug 01, 2022 at 11:06:07AM +0000, Wilco Dijkstra via Libc-alpha wrote: > >>> Improve performance of recursive IO locks by adding a fast path for > >>> the single-threaded case. To reduce the number of memory accesses for > >>> locking/unlocking, only increment the recursion counter if the lock > >>> is already taken. > >>> > >>> On Neoverse V1, a microbenchmark with many small freads improved by > >>> 2.9 times. Multithreaded performance improved by 2%. > >> > >> Strangely this seems to have broken the glibc-debian-ppc64 buildbot: > >> https://builder.sourceware.org/buildbot/#/builders/glibc-debian-ppc64 > >> > >> I don't see how this commit can cause a failure just on debian-ppc64 > >> (all other distro/arches are fine). > >> > >> And the corresponding bunsen test results don't really show why. > >> https://builder.sourceware.org/testrun/498f51f327afdd7030516455b709a31a0038b432 > >> Most of the .out files are empty, but some indicate an segmentation fault. > >> > >> Comparing to the build before only shows test result diffs, no > >> configuration differences. > >> https://builder.sourceware.org/testrun-diffs?commitish=498f51f327afdd7030516455b709a31a0038b432&commitish=58fd9d63b078b6bbfdba45135c4021038f33534e > >> > >> I don't have access to the buildbot, so cannot easily investigate more. > >> > >> Tom, could you have a look and see if you can find out more? Does just > >> reverting commit c51c483d2b8ae66fe31a12509aedae02a6982ced make things > >> OK again? > > > > Yes reverting that commit, the result is: > > > > Summary of test results: > > 317 PASS > > 10 UNSUPPORTED > > 2 XPASS > > > > Without the reversion: > > > > Summary of test results: > > 256 FAIL > > 68 PASS > > 3 UNSUPPORTED > > 2 XFAIL > > > > I looked at elf/unload as an example; it's segfaulting in > > _dl_relocate_object, backtrace attached; not sure what else to check. > > I don't see this on powerpc64, with a toolchain based on GCC 8.2 and > binutils 2.30. I'm at a loss how these things could be related. Yeah, I am actually surprised just reverting the patch fixed things. But looking at the bunsen data for glibc-debian-ppc64 does seem to show that the only thing changed is this particular patch. Runs before it pass, runs after show lots of fails. https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc-debian-ppc64%25 The debian-ppc64 worker has GCC 11.2.0 and binutils 2.38 https://builder.sourceware.org/buildbot/#/workers/10 But I don't see how the patch and the crash backtrace are related. Cheers, Mark
* Mark Wielaard: > Yeah, I am actually surprised just reverting the patch fixed things. > But looking at the bunsen data for glibc-debian-ppc64 does seem to > show that the only thing changed is this particular patch. Runs before > it pass, runs after show lots of fails. > > https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc-debian-ppc64%25 How do I read this table? > The debian-ppc64 worker has GCC 11.2.0 and binutils 2.38 > https://builder.sourceware.org/buildbot/#/workers/10 > > But I don't see how the patch and the crash backtrace are related. It could be a change in relocation types used. Thanks, Florian
Hi, >> I looked at elf/unload as an example; it's segfaulting in >> _dl_relocate_object, backtrace attached; not sure what else to check. > > I don't see this on powerpc64, with a toolchain based on GCC 8.2 and > binutils 2.30. I'm at a loss how these things could be related. I would not expect the dynamic linker to have changed at all, so one option is to check the binary is identical before/after my commit. If the dynamic linker somehow got some uses of SINGLE_THREAD_P then that might access TLS before it is setup. The other possibility is that the binary it is trying to link has corrupted relocations. It's hard to imagine how that could happen unless you use the new GLIBC to link an application and fileio fails to write out the data for the relocations. Cheers, Wilco
It's working fine here on openSUSE Tumbleweed: https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:glibc/glibc:testsuite/p/ppc64
Hi Wilco, Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes: >>> I looked at elf/unload as an example; it's segfaulting in >>> _dl_relocate_object, backtrace attached; not sure what else to check. >> >> I don't see this on powerpc64, with a toolchain based on GCC 8.2 and >> binutils 2.30. I'm at a loss how these things could be related. > > I would not expect the dynamic linker to have changed at all, so one > option is to check the binary is identical before/after my commit. If > the dynamic linker somehow got some uses of SINGLE_THREAD_P then that > might access TLS before it is setup. elf/ld64.so.1 is identical before and after your change. > The other possibility is that the binary it is trying to link has > corrupted relocations. It's hard to imagine how that could happen > unless you use the new GLIBC to link an application and fileio fails > to write out the data for the relocations. I don't know how to check this. I'll contact you off-list to set up temporary remote access for you. Thomas
Hi, Thomas Fitzsimmons <fitzsim@fitzsim.org> writes: > Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes: > >>>> I looked at elf/unload as an example; it's segfaulting in >>>> _dl_relocate_object, backtrace attached; not sure what else to check. >>> >>> I don't see this on powerpc64, with a toolchain based on GCC 8.2 and >>> binutils 2.30. I'm at a loss how these things could be related. >> >> I would not expect the dynamic linker to have changed at all, so one >> option is to check the binary is identical before/after my commit. If >> the dynamic linker somehow got some uses of SINGLE_THREAD_P then that >> might access TLS before it is setup. > > elf/ld64.so.1 is identical before and after your change. > >> The other possibility is that the binary it is trying to link has >> corrupted relocations. It's hard to imagine how that could happen >> unless you use the new GLIBC to link an application and fileio fails >> to write out the data for the relocations. > > I don't know how to check this. I'll contact you off-list to set up > temporary remote access for you. Before setting this up, I checked the system's package list. This builder is running Debian unstable, without unattended upgrades, so some of the toolchain packages were out-of-date. I did an upgrade (388 packages) and the issue seems to have disappeared, so this was likely a toolchain bug that has since been fixed. I have the list of old and new package versions if anyone's interested. Anyway, I think glibc-debian-ppc64 builds will succeed now. Thomas
Hi Tom. On Tue, Aug 16, 2022 at 10:31:39AM -0400, Thomas Fitzsimmons wrote: > Before setting this up, I checked the system's package list. This > builder is running Debian unstable, without unattended upgrades, so some > of the toolchain packages were out-of-date. > > I did an upgrade (388 packages) and the issue seems to have disappeared, > so this was likely a toolchain bug that has since been fixed. I have > the list of old and new package versions if anyone's interested. > > Anyway, I think glibc-debian-ppc64 builds will succeed now. It did! https://builder.sourceware.org/buildbot/#builders/128/builds/182 Thanks, Mark
Hi Florian, On Tue, Aug 16, 2022 at 12:08:03PM +0200, Florian Weimer wrote: > * Mark Wielaard: > > Yeah, I am actually surprised just reverting the patch fixed things. > > But looking at the bunsen data for glibc-debian-ppc64 does seem to > > show that the only thing changed is this particular patch. Runs before > > it pass, runs after show lots of fails. > > > > https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc-debian-ppc64%25 > > How do I read this table? Glad you ask! Although I think the mystery of the debian-ppc64 build failure has now been solved, the intent of the bunsenweb tool is to help figure out test result differences between builders. I have added Frank to the CC who can explain better than me. We'll also have a BoF at the GNU Tools Cauldron for suggestions on how the improve it and the integration with other sourceware services [*]. So the table above is all the builds that bunsen knows about for glibc-debian-ppc64. Note that currently we only have one ppc64[be] worker that does glibc builds. You can select a couple of builds and compare them. Pick the failing one, which was 498f51, and the last succeeding one. And compare the results, configs, etc. There are a couple of ways to explore the diffs. One is by looking at the buildbot "upload to bunsen" step and select that URL (this really should be improved to make this easier to find). This then gives you different "clusters" of similar (but possible different) other builds to compare to. e.g. look at https://builder.sourceware.org/testrun/498f51f327afdd7030516455b709a31a0038b432 Another is to look at all results for a particular builder (as above) or to look at all the glibc builds using the (sql) search form. e.g. https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc%25 And then pick various results by hand and hit "diff". The diffs are split up between groups of results, like the glibc test results or autoconf logs, etc. Clicking through will give you the actual config.log, the results.out files, etc. Cheers, Mark [*] https://gcc.gnu.org/wiki/cauldron2022#cauldron2022talks.Sourceware_GNU_Toolchain_Infrastructure_and_beyond
* Mark Wielaard: > Hi Florian, > > On Tue, Aug 16, 2022 at 12:08:03PM +0200, Florian Weimer wrote: >> * Mark Wielaard: >> > Yeah, I am actually surprised just reverting the patch fixed things. >> > But looking at the bunsen data for glibc-debian-ppc64 does seem to >> > show that the only thing changed is this particular patch. Runs before >> > it pass, runs after show lots of fails. >> > >> > https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc-debian-ppc64%25 >> >> How do I read this table? > > Glad you ask! Although I think the mystery of the debian-ppc64 build > failure has now been solved, the intent of the bunsenweb tool is to > help figure out test result differences between builders. I have added > Frank to the CC who can explain better than me. We'll also have a BoF > at the GNU Tools Cauldron for suggestions on how the improve it and > the integration with other sourceware services [*]. > > So the table above is all the builds that bunsen knows about for > glibc-debian-ppc64. Note that currently we only have one ppc64[be] > worker that does glibc builds. > > You can select a couple of builds and compare them. Pick the failing > one, which was 498f51, and the last succeeding one. And compare the > results, configs, etc. > There are a couple of ways to explore the diffs. One is by looking at > the buildbot "upload to bunsen" step and select that URL (this really > should be improved to make this easier to find). This then gives you > different "clusters" of similar (but possible different) other builds > to compare to. e.g. look at > https://builder.sourceware.org/testrun/498f51f327afdd7030516455b709a31a0038b432 > > Another is to look at all results for a particular builder (as above) > or to look at all the glibc builds using the (sql) search form. e.g. > https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc%25 Thank you for these links, they surely look useful. I assume the zebra pattern in the table is not actually meaningful (it's too regular for that), it's just to improve readability, right? What confuses me still is that the run lists do not seem to have numbers with failure counts. Thanks, Florian
Hi - > > Another is to look at all results for a particular builder (as above) > > or to look at all the glibc builds using the (sql) search form. e.g. > > https://builder.sourceware.org/testruns/?has_keyvalue_like_k=testrun.git_describe&has_keyvalue_like_v=%25glibc%25 > > Thank you for these links, they surely look useful. I assume the zebra > pattern in the table is not actually meaningful (it's too regular for > that), it's just to improve readability, right? Correct. > What confuses me still is that the run lists do not seem to have numbers > with failure counts. Yes, while one can view fail/etc. counts of all the test cases in one testrun [https://tinyurl.com/mry25ekt], there is no single view that contrasts such counts for multiple runs. It's coming. - FChE
diff --git a/sysdeps/nptl/stdio-lock.h b/sysdeps/nptl/stdio-lock.h index afa0b779c81d7dd915f8edb6c0974e4f231d4e0a..45823cd1629d3e3efecc64a7d07706a6e6de9af1 100644 --- a/sysdeps/nptl/stdio-lock.h +++ b/sysdeps/nptl/stdio-lock.h @@ -37,12 +37,18 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; #define _IO_lock_lock(_name) \ do { \ void *__self = THREAD_SELF; \ - if ((_name).owner != __self) \ + if (SINGLE_THREAD_P && (_name).owner == NULL) \ + { \ + (_name).lock = LLL_LOCK_INITIALIZER_LOCKED; \ + (_name).owner = __self; \ + } \ + else if ((_name).owner != __self) \ { \ lll_lock ((_name).lock, LLL_PRIVATE); \ - (_name).owner = __self; \ + (_name).owner = __self; \ } \ - ++(_name).cnt; \ + else \ + ++(_name).cnt; \ } while (0) #define _IO_lock_trylock(_name) \ @@ -52,10 +58,7 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; if ((_name).owner != __self) \ { \ if (lll_trylock ((_name).lock) == 0) \ - { \ - (_name).owner = __self; \ - (_name).cnt = 1; \ - } \ + (_name).owner = __self; \ else \ __result = EBUSY; \ } \ @@ -66,11 +69,18 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; #define _IO_lock_unlock(_name) \ do { \ - if (--(_name).cnt == 0) \ + if (SINGLE_THREAD_P && (_name).cnt == 0) \ + { \ + (_name).owner = NULL; \ + (_name).lock = 0; \ + } \ + else if ((_name).cnt == 0) \ { \ - (_name).owner = NULL; \ + (_name).owner = NULL; \ lll_unlock ((_name).lock, LLL_PRIVATE); \ } \ + else \ + --(_name).cnt; \ } while (0)