Gnulib’s ‘test-lock’ fails to complete on machines with >= 32 cores

  • Done
  • quality assurance status badge
Details
4 participants
  • Bruno Haible
  • Ludovic Courtès
  • Mathieu Othacehe
  • Marius Bakke
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important
L
L
Ludovic Courtès wrote on 11 Apr 2017 10:16
Gnulib’s ‘test-lock’ fails to complete on machines with >= 32 cores
(address . bug-guix@gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
871sszuznt.fsf@gnu.org
With current ‘master’, Gnulib’s ‘test-lock’ (a test found in many GNU
packages that use Gnulib, such as gettext-0.19.8.1) fails to complete in
1 hour almost systematically on bayfront, which is a 32-core x86_64
machine. The backtrace is like this:

Toggle snippet (133 lines)
(gdb) thread apply all bt

Thread 21 (LWP 5636):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 20 (LWP 5635):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 19 (LWP 5634):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 18 (LWP 5633):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 17 (LWP 5632):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 16 (LWP 5631):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 15 (LWP 5630):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 14 (LWP 5629):
#0 0x00007ffff77a93d6 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 13 (LWP 5628):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 12 (LWP 5627):
#0 0x00007ffff77a9602 in pthread_rwlock_wrlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x000000000040172f in rwlock_mutator_thread (arg=<optimized out>) at test-lock.c:348
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 11 (LWP 5626):
#0 0x00007ffff74cf777 in sched_yield () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6
#1 0x0000000000401810 in gl_thread_yield () at test-lock.c:104
#2 rwlock_checker_thread (arg=<optimized out>) at test-lock.c:381
#3 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#4 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 10 (LWP 5625):
#0 0x00007ffff74cf777 in sched_yield () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6
#1 0x0000000000401810 in gl_thread_yield () at test-lock.c:104
#2 rwlock_checker_thread (arg=<optimized out>) at test-lock.c:381
#3 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#4 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 9 (LWP 5624):
#0 0x00007ffff77a9dd7 in pthread_rwlock_unlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000401807 in rwlock_checker_thread (arg=<optimized out>) at test-lock.c:378
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 8 (LWP 5623):
#0 0x00007ffff77a9dd7 in pthread_rwlock_unlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000401807 in rwlock_checker_thread (arg=<optimized out>) at test-lock.c:378
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 7 (LWP 5622):
#0 0x00007ffff77a9dd7 in pthread_rwlock_unlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000401807 in rwlock_checker_thread (arg=<optimized out>) at test-lock.c:378
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 6 (LWP 5621):
#0 0x00007ffff77a9dd7 in pthread_rwlock_unlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000401807 in rwlock_checker_thread (arg=<optimized out>) at test-lock.c:378
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 5 (LWP 5620):
#0 0x00007ffff74cf777 in sched_yield () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6
#1 0x0000000000401810 in gl_thread_yield () at test-lock.c:104
#2 rwlock_checker_thread (arg=<optimized out>) at test-lock.c:381
#3 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#4 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 4 (LWP 5619):
#0 0x00007ffff74cf777 in sched_yield () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6
#1 0x0000000000401810 in gl_thread_yield () at test-lock.c:104
#2 rwlock_checker_thread (arg=<optimized out>) at test-lock.c:381
#3 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#4 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 3 (LWP 5618):
#0 0x00007ffff77a9dd7 in pthread_rwlock_unlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000401807 in rwlock_checker_thread (arg=<optimized out>) at test-lock.c:378
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 2 (LWP 5617):
#0 0x00007ffff77a9dd7 in pthread_rwlock_unlock () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000401807 in rwlock_checker_thread (arg=<optimized out>) at test-lock.c:378
#2 0x00007ffff77a4454 in start_thread () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#3 0x00007ffff74e67bf in clone () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libc.so.6

Thread 1 (LWP 5605):
#0 0x00007ffff77a568d in pthread_join () from target:/gnu/store/rmjlycdgiq8pfy5hfi42qhw3k7p6kdav-glibc-2.25/lib/libpthread.so.0
#1 0x0000000000400e29 in gl_thread_join (retvalp=0x0, thread=<optimized out>) at test-lock.c:99
#2 test_rwlock () at test-lock.c:408
#3 main () at test-lock.c:682

Mathieu Othacehe reported the same issue on a 32-core machine.

This may be fixed by Gnulib commit
480d374e596a0ee3fed168ab42cd84c313ad3c89 (not present in
gettext-0.19.8.1), which introduces this:

--- a/tests/test-lock.c
+++ b/tests/test-lock.c
@@ -50,6 +50,13 @@
Uncomment this to see if the operating system has a fair scheduler. */
#define EXPLICIT_YIELD 1

+/* Whether to use 'volatile' on some variables that communicate information
+ between threads. If set to 0, a lock is used to protect these variables.
+ If set to 1, 'volatile' is used; this is theoretically equivalent but can
+ lead to much slower execution (e.g. 30x slower total run time on a 40-core
+ machine. */
+#define USE_VOLATILE 0

30x slower could exceed the default max-silent-timeout, which is 1 hour.

Ludo’.
M
M
Mathieu Othacehe wrote on 12 Apr 2017 11:01
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . bug-guix@gnu.org)
86shleatk3.fsf@gmail.com
Hi,

