From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathieu Othacehe Subject: bug#26441: [PATCH 2/3] gnu: findutils: Fix make check issues on multi-core machines. Date: Mon, 17 Apr 2017 12:18:28 +0200 Message-ID: <20170417101829.24362-2-m.othacehe@gmail.com> References: <20170417101829.24362-1-m.othacehe@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:34784) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d03kQ-0002SP-U3 for bug-guix@gnu.org; Mon, 17 Apr 2017 06:19:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d03kN-0000y0-LS for bug-guix@gnu.org; Mon, 17 Apr 2017 06:19:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:53362) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1d03kN-0000xw-HN for bug-guix@gnu.org; Mon, 17 Apr 2017 06:19:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1d03kN-0007CE-Bs for bug-guix@gnu.org; Mon, 17 Apr 2017 06:19:03 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <20170417101829.24362-1-m.othacehe@gmail.com> List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: 26441@debbugs.gnu.org * 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 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 ;;; Copyright © 2016 Jan Nieuwenhuizen ;;; Copyright © 2017 Rene Saavedra +;;; Copyright © 2017 Mathieu Othacehe ;;; ;;; 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 . + +See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in Gnulib +by Bruno Haible . + +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 ++# include ++# include ++# include ++#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