Toggle quote (3 lines)
> This may be fixed by Gnulib commit
> 480d374e596a0ee3fed168ab42cd84c313ad3c89 (not present in

On gettext, commit 1afbcb06fded2a427b761dd1615b1e48e1e853cc seems to fix
the problem. I ran three consecutive tests :

Toggle snippet (31 lines)
mathieu@elbruz /tmp/guix-build-gettext-next-0.19.8.1.drv-0/gettext-0.19.8.1$ time ./gettext-tools/gnulib-tests/.libs/test-lock
Starting test_lock ... OK
Starting test_rwlock ... OK
Starting test_recursive_lock ... OK
Starting test_once ... OK

real 3m50.493s
user 31m15.128s
sys 6m15.440s

mathieu@elbruz /tmp/guix-build-gettext-next-0.19.8.1.drv-0/gettext-0.19.8.1$ time ./gettext-tools/gnulib-tests/.libs/test-lock
Starting test_lock ... OK
Starting test_rwlock ... OK
Starting test_recursive_lock ... OK
Starting test_once ... OK

real 4m11.575s
user 34m10.304s
sys 6m46.288s

mathieu@elbruz /tmp/guix-build-gettext-next-0.19.8.1.drv-0/gettext-0.19.8.1$ time ./gettext-tools/gnulib-tests/.libs/test-lock
Starting test_lock ... OK
Starting test_rwlock ... OK
Starting test_recursive_lock ... OK
Starting test_once ... OK

real 3m21.155s
user 27m6.328s
sys 5m27.248s

Tests succeeds in a reasonable time and it can be backported to our
version of gettext.

However, I don't fully understand the difference between commit
1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext and commit
480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib.

Mathieu
L
L
Ludovic Courtès wrote on 12 Apr 2017 13:53
Re: bug#26441: Gnulib’s ‘test-lock ’ fails to complete on machines with >= 32 cores
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
878tn5et9m.fsf@gnu.org
ludo@gnu.org (Ludovic Courtès) skribis:

Toggle quote (19 lines)
> This may be fixed by Gnulib commit
> 480d374e596a0ee3fed168ab42cd84c313ad3c89 (not present in
> gettext-0.19.8.1), which introduces this:
>
> --- a/tests/test-lock.c
> +++ b/tests/test-lock.c
> @@ -50,6 +50,13 @@
> Uncomment this to see if the operating system has a fair scheduler. */
> #define EXPLICIT_YIELD 1
>
> +/* Whether to use 'volatile' on some variables that communicate information
> + between threads. If set to 0, a lock is used to protect these variables.
> + If set to 1, 'volatile' is used; this is theoretically equivalent but can
> + lead to much slower execution (e.g. 30x slower total run time on a 40-core
> + machine. */
> +#define USE_VOLATILE 0
>
> 30x slower could exceed the default max-silent-timeout, which is 1 hour.

Maybe not: building with --max-silent-time=18000 didn’t help…

Ludo’.
L
L
Ludovic Courtès wrote on 12 Apr 2017 14:00
(name . Bruno Haible)(address . bruno@clisp.org)
87wpapdedp.fsf@gnu.org
Hello Gnulib,

The ‘test-lock’ test as shipped with gettext-0.19.8.1 fails to complete
(or hangs?) on machines with 32+ cores, as reported here:


Does that ring a bell?

Mathieu Othacehe found that this is fixed by gettext commit
1afbcb06fded2a427b761dd1615b1e48e1e853cc but writes:

Toggle quote (4 lines)
> However, I don't fully understand the difference between commit
> 1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext and commit
> 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib.

Could you clarify this?

Thanks in advance,
Ludo’.
B
B
Bruno Haible wrote on 12 Apr 2017 16:37
Re: bug#26441: Gnulib’s ‘test-lock’ fails to complete on machines with >= 32 cores
(name . Ludovic Courtès)(address . ludo@gnu.org)
2008088.TpEHzIiNi8@omega
Hi Ludo,

Toggle quote (7 lines)
> The ‘test-lock’ test as shipped with gettext-0.19.8.1 fails to complete
> (or hangs?) on machines with 32+ cores, as reported here:
>
> https://bugs.gnu.org/26441
>
> Does that ring a bell?

Yes, this is the bug that was investigated in December 2016 [1] and
subsequently fixed [2].

Toggle quote (7 lines)
> Mathieu Othacehe found that this is fixed by gettext commit
> 1afbcb06fded2a427b761dd1615b1e48e1e853cc but writes:
>
> > However, I don't fully understand the difference between commit
> > 1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext and commit
> > 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib.

gettext contains two copies of test-lock.c, one from gnulib and
a another one that doesn't use gnulib.
The commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib fixed
the first one.
The commit 1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext fixed
the second one, in the same way.

Bruno

L
L
Ludovic Courtès wrote on 12 Apr 2017 18:11
Re: bug#26441: Gnulib’s ‘test-lock ’ fails to complete on machines with >= 32 cores
(name . Bruno Haible)(address . bruno@clisp.org)
87h91td2s5.fsf@gnu.org
Hi Bruno,

Bruno Haible <bruno@clisp.org> skribis:

Toggle quote (24 lines)
>> The ‘test-lock’ test as shipped with gettext-0.19.8.1 fails to complete
>> (or hangs?) on machines with 32+ cores, as reported here:
>>
>> https://bugs.gnu.org/26441
>>
>> Does that ring a bell?
>
> Yes, this is the bug that was investigated in December 2016 [1] and
> subsequently fixed [2].
>
>> Mathieu Othacehe found that this is fixed by gettext commit
>> 1afbcb06fded2a427b761dd1615b1e48e1e853cc but writes:
>>
>> > However, I don't fully understand the difference between commit
>> > 1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext and commit
>> > 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib.
>
> gettext contains two copies of test-lock.c, one from gnulib and
> a another one that doesn't use gnulib.
> The commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib fixed
> the first one.
> The commit 1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext fixed
> the second one, in the same way.

OK, thanks for the explanation. I guess we’ll borrow the relevant
patches.

Ludo’.
M
M
Mathieu Othacehe wrote on 13 Apr 2017 08:39
86poggbykk.fsf@gmail.com
Hi,

Here's a patch for gettext, to solve test-lock performance issues.

While rebuilding all gettext dependencies, I noticed that there are
other programs impacted by this bug :

* libunistring
* coreutils
* findutils

But the list may be bigger :


Those users are embedding gnulib source code (via git submodules for
example), so I'm not sure what to do here ...

We can consider that it's not such a big deal and ignore this problem
waiting for programs to integrate the fix, or we can patch them on
core-updates.

WDYT ?

Thank you,

Mathieu
Attachment: 0001-gnu-gettext-Fix-make-check-issues-on-multi-core-mach.patch
L
L
Ludovic Courtès wrote on 13 Apr 2017 14:23
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 26441@debbugs.gnu.org)
87poggqyw3.fsf@gnu.org
Hi Mathieu,

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

Toggle quote (2 lines)
> Here's a patch for gettext, to solve test-lock performance issues.

Awesome.

Toggle quote (7 lines)
> While rebuilding all gettext dependencies, I noticed that there are
> other programs impacted by this bug :
>
> * libunistring
> * coreutils
> * findutils

Coreutils 8.27 was released very recently so it probably already has the
‘test-lock’ fix. But yeah, there are many other GNU packages that
possibly include it, although not all of them depend on this specific
Gnulib module.

Toggle quote (6 lines)
> We can consider that it's not such a big deal and ignore this problem
> waiting for programs to integrate the fix, or we can patch them on
> core-updates.
>
> WDYT ?

Maybe we can simply patch the important packages (findutils,
libunistring) and wait and see if others need the patch. Not ideal, but
I can’t think of a better way.

Toggle quote (10 lines)
> From e4ad9aa61fa75afa4417616de36cd25e9631fd67 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <m.othacehe@gmail.com>
> Date: Wed, 12 Apr 2017 21:17:24 +0200
> Subject: [PATCH] gnu: gettext: Fix make check issues on multi-core machines.
>
> * gnu/packages/patches/gettext-gnulib-multi-core.patch: New file.
> * gnu/packages/patches/gettext-multi-core.patch: New file.
> * gnu/packages/gettext.scm (gettext-minimal)[patches]: Add a reference
> to the two previous patches.

[...]

Toggle quote (7 lines)
> --- /dev/null
> +++ b/gnu/packages/patches/gettext-gnulib-multi-core.patch
> @@ -0,0 +1,176 @@
> +This patch fixes performance problems on multi-core machines.
> +See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib
> +from Bruno Haible <bruno@clisp.org>.

I added the reference to https://bugs.gnu.org/26441 and pushed.

Thank you!

Ludo’.
M
M
Mathieu Othacehe wrote on 13 Apr 2017 18:31
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 26441@debbugs.gnu.org)
871sswjmlj.fsf@gmail.com
Toggle quote (4 lines)
> Maybe we can simply patch the important packages (findutils,
> libunistring) and wait and see if others need the patch. Not ideal, but
> I can’t think of a better way.

Ok fine for me.

I'll send findutils and libunistring patches for now.

Thanks,

Mathieu
M
M
Mathieu Othacehe wrote on 13 Apr 2017 18:33
[PATCH] gnu: libunistring: Fix make check issues on multi-core machines.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170413163340.17497-1-m.othacehe@gmail.com
* gnu/packages/patches/libunistring-gnulib-multi-core.patch: New file.
* gnu/packages/libunistring.scm (libunistring)[patches]: Add a reference
to previous patch.
---
gnu/packages/libunistring.scm | 8 +-
.../patches/libunistring-gnulib-multi-core.patch | 178 +++++++++++++++++++++
2 files changed, 185 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/libunistring-gnulib-multi-core.patch

Toggle diff (219 lines)
diff --git a/gnu/packages/libunistring.scm b/gnu/packages/libunistring.scm
index 212bec4b4..df02f68ce 100644
--- a/gnu/packages/libunistring.scm
+++ b/gnu/packages/libunistring.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -24,6 +25,7 @@
#:use-module (guix packages)
#:use-module (guix download)
#:use-module (guix build-system gnu)
+ #:use-module (gnu packages)
#:use-module (gnu packages base))
(define-public libunistring
@@ -37,7 +39,11 @@
version ".tar.xz"))
(sha256
(base32
- "15z76qrmrvkc3c6hfq2lzzqysgd21s682f2smycfab5g598n8drf"))))
+ "15z76qrmrvkc3c6hfq2lzzqysgd21s682f2smycfab5g598n8drf"))
+ ;; test-lock has performance issues on multi-core machines,
+ ;; it hangs or takes a long time to complete.
+ ;; This is a commit from gnulib to fix this issue.
+ (patches (search-patches "libunistring-gnulib-multi-core.patch"))))
(propagated-inputs (libiconv-if-needed))
(build-system gnu-build-system)
(arguments
diff --git a/gnu/packages/patches/libunistring-gnulib-multi-core.patch b/gnu/packages/patches/libunistring-gnulib-multi-core.patch
new file mode 100644
index 000000000..709b20c6d
--- /dev/null
+++ b/gnu/packages/patches/libunistring-gnulib-multi-core.patch
@@ -0,0 +1,178 @@
+This patch fixes performance problems on multi-core machines
+as reported at <https://bugs.gnu.org/26441>.
+
+See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in Gnulib
+by Bruno Haible <bruno@clisp.org>.
+
+diff --git a/tests/test-lock.c b/tests/test-lock.c
+index cb734b4e6..aa6de2739 100644
+--- a/tests/test-lock.c
++++ b/tests/test-lock.c
+@@ -50,6 +50,13 @@
+ Uncomment this to see if the operating system has a fair scheduler. */
+ #define EXPLICIT_YIELD 1
+
++/* Whether to use 'volatile' on some variables that communicate information
++ between threads. If set to 0, a lock is used to protect these variables.
++ If set to 1, 'volatile' is used; this is theoretically equivalent but can
++ lead to much slower execution (e.g. 30x slower total run time on a 40-core
++ machine. */
++#define USE_VOLATILE 0
++
+ /* Whether to print debugging messages. */
+ #define ENABLE_DEBUGGING 0
+
+@@ -103,6 +110,51 @@
+ # define yield()
+ #endif
+
++#if USE_VOLATILE
++struct atomic_int {
++ volatile int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ return ai->value;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ ai->value = new_value;
++}
++#else
++struct atomic_int {
++ gl_lock_define (, lock)
++ int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ gl_lock_init (ai->lock);
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ gl_lock_lock (ai->lock);
++ int ret = ai->value;
++ gl_lock_unlock (ai->lock);
++ return ret;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ gl_lock_lock (ai->lock);
++ ai->value = new_value;
++ gl_lock_unlock (ai->lock);
++}
++#endif
++
+ #define ACCOUNT_COUNT 4
+
+ static int account[ACCOUNT_COUNT];
+@@ -170,12 +222,12 @@ lock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int lock_checker_done;
++static struct atomic_int lock_checker_done;
+
+ static void *
+ lock_checker_thread (void *arg)
+ {
+- while (!lock_checker_done)
++ while (get_atomic_int_value (&lock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_lock_lock (my_lock);
+@@ -200,7 +252,8 @@ test_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- lock_checker_done = 0;
++ init_atomic_int (&lock_checker_done);
++ set_atomic_int_value (&lock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (lock_checker_thread, NULL);
+@@ -210,7 +263,7 @@ test_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- lock_checker_done = 1;
++ set_atomic_int_value (&lock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
+@@ -254,12 +307,12 @@ rwlock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int rwlock_checker_done;
++static struct atomic_int rwlock_checker_done;
+
+ static void *
+ rwlock_checker_thread (void *arg)
+ {
+- while (!rwlock_checker_done)
++ while (get_atomic_int_value (&rwlock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ());
+ gl_rwlock_rdlock (my_rwlock);
+@@ -284,7 +337,8 @@ test_rwlock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- rwlock_checker_done = 0;
++ init_atomic_int (&rwlock_checker_done);
++ set_atomic_int_value (&rwlock_checker_done, 0);
+
+ /* Spawn the threads. */
+ for (i = 0; i < THREAD_COUNT; i++)
+@@ -295,7 +349,7 @@ test_rwlock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- rwlock_checker_done = 1;
++ set_atomic_int_value (&rwlock_checker_done, 1);
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (checkerthreads[i], NULL);
+ check_accounts ();
+@@ -356,12 +410,12 @@ reclock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int reclock_checker_done;
++static struct atomic_int reclock_checker_done;
+
+ static void *
+ reclock_checker_thread (void *arg)
+ {
+- while (!reclock_checker_done)
++ while (get_atomic_int_value (&reclock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_recursive_lock_lock (my_reclock);
+@@ -386,7 +440,8 @@ test_recursive_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- reclock_checker_done = 0;
++ init_atomic_int (&reclock_checker_done);
++ set_atomic_int_value (&reclock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (reclock_checker_thread, NULL);
+@@ -396,7 +451,7 @@ test_recursive_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- reclock_checker_done = 1;
++ set_atomic_int_value (&reclock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
--
2.12.2
M
M
Mathieu Othacehe wrote on 13 Apr 2017 18:34
[PATCH] gnu: findutils: Fix make check issues on multi-core machines.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170413163408.17571-1-m.othacehe@gmail.com
* gnu/packages/patches/findutils-gnulib-multi-core.patch: New file.
* gnu/packages/base.scm (findutils)[patches]: Add a reference
to the previous patch.
---
gnu/packages/base.scm | 60 +++--
.../patches/findutils-gnulib-multi-core.patch | 294 +++++++++++++++++++++
2 files changed, 327 insertions(+), 27 deletions(-)
create mode 100644 gnu/packages/patches/findutils-gnulib-multi-core.patch

Toggle diff (385 lines)
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index 7bcfd7a2e..e620d9cea 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -8,6 +8,7 @@
;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2017 Rene Saavedra <rennes@openmailbox.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -250,38 +251,43 @@ interactive means to merge two files.")
(define-public findutils
(package
- (name "findutils")
- (version "4.6.0")
- (source (origin
- (method url-fetch)
- (uri (string-append "mirror://gnu/findutils/findutils-"
- version ".tar.gz"))
- (sha256
- (base32
- "178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
- (patches (search-patches "findutils-localstatedir.patch"
- "findutils-test-xargs.patch"))))
- (build-system gnu-build-system)
- (arguments
- `(#:configure-flags (list
- ;; Tell 'updatedb' to write to /var.
- "--localstatedir=/var"
-
- ;; Work around cross-compilation failure. See
- ;; <http://savannah.gnu.org/bugs/?27299#comment1>.
- ,@(if (%current-target-system)
- '("gl_cv_func_wcwidth_works=yes")
- '()))))
- (synopsis "Operating on files matching given criteria")
- (description
- "Findutils supplies the basic file directory searching utilities of the
+ (name "findutils")
+ (version "4.6.0")
+ (source (origin
+ (method url-fetch)
+ (uri (string-append "mirror://gnu/findutils/findutils-"
+ version ".tar.gz"))
+ (sha256
+ (base32
+ "178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
+ (patches (search-patches
+ "findutils-localstatedir.patch"
+ "findutils-test-xargs.patch"
+ ;; test-lock has performance issues on multi-core
+ ;; machines, it hangs or takes a long time to complete.
+ ;; This is a commit from gnulib to fix this issue.
+ "findutils-gnulib-multi-core.patch"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:configure-flags (list
+ ;; Tell 'updatedb' to write to /var.
+ "--localstatedir=/var"
+
+ ;; Work around cross-compilation failure. See
+ ;; <http://savannah.gnu.org/bugs/?27299#comment1>.
+ ,@(if (%current-target-system)
+ '("gl_cv_func_wcwidth_works=yes")
+ '()))))
+ (synopsis "Operating on files matching given criteria")
+ (description
+ "Findutils supplies the basic file directory searching utilities of the
GNU system. It consists of two primary searching utilities: \"find\"
recursively searches for files in a directory according to given criteria and
\"locate\" lists files in a database that match a query. Two auxiliary tools
are included: \"updatedb\" updates the file name database and \"xargs\" may be
used to apply commands with arbitrarily long arguments.")
- (license gpl3+)
- (home-page "https://www.gnu.org/software/findutils/")))
+ (license gpl3+)
+ (home-page "https://www.gnu.org/software/findutils/")))
(define-public coreutils
(package
diff --git a/gnu/packages/patches/findutils-gnulib-multi-core.patch b/gnu/packages/patches/findutils-gnulib-multi-core.patch
new file mode 100644
index 000000000..5a37f4f1f
--- /dev/null
+++ b/gnu/packages/patches/findutils-gnulib-multi-core.patch
@@ -0,0 +1,294 @@
+This patch fixes performance problems on multi-core machines
+as reported at <https://bugs.gnu.org/26441>.
+
+See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in Gnulib
+by Bruno Haible <bruno@clisp.org>.
+
+diff --git a/tests/test-lock.c b/tests/test-lock.c
+index a992f64..fb18dee 100644
+--- a/tests/test-lock.c
++++ b/tests/test-lock.c
+@@ -1,5 +1,5 @@
+ /* Test of locking in multithreaded situations.
+- Copyright (C) 2005, 2008-2015 Free Software Foundation, Inc.
++ Copyright (C) 2005, 2008-2017 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+@@ -50,6 +50,28 @@
+ Uncomment this to see if the operating system has a fair scheduler. */
+ #define EXPLICIT_YIELD 1
+
++/* Whether to use 'volatile' on some variables that communicate information
++ between threads. If set to 0, a semaphore or a lock is used to protect
++ these variables. If set to 1, 'volatile' is used; this is theoretically
++ equivalent but can lead to much slower execution (e.g. 30x slower total
++ run time on a 40-core machine), because 'volatile' does not imply any
++ synchronization/communication between different CPUs. */
++#define USE_VOLATILE 0
++
++#if USE_POSIX_THREADS && HAVE_SEMAPHORE_H
++/* Whether to use a semaphore to communicate information between threads.
++ If set to 0, a lock is used. If set to 1, a semaphore is used.
++ Uncomment this to reduce the dependencies of this test. */
++# define USE_SEMAPHORE 1
++/* Mac OS X provides only named semaphores (sem_open); its facility for
++ unnamed semaphores (sem_init) does not work. */
++# if defined __APPLE__ && defined __MACH__
++# define USE_NAMED_SEMAPHORE 1
++# else
++# define USE_UNNAMED_SEMAPHORE 1
++# endif
++#endif
++
+ /* Whether to print debugging messages. */
+ #define ENABLE_DEBUGGING 0
+
+@@ -90,6 +112,12 @@
+
+ #include "glthread/thread.h"
+ #include "glthread/yield.h"
++#if USE_SEMAPHORE
++# include <errno.h>
++# include <fcntl.h>
++# include <semaphore.h>
++# include <unistd.h>
++#endif
+
+ #if ENABLE_DEBUGGING
+ # define dbgprintf printf
+@@ -103,6 +131,132 @@
+ # define yield()
+ #endif
+
++#if USE_VOLATILE
++struct atomic_int {
++ volatile int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ return ai->value;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ ai->value = new_value;
++}
++#elif USE_SEMAPHORE
++/* This atomic_int implementation can only support the values 0 and 1.
++ It is initially 0 and can be set to 1 only once. */
++# if USE_UNNAMED_SEMAPHORE
++struct atomic_int {
++ sem_t semaphore;
++};
++#define atomic_int_semaphore(ai) (&(ai)->semaphore)
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ sem_init (&ai->semaphore, 0, 0);
++}
++# endif
++# if USE_NAMED_SEMAPHORE
++struct atomic_int {
++ sem_t *semaphore;
++};
++#define atomic_int_semaphore(ai) ((ai)->semaphore)
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ sem_t *s;
++ unsigned int count;
++ for (count = 0; ; count++)
++ {
++ char name[80];
++ /* Use getpid() in the name, so that different processes running at the
++ same time will not interfere. Use ai in the name, so that different
++ atomic_int in the same process will not interfere. Use a count in
++ the name, so that even in the (unlikely) case that a semaphore with
++ the specified name already exists, we can try a different name. */
++ sprintf (name, "test-lock-%lu-%p-%u",
++ (unsigned long) getpid (), ai, count);
++ s = sem_open (name, O_CREAT | O_EXCL, 0600, 0);
++ if (s == SEM_FAILED)
++ {
++ if (errno == EEXIST)
++ /* Retry with a different name. */
++ continue;
++ else
++ {
++ perror ("sem_open failed");
++ abort ();
++ }
++ }
++ else
++ {
++ /* Try not to leave a semaphore hanging around on the file system
++ eternally, if we can avoid it. */
++ sem_unlink (name);
++ break;
++ }
++ }
++ ai->semaphore = s;
++}
++# endif
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ if (sem_trywait (atomic_int_semaphore (ai)) == 0)
++ {
++ if (sem_post (atomic_int_semaphore (ai)))
++ abort ();
++ return 1;
++ }
++ else if (errno == EAGAIN)
++ return 0;
++ else
++ abort ();
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ if (new_value == 0)
++ /* It's already initialized with 0. */
++ return;
++ /* To set the value 1: */
++ if (sem_post (atomic_int_semaphore (ai)))
++ abort ();
++}
++#else
++struct atomic_int {
++ gl_lock_define (, lock)
++ int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ gl_lock_init (ai->lock);
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ gl_lock_lock (ai->lock);
++ int ret = ai->value;
++ gl_lock_unlock (ai->lock);
++ return ret;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ gl_lock_lock (ai->lock);
++ ai->value = new_value;
++ gl_lock_unlock (ai->lock);
++}
++#endif
++
+ #define ACCOUNT_COUNT 4
+
+ static int account[ACCOUNT_COUNT];
+@@ -170,12 +324,12 @@ lock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int lock_checker_done;
++static struct atomic_int lock_checker_done;
+
+ static void *
+ lock_checker_thread (void *arg)
+ {
+- while (!lock_checker_done)
++ while (get_atomic_int_value (&lock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_lock_lock (my_lock);
+@@ -200,7 +354,8 @@ test_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- lock_checker_done = 0;
++ init_atomic_int (&lock_checker_done);
++ set_atomic_int_value (&lock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (lock_checker_thread, NULL);
+@@ -210,7 +365,7 @@ test_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- lock_checker_done = 1;
++ set_atomic_int_value (&lock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
+@@ -254,12 +409,12 @@ rwlock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int rwlock_checker_done;
++static struct atomic_int rwlock_checker_done;
+
+ static void *
+ rwlock_checker_thread (void *arg)
+ {
+- while (!rwlock_checker_done)
++ while (get_atomic_int_value (&rwlock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ());
+ gl_rwlock_rdlock (my_rwlock);
+@@ -284,7 +439,8 @@ test_rwlock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- rwlock_checker_done = 0;
++ init_atomic_int (&rwlock_checker_done);
++ set_atomic_int_value (&rwlock_checker_done, 0);
+
+ /* Spawn the threads. */
+ for (i = 0; i < THREAD_COUNT; i++)
+@@ -295,7 +451,7 @@ test_rwlock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- rwlock_checker_done = 1;
++ set_atomic_int_value (&rwlock_checker_done, 1);
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (checkerthreads[i], NULL);
+ check_accounts ();
+@@ -356,12 +512,12 @@ reclock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int reclock_checker_done;
++static struct atomic_int reclock_checker_done;
+
+ static void *
+ reclock_checker_thread (void *arg)
+ {
+- while (!reclock_checker_done)
++ while (get_atomic_int_value (&reclock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_recursive_lock_lock (my_reclock);
+@@ -386,7 +542,8 @@ test_recursive_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- reclock_checker_done = 0;
++ init_atomic_int (&reclock_checker_done);
++ set_atomic_int_value (&reclock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (reclock_checker_thread, NULL);
+@@ -396,7 +553,7 @@ test_recursive_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- reclock_checker_done = 1;
++ set_atomic_int_value (&reclock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
--
2.11.1
M
M
Marius Bakke wrote on 13 Apr 2017 21:38
Re: bug#26441: [PATCH] gnu: findutils: Fix make check issues on multi-core machines.
8737dc9jy1.fsf@fastmail.com
Hi, thanks for working on these!

[...]
Toggle quote (44 lines)
> (define-public findutils
> (package
> - (name "findutils")
> - (version "4.6.0")
> - (source (origin
> - (method url-fetch)
> - (uri (string-append "mirror://gnu/findutils/findutils-"
> - version ".tar.gz"))
> - (sha256
> - (base32
> - "178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
> - (patches (search-patches "findutils-localstatedir.patch"
> - "findutils-test-xargs.patch"))))
> - (build-system gnu-build-system)
> - (arguments
> - `(#:configure-flags (list
> - ;; Tell 'updatedb' to write to /var.
> - "--localstatedir=/var"
> -
> - ;; Work around cross-compilation failure. See
> - ;; <http://savannah.gnu.org/bugs/?27299#comment1>.
> - ,@(if (%current-target-system)
> - '("gl_cv_func_wcwidth_works=yes")
> - '()))))
> - (synopsis "Operating on files matching given criteria")
> - (description
> - "Findutils supplies the basic file directory searching utilities of the
> + (name "findutils")
> + (version "4.6.0")
> + (source (origin
> + (method url-fetch)
> + (uri (string-append "mirror://gnu/findutils/findutils-"
> + version ".tar.gz"))
> + (sha256
> + (base32
> + "178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
> + (patches (search-patches
> + "findutils-localstatedir.patch"
> + "findutils-test-xargs.patch"
> + ;; test-lock has performance issues on multi-core
> + ;; machines, it hangs or takes a long time to complete.
> + ;; This is a commit from gnulib to fix this issue.
> + "findutils-gnulib-multi-core.patch"))))

Please don't mix large indentation changes with functional changes. It
is really difficult to tell what this commit does. Can you split the
indentation change out in a separate commit?
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAljv08YACgkQoqBt8qM6
VPrICgf/VilyyNjEuaKUjp88q7JOBeMocu9Lby3M9oRiLwOZzSkCsUQUV5BslX0a
YI7zmvamW8LJAD0W7wZZYc35+VaeJhbBUsEHevUMcdazfZXc6Uxwh4nzvcNs3Hdu
hc1vNquIJZd74PbdfqZQlyF+0O0SgnzFYtEC9E4N1F67CFkg5QeigLKZ0ow2rpeu
A+o2en1yuKJTbU+vNSZvR6ZB2n1uvf5AdrGng5Zq5pdeS3FkNEb+hXzDH93ke/W4
A1UdZPyi3jyiB8lchsirwi8C2x4ekOuCDwcz879yzlO51udNuWeHj42SvhIyY44M
FiDZHAjRnH4BjvFgk6tGHyXbv/rnbg==
=EU6l
-----END PGP SIGNATURE-----

M
M
Mathieu Othacehe wrote on 13 Apr 2017 22:04
Re: bug#26441: [PATCH] gnu: findutils: Fix make check issues on multi-core machines.
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 26441@debbugs.gnu.org)
87zifkhy63.fsf@gmail.com
Hi Marius,

Toggle quote (4 lines)
> Please don't mix large indentation changes with functional changes. It
> is really difficult to tell what this commit does. Can you split the
> indentation change out in a separate commit?

Sure, I'll send new patches.

Mathieu
M
M
Mathieu Othacehe wrote on 13 Apr 2017 22:06
[PATCH 1/2] gnu: findutils: Fix make check issues on multi-core machines.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170413200636.8332-1-m.othacehe@gmail.com
* gnu/packages/patches/findutils-gnulib-multi-core.patch: New file.
* gnu/packages/base.scm (findutils)[patches]: Add a reference
to the previous patch.
---
gnu/packages/base.scm | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

Toggle diff (30 lines)
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index 7bcfd7a2e..1ae3c8911 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -8,6 +8,7 @@
;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2017 Rene Saavedra <rennes@openmailbox.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -259,8 +260,13 @@ interactive means to merge two files.")
(sha256
(base32
"178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
- (patches (search-patches "findutils-localstatedir.patch"
- "findutils-test-xargs.patch"))))
+ (patches (search-patches
+ "findutils-localstatedir.patch"
+ "findutils-test-xargs.patch"
+ ;; test-lock has performance issues on multi-core
+ ;; machines, it hangs or takes a long time to complete.
+ ;; This is a commit from gnulib to fix this issue.
+ "findutils-gnulib-multi-core.patch"))))
(build-system gnu-build-system)
(arguments
`(#:configure-flags (list
--
2.12.2
M
M
Mathieu Othacehe wrote on 13 Apr 2017 22:06
[PATCH 2/2] gnu: findutils: Reindent package definition.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170413200636.8332-2-m.othacehe@gmail.com
* gnu/packages/base.scm (findutils): Reindent, no functional change.
---
gnu/packages/base.scm | 64 +++++++++++++++++++++++++--------------------------
1 file changed, 32 insertions(+), 32 deletions(-)

Toggle diff (82 lines)
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index 1ae3c8911..e620d9cea 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -251,43 +251,43 @@ interactive means to merge two files.")
(define-public findutils
(package
- (name "findutils")
- (version "4.6.0")
- (source (origin
- (method url-fetch)
- (uri (string-append "mirror://gnu/findutils/findutils-"
- version ".tar.gz"))
- (sha256
- (base32
- "178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
- (patches (search-patches
- "findutils-localstatedir.patch"
- "findutils-test-xargs.patch"
- ;; test-lock has performance issues on multi-core
- ;; machines, it hangs or takes a long time to complete.
- ;; This is a commit from gnulib to fix this issue.
- "findutils-gnulib-multi-core.patch"))))
- (build-system gnu-build-system)
- (arguments
- `(#:configure-flags (list
- ;; Tell 'updatedb' to write to /var.
- "--localstatedir=/var"
-
- ;; Work around cross-compilation failure. See
- ;; <http://savannah.gnu.org/bugs/?27299#comment1>.
- ,@(if (%current-target-system)
- '("gl_cv_func_wcwidth_works=yes")
- '()))))
- (synopsis "Operating on files matching given criteria")
- (description
- "Findutils supplies the basic file directory searching utilities of the
+ (name "findutils")
+ (version "4.6.0")
+ (source (origin
+ (method url-fetch)
+ (uri (string-append "mirror://gnu/findutils/findutils-"
+ version ".tar.gz"))
+ (sha256
+ (base32
+ "178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
+ (patches (search-patches
+ "findutils-localstatedir.patch"
+ "findutils-test-xargs.patch"
+ ;; test-lock has performance issues on multi-core
+ ;; machines, it hangs or takes a long time to complete.
+ ;; This is a commit from gnulib to fix this issue.
+ "findutils-gnulib-multi-core.patch"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:configure-flags (list
+ ;; Tell 'updatedb' to write to /var.
+ "--localstatedir=/var"
+
+ ;; Work around cross-compilation failure. See
+ ;; <http://savannah.gnu.org/bugs/?27299#comment1>.
+ ,@(if (%current-target-system)
+ '("gl_cv_func_wcwidth_works=yes")
+ '()))))
+ (synopsis "Operating on files matching given criteria")
+ (description
+ "Findutils supplies the basic file directory searching utilities of the
GNU system. It consists of two primary searching utilities: \"find\"
recursively searches for files in a directory according to given criteria and
\"locate\" lists files in a database that match a query. Two auxiliary tools
are included: \"updatedb\" updates the file name database and \"xargs\" may be
used to apply commands with arbitrarily long arguments.")
- (license gpl3+)
- (home-page "https://www.gnu.org/software/findutils/")))
+ (license gpl3+)
+ (home-page "https://www.gnu.org/software/findutils/")))
(define-public coreutils
(package
--
2.12.2
M
M
Marius Bakke wrote on 15 Apr 2017 20:11
Re: bug#26441: [PATCH 1/2] gnu: findutils: Fix make check issues on multi-core machines.
877f2lmthb.fsf@fastmail.com
Mathieu Othacehe <m.othacehe@gmail.com> writes:

Toggle quote (7 lines)
> * gnu/packages/patches/findutils-gnulib-multi-core.patch: New file.
> * gnu/packages/base.scm (findutils)[patches]: Add a reference
> to the previous patch.
> ---
> gnu/packages/base.scm | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)

It looks like you forgot the actual patch :)

Please also add it to gnu/local.mk. TIA!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAljyYkAACgkQoqBt8qM6
VPr/zQf/cwCsnmrh+isEs6BPt3XZObLkvvQTXpV6q+MnfjCLA95ForOFoCOg5QOV
tTJ4kp6La8DAfa9Q1FJ14yayTr8jBGODGRRltWarGJuhYsSj1ei2RwQN9cpD2mfS
35p0xp0V3EMCsnyoRBHhDFTIeCEZWMyZ77mVX/OWhzzphJULHwLvVEJhyOOBB+LA
7gcT0+TJUhIHFkcCCwkdIgMKcB4SAAmXxdOt9NNh025D81J4QBxeSp4cxJjm8p02
WcCuPjdw5Y1yCGsNAwPLiPebprocWlZOMsLSgWWaI2YWeLRIaiH4LkNcNRAkBw4Y
cSxMvEb+TWZfDONnMYoFJfHVeUpRjw==
=e/h4
-----END PGP SIGNATURE-----

M
M
Mathieu Othacehe wrote on 17 Apr 2017 12:18
Re: bug#26441: [PATCH 1/2] gnu: findutils: Fix make check issues on multi-core machines.
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 26441@debbugs.gnu.org)
87d1cb5odh.fsf@gmail.com
Hi Marius !

Toggle quote (2 lines)
> It looks like you forgot the actual patch :)

Ha ! You're right :)

Toggle quote (3 lines)
>
> Please also add it to gnu/local.mk. TIA!

Ok, I'll a new batch, to be applied to core-updates.

Thanks,

Mathieu
M
M
Mathieu Othacehe wrote on 17 Apr 2017 12:18
[PATCH 3/3] gnu: libunistring: Fix make check issues on multi-core machines.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170417101829.24362-3-m.othacehe@gmail.com
* gnu/packages/patches/libunistring-gnulib-multi-core.patch: New file.
* gnu/local.mk (dist_patch): Add previous patch.
* gnu/packages/libunistring.scm (libunistring)[patches]: Add a reference
to previous patch.
---
gnu/local.mk | 1 +
gnu/packages/libunistring.scm | 8 +-
.../patches/libunistring-gnulib-multi-core.patch | 178 +++++++++++++++++++++
3 files changed, 186 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/libunistring-gnulib-multi-core.patch

Toggle diff (231 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index b1dbfec5e..a0d7cfd0a 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -732,6 +732,7 @@ dist_patch_DATA = \
%D%/packages/patches/libtorrent-rasterbar-boost-compat.patch \
%D%/packages/patches/libtool-skip-tests2.patch \
%D%/packages/patches/libunwind-CVE-2015-3239.patch \
+ %D%/packages/patches/libunistring-gnulib-multi-core.patch \
%D%/packages/patches/libvpx-CVE-2016-2818.patch \
%D%/packages/patches/libwmf-CAN-2004-0941.patch \
%D%/packages/patches/libwmf-CVE-2006-3376.patch \
diff --git a/gnu/packages/libunistring.scm b/gnu/packages/libunistring.scm
index 212bec4b4..df02f68ce 100644
--- a/gnu/packages/libunistring.scm
+++ b/gnu/packages/libunistring.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -24,6 +25,7 @@
#:use-module (guix packages)
#:use-module (guix download)
#:use-module (guix build-system gnu)
+ #:use-module (gnu packages)
#:use-module (gnu packages base))
(define-public libunistring
@@ -37,7 +39,11 @@
version ".tar.xz"))
(sha256
(base32
- "15z76qrmrvkc3c6hfq2lzzqysgd21s682f2smycfab5g598n8drf"))))
+ "15z76qrmrvkc3c6hfq2lzzqysgd21s682f2smycfab5g598n8drf"))
+ ;; test-lock has performance issues on multi-core machines,
+ ;; it hangs or takes a long time to complete.
+ ;; This is a commit from gnulib to fix this issue.
+ (patches (search-patches "libunistring-gnulib-multi-core.patch"))))
(propagated-inputs (libiconv-if-needed))
(build-system gnu-build-system)
(arguments
diff --git a/gnu/packages/patches/libunistring-gnulib-multi-core.patch b/gnu/packages/patches/libunistring-gnulib-multi-core.patch
new file mode 100644
index 000000000..709b20c6d
--- /dev/null
+++ b/gnu/packages/patches/libunistring-gnulib-multi-core.patch
@@ -0,0 +1,178 @@
+This patch fixes performance problems on multi-core machines
+as reported at <https://bugs.gnu.org/26441>.
+
+See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in Gnulib
+by Bruno Haible <bruno@clisp.org>.
+
+diff --git a/tests/test-lock.c b/tests/test-lock.c
+index cb734b4e6..aa6de2739 100644
+--- a/tests/test-lock.c
++++ b/tests/test-lock.c
+@@ -50,6 +50,13 @@
+ Uncomment this to see if the operating system has a fair scheduler. */
+ #define EXPLICIT_YIELD 1
+
++/* Whether to use 'volatile' on some variables that communicate information
++ between threads. If set to 0, a lock is used to protect these variables.
++ If set to 1, 'volatile' is used; this is theoretically equivalent but can
++ lead to much slower execution (e.g. 30x slower total run time on a 40-core
++ machine. */
++#define USE_VOLATILE 0
++
+ /* Whether to print debugging messages. */
+ #define ENABLE_DEBUGGING 0
+
+@@ -103,6 +110,51 @@
+ # define yield()
+ #endif
+
++#if USE_VOLATILE
++struct atomic_int {
++ volatile int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ return ai->value;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ ai->value = new_value;
++}
++#else
++struct atomic_int {
++ gl_lock_define (, lock)
++ int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ gl_lock_init (ai->lock);
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ gl_lock_lock (ai->lock);
++ int ret = ai->value;
++ gl_lock_unlock (ai->lock);
++ return ret;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ gl_lock_lock (ai->lock);
++ ai->value = new_value;
++ gl_lock_unlock (ai->lock);
++}
++#endif
++
+ #define ACCOUNT_COUNT 4
+
+ static int account[ACCOUNT_COUNT];
+@@ -170,12 +222,12 @@ lock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int lock_checker_done;
++static struct atomic_int lock_checker_done;
+
+ static void *
+ lock_checker_thread (void *arg)
+ {
+- while (!lock_checker_done)
++ while (get_atomic_int_value (&lock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_lock_lock (my_lock);
+@@ -200,7 +252,8 @@ test_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- lock_checker_done = 0;
++ init_atomic_int (&lock_checker_done);
++ set_atomic_int_value (&lock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (lock_checker_thread, NULL);
+@@ -210,7 +263,7 @@ test_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- lock_checker_done = 1;
++ set_atomic_int_value (&lock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
+@@ -254,12 +307,12 @@ rwlock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int rwlock_checker_done;
++static struct atomic_int rwlock_checker_done;
+
+ static void *
+ rwlock_checker_thread (void *arg)
+ {
+- while (!rwlock_checker_done)
++ while (get_atomic_int_value (&rwlock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ());
+ gl_rwlock_rdlock (my_rwlock);
+@@ -284,7 +337,8 @@ test_rwlock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- rwlock_checker_done = 0;
++ init_atomic_int (&rwlock_checker_done);
++ set_atomic_int_value (&rwlock_checker_done, 0);
+
+ /* Spawn the threads. */
+ for (i = 0; i < THREAD_COUNT; i++)
+@@ -295,7 +349,7 @@ test_rwlock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- rwlock_checker_done = 1;
++ set_atomic_int_value (&rwlock_checker_done, 1);
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (checkerthreads[i], NULL);
+ check_accounts ();
+@@ -356,12 +410,12 @@ reclock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int reclock_checker_done;
++static struct atomic_int reclock_checker_done;
+
+ static void *
+ reclock_checker_thread (void *arg)
+ {
+- while (!reclock_checker_done)
++ while (get_atomic_int_value (&reclock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_recursive_lock_lock (my_reclock);
+@@ -386,7 +440,8 @@ test_recursive_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- reclock_checker_done = 0;
++ init_atomic_int (&reclock_checker_done);
++ set_atomic_int_value (&reclock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (reclock_checker_thread, NULL);
+@@ -396,7 +451,7 @@ test_recursive_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- reclock_checker_done = 1;
++ set_atomic_int_value (&reclock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
--
2.12.2
M
M
Mathieu Othacehe wrote on 17 Apr 2017 12:18
[PATCH 2/3] gnu: findutils: Fix make check issues on multi-core machines.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170417101829.24362-2-m.othacehe@gmail.com
* gnu/packages/patches/findutils-gnulib-multi-core.patch: New file.
* gnu/local.mk (dist_patch): Add previous patch.
* gnu/packages/base.scm (findutils)[patches]: Add a reference
to the previous patch.
---
gnu/local.mk | 1 +
gnu/packages/base.scm | 10 +-
.../patches/findutils-gnulib-multi-core.patch | 294 +++++++++++++++++++++
3 files changed, 303 insertions(+), 2 deletions(-)
create mode 100644 gnu/packages/patches/findutils-gnulib-multi-core.patch

Toggle diff (342 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 620fb07f3..b1dbfec5e 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -562,6 +562,7 @@ dist_patch_DATA = \
%D%/packages/patches/fcgi-2.4.0-gcc44-fixes.patch \
%D%/packages/patches/fcgi-2.4.0-poll.patch \
%D%/packages/patches/findutils-localstatedir.patch \
+ %D%/packages/patches/findutils-gnulib-multi-core.patch \
%D%/packages/patches/findutils-test-xargs.patch \
%D%/packages/patches/flint-ldconfig.patch \
%D%/packages/patches/fltk-shared-lib-defines.patch \
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index c56859021..004aacfa0 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -8,6 +8,7 @@
;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2016 Jan Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2017 Rene Saavedra <rennes@openmailbox.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -259,8 +260,13 @@ interactive means to merge two files.")
(sha256
(base32
"178nn4dl7wbcw499czikirnkniwnx36argdnqgz4ik9i6zvwkm6y"))
- (patches (search-patches "findutils-localstatedir.patch"
- "findutils-test-xargs.patch"))))
+ (patches (search-patches
+ "findutils-localstatedir.patch"
+ "findutils-test-xargs.patch"
+ ;; test-lock has performance issues on multi-core
+ ;; machines, it hangs or takes a long time to complete.
+ ;; This is a commit from gnulib to fix this issue.
+ "findutils-gnulib-multi-core.patch"))))
(build-system gnu-build-system)
(arguments
`(#:configure-flags (list
diff --git a/gnu/packages/patches/findutils-gnulib-multi-core.patch b/gnu/packages/patches/findutils-gnulib-multi-core.patch
new file mode 100644
index 000000000..5a37f4f1f
--- /dev/null
+++ b/gnu/packages/patches/findutils-gnulib-multi-core.patch
@@ -0,0 +1,294 @@
+This patch fixes performance problems on multi-core machines
+as reported at <https://bugs.gnu.org/26441>.
+
+See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in Gnulib
+by Bruno Haible <bruno@clisp.org>.
+
+diff --git a/tests/test-lock.c b/tests/test-lock.c
+index a992f64..fb18dee 100644
+--- a/tests/test-lock.c
++++ b/tests/test-lock.c
+@@ -1,5 +1,5 @@
+ /* Test of locking in multithreaded situations.
+- Copyright (C) 2005, 2008-2015 Free Software Foundation, Inc.
++ Copyright (C) 2005, 2008-2017 Free Software Foundation, Inc.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+@@ -50,6 +50,28 @@
+ Uncomment this to see if the operating system has a fair scheduler. */
+ #define EXPLICIT_YIELD 1
+
++/* Whether to use 'volatile' on some variables that communicate information
++ between threads. If set to 0, a semaphore or a lock is used to protect
++ these variables. If set to 1, 'volatile' is used; this is theoretically
++ equivalent but can lead to much slower execution (e.g. 30x slower total
++ run time on a 40-core machine), because 'volatile' does not imply any
++ synchronization/communication between different CPUs. */
++#define USE_VOLATILE 0
++
++#if USE_POSIX_THREADS && HAVE_SEMAPHORE_H
++/* Whether to use a semaphore to communicate information between threads.
++ If set to 0, a lock is used. If set to 1, a semaphore is used.
++ Uncomment this to reduce the dependencies of this test. */
++# define USE_SEMAPHORE 1
++/* Mac OS X provides only named semaphores (sem_open); its facility for
++ unnamed semaphores (sem_init) does not work. */
++# if defined __APPLE__ && defined __MACH__
++# define USE_NAMED_SEMAPHORE 1
++# else
++# define USE_UNNAMED_SEMAPHORE 1
++# endif
++#endif
++
+ /* Whether to print debugging messages. */
+ #define ENABLE_DEBUGGING 0
+
+@@ -90,6 +112,12 @@
+
+ #include "glthread/thread.h"
+ #include "glthread/yield.h"
++#if USE_SEMAPHORE
++# include <errno.h>
++# include <fcntl.h>
++# include <semaphore.h>
++# include <unistd.h>
++#endif
+
+ #if ENABLE_DEBUGGING
+ # define dbgprintf printf
+@@ -103,6 +131,132 @@
+ # define yield()
+ #endif
+
++#if USE_VOLATILE
++struct atomic_int {
++ volatile int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ return ai->value;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ ai->value = new_value;
++}
++#elif USE_SEMAPHORE
++/* This atomic_int implementation can only support the values 0 and 1.
++ It is initially 0 and can be set to 1 only once. */
++# if USE_UNNAMED_SEMAPHORE
++struct atomic_int {
++ sem_t semaphore;
++};
++#define atomic_int_semaphore(ai) (&(ai)->semaphore)
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ sem_init (&ai->semaphore, 0, 0);
++}
++# endif
++# if USE_NAMED_SEMAPHORE
++struct atomic_int {
++ sem_t *semaphore;
++};
++#define atomic_int_semaphore(ai) ((ai)->semaphore)
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ sem_t *s;
++ unsigned int count;
++ for (count = 0; ; count++)
++ {
++ char name[80];
++ /* Use getpid() in the name, so that different processes running at the
++ same time will not interfere. Use ai in the name, so that different
++ atomic_int in the same process will not interfere. Use a count in
++ the name, so that even in the (unlikely) case that a semaphore with
++ the specified name already exists, we can try a different name. */
++ sprintf (name, "test-lock-%lu-%p-%u",
++ (unsigned long) getpid (), ai, count);
++ s = sem_open (name, O_CREAT | O_EXCL, 0600, 0);
++ if (s == SEM_FAILED)
++ {
++ if (errno == EEXIST)
++ /* Retry with a different name. */
++ continue;
++ else
++ {
++ perror ("sem_open failed");
++ abort ();
++ }
++ }
++ else
++ {
++ /* Try not to leave a semaphore hanging around on the file system
++ eternally, if we can avoid it. */
++ sem_unlink (name);
++ break;
++ }
++ }
++ ai->semaphore = s;
++}
++# endif
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ if (sem_trywait (atomic_int_semaphore (ai)) == 0)
++ {
++ if (sem_post (atomic_int_semaphore (ai)))
++ abort ();
++ return 1;
++ }
++ else if (errno == EAGAIN)
++ return 0;
++ else
++ abort ();
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ if (new_value == 0)
++ /* It's already initialized with 0. */
++ return;
++ /* To set the value 1: */
++ if (sem_post (atomic_int_semaphore (ai)))
++ abort ();
++}
++#else
++struct atomic_int {
++ gl_lock_define (, lock)
++ int value;
++};
++static void
++init_atomic_int (struct atomic_int *ai)
++{
++ gl_lock_init (ai->lock);
++}
++static int
++get_atomic_int_value (struct atomic_int *ai)
++{
++ gl_lock_lock (ai->lock);
++ int ret = ai->value;
++ gl_lock_unlock (ai->lock);
++ return ret;
++}
++static void
++set_atomic_int_value (struct atomic_int *ai, int new_value)
++{
++ gl_lock_lock (ai->lock);
++ ai->value = new_value;
++ gl_lock_unlock (ai->lock);
++}
++#endif
++
+ #define ACCOUNT_COUNT 4
+
+ static int account[ACCOUNT_COUNT];
+@@ -170,12 +324,12 @@ lock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int lock_checker_done;
++static struct atomic_int lock_checker_done;
+
+ static void *
+ lock_checker_thread (void *arg)
+ {
+- while (!lock_checker_done)
++ while (get_atomic_int_value (&lock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_lock_lock (my_lock);
+@@ -200,7 +354,8 @@ test_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- lock_checker_done = 0;
++ init_atomic_int (&lock_checker_done);
++ set_atomic_int_value (&lock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (lock_checker_thread, NULL);
+@@ -210,7 +365,7 @@ test_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- lock_checker_done = 1;
++ set_atomic_int_value (&lock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
+@@ -254,12 +409,12 @@ rwlock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int rwlock_checker_done;
++static struct atomic_int rwlock_checker_done;
+
+ static void *
+ rwlock_checker_thread (void *arg)
+ {
+- while (!rwlock_checker_done)
++ while (get_atomic_int_value (&rwlock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ());
+ gl_rwlock_rdlock (my_rwlock);
+@@ -284,7 +439,8 @@ test_rwlock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- rwlock_checker_done = 0;
++ init_atomic_int (&rwlock_checker_done);
++ set_atomic_int_value (&rwlock_checker_done, 0);
+
+ /* Spawn the threads. */
+ for (i = 0; i < THREAD_COUNT; i++)
+@@ -295,7 +451,7 @@ test_rwlock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- rwlock_checker_done = 1;
++ set_atomic_int_value (&rwlock_checker_done, 1);
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (checkerthreads[i], NULL);
+ check_accounts ();
+@@ -356,12 +512,12 @@ reclock_mutator_thread (void *arg)
+ return NULL;
+ }
+
+-static volatile int reclock_checker_done;
++static struct atomic_int reclock_checker_done;
+
+ static void *
+ reclock_checker_thread (void *arg)
+ {
+- while (!reclock_checker_done)
++ while (get_atomic_int_value (&reclock_checker_done) == 0)
+ {
+ dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ());
+ gl_recursive_lock_lock (my_reclock);
+@@ -386,7 +542,8 @@ test_recursive_lock (void)
+ /* Initialization. */
+ for (i = 0; i < ACCOUNT_COUNT; i++)
+ account[i] = 1000;
+- reclock_checker_done = 0;
++ init_atomic_int (&reclock_checker_done);
++ set_atomic_int_value (&reclock_checker_done, 0);
+
+ /* Spawn the threads. */
+ checkerthread = gl_thread_create (reclock_checker_thread, NULL);
+@@ -396,7 +553,7 @@ test_recursive_lock (void)
+ /* Wait for the threads to terminate. */
+ for (i = 0; i < THREAD_COUNT; i++)
+ gl_thread_join (threads[i], NULL);
+- reclock_checker_done = 1;
++ set_atomic_int_value (&reclock_checker_done, 1);
+ gl_thread_join (checkerthread, NULL);
+ check_accounts ();
+ }
--
2.12.2
M
M
Mathieu Othacehe wrote on 17 Apr 2017 12:18
[PATCH 1/3] build: Add two missing patches to local.mk.
(address . 26441@debbugs.gnu.org)(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)
20170417101829.24362-1-m.othacehe@gmail.com
gnu/local.mk (dist_patch): Add gettext-multi-core.patch and
gettext-gnulib-multi-core.patch.

Commit 480da86d0969a667e8d2a564de535cb73a6a2229 ommited them.
---
gnu/local.mk | 2 ++
1 file changed, 2 insertions(+)

Toggle diff (15 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 51f92f088..620fb07f3 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -593,6 +593,8 @@ dist_patch_DATA = \
%D%/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch \
%D%/packages/patches/gegl-CVE-2012-4433.patch \
%D%/packages/patches/geoclue-config.patch \
+ %D%/packages/patches/gettext-multi-core.patch \
+ %D%/packages/patches/gettext-gnulib-multi-core.patch \
%D%/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch \
%D%/packages/patches/ghostscript-CVE-2013-5653.patch \
%D%/packages/patches/ghostscript-CVE-2015-3228.patch \
--
2.12.2
M
M
Marius Bakke wrote on 17 Apr 2017 21:47
Re: bug#26441: [PATCH 1/2] gnu: findutils: Fix make check issues on multi-core machines.
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 26441@debbugs.gnu.org)
87a87elsu2.fsf@fastmail.com
Mathieu Othacehe <m.othacehe@gmail.com> writes:

Toggle quote (11 lines)
> Hi Marius !
>
>> It looks like you forgot the actual patch :)
>
> Ha ! You're right :)
>
>>
>> Please also add it to gnu/local.mk. TIA!
>
> Ok, I'll a new batch, to be applied to core-updates.

Applied, thank you! Can this be closed now?
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlj1G8UACgkQoqBt8qM6
VPoNRgf/UVxU63hwUPa1py0vClxm4KMe3s9N2CHKMaW9aV/K4JCg06WG8Uwy2Shq
ctbqeaTZvI05lfkLgb18rlIW66IuTlH64wfKpBElW2s07T2+OlL5JqVmHfG7kGEc
ic5jn2bzOJCzzfUJk/N+NDkCTPKJ8yYdxd7r6JkQ3cVbQdO9X4yzXzz4BRuUOtuL
jCFSZFuXXos+Q+L1DzMEC+RwyHLBM2NTU6AFh+yPz6o0p+fHMeUlFDLYe2OCSvU+
fGkBXGoEoZG+vZ05tr1zKwiuXVZwHgCEsUOKY0O5t8pkUlp/sM/C5ia2Ut/1bMuH
6fZvhf9yF09/1jctAxZqj+9LuqOY5A==
=kTFO
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 19 Apr 2017 21:45
control message for bug #26441
(address . control@debbugs.gnu.org)
87pog8npux.fsf@gnu.org
severity 26441 important
L
L
Ludovic Courtès wrote on 23 Apr 2017 01:29
Re: bug#26441: Gnulib’s ‘test-lock ’ fails to complete on machines with >= 32 cores
(name . Mathieu Othacehe)(address . m.othacehe@gmail.com)(address . 26441-done@debbugs.gnu.org)
87r30kf2c3.fsf@gnu.org
Mathieu Othacehe <m.othacehe@gmail.com> skribis:

Toggle quote (8 lines)
>> Maybe we can simply patch the important packages (findutils,
>> libunistring) and wait and see if others need the patch. Not ideal, but
>> I can’t think of a better way.
>
> Ok fine for me.
>
> I'll send findutils and libunistring patches for now.

I see these have been committed to core-updates:

commit b100e0e89ebd1a6e73d6be4e354dd684abb057a4
Author: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Mon Apr 17 12:18:29 2017 +0200

gnu: libunistring: Fix make check issues on multi-core machines.

* gnu/packages/patches/libunistring-gnulib-multi-core.patch: New file.
* gnu/local.mk (dist_patch): Add previous patch.
* gnu/packages/libunistring.scm (libunistring)[patches]: Add a reference
to previous patch.

Signed-off-by: Marius Bakke <mbakke@fastmail.com>

commit da8e256a527a21ad88005c1d84514d910b964c19
Author: Mathieu Othacehe <m.othacehe@gmail.com>
Date: Mon Apr 17 12:18:28 2017 +0200

gnu: findutils: Fix make check issues on multi-core machines.

* gnu/packages/patches/findutils-gnulib-multi-core.patch: New file.
* gnu/local.mk (dist_patch): Add previous patch.
* gnu/packages/base.scm (findutils)[patches]: Add a reference
to the previous patch.

Signed-off-by: Marius Bakke <mbakke@fastmail.com>

Closing this bug now. Thank you Mathieu!

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 26441@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 26441
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch