From a5d45925af8253accce512c986fdec3269dc9604 Mon Sep 17 00:00:00 2001 From: Packit Service Date: Dec 10 2020 00:34:06 +0000 Subject: Apply patch papi-rhbz1807346.patch patch_name: papi-rhbz1807346.patch present_in_specfile: true --- diff --git a/src/ctests/Makefile.recipies b/src/ctests/Makefile.recipies index 201f3c8..d7cfbf6 100644 --- a/src/ctests/Makefile.recipies +++ b/src/ctests/Makefile.recipies @@ -157,6 +157,12 @@ locks_pthreads: locks_pthreads.c $(TESTLIB) $(PAPILIB) krentel_pthreads: krentel_pthreads.c $(TESTLIB) $(PAPILIB) $(CC_R) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) krentel_pthreads.c $(TESTLIB) $(PAPILIB) $(LDFLAGS) -o krentel_pthreads -lpthread +# krentel_pthreads_race is not included with the standard tests; +# it is a modification of krentel_pthreads intended to be run with +# "valgrind --tool=helgrind" to test for race conditions. +krentel_pthreads_race: krentel_pthreads_race.c $(TESTLIB) $(PAPILIB) + $(CC_R) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) krentel_pthreads_race.c $(TESTLIB) $(PAPILIB) $(LDFLAGS) -o krentel_pthreads_race -lpthread + overflow_pthreads: overflow_pthreads.c $(TESTLIB) $(DOLOOPS) $(PAPILIB) $(CC_R) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) overflow_pthreads.c $(TESTLIB) $(DOLOOPS) $(PAPILIB) $(LDFLAGS) -o overflow_pthreads -lpthread @@ -421,6 +427,9 @@ forkexec4: forkexec4.c $(TESTLIB) $(PAPILIB) prof_utils.o: prof_utils.c $(testlibdir)/papi_test.h prof_utils.h $(CC) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) -c prof_utils.c +filter_helgrind: filter_helgrind.c $(TESTLIB) $(PAPILIB) + -$(CC) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) filter_helgrind.c $(TESTLIB) $(PAPILIB) $(LDFLAGS) -o filter_helgrind + .PHONY : all default ctests ctest clean clean: diff --git a/src/ctests/filter_helgrind.c b/src/ctests/filter_helgrind.c new file mode 100644 index 0000000..d918a78 --- /dev/null +++ b/src/ctests/filter_helgrind.c @@ -0,0 +1,170 @@ +/* + * This code is a simple filter for the helgrind_out.txt file + * produced by: + * "valgrind --tool=helgrind --log-file=helgrind_out.txt someProgram" + * + * This is useful because the tool does not recognize PAPI locks, + * thus reports as possible race conditions reads/writes by + * different threads that are actually fine (surrounded by locks). + * + * This was written particularly for krentel_pthreads_race.c + * when processed by the above valgrind. We produce a line per + * condition, in the form: + * OP@file:line OP@file:line + * where OP is R or W. The first file:line code occurred + * after the second file:line code, and on a different thread. + * + * We print the results to stdout. It is useful to filter this + * through the standard utility 'uniq', each occurrence only + * needs to be investigated once. Just insure there are + * MATCHING locks around each operation within the code. + * + * An example run (using uniq): The options -uc will print + * only unique lines, preceeded by a count of how many times + * it occurs. + * + * ./filter_helgrind | uniq -uc + * + * An example output line (piped through uniq as above): + * 1 R@threads.c:190 W@threads.c:206 + * An investigation shows threads.c:190 is protected by + * _papi_hwi_lock(THREADS_LOCK); and threads.c:206 is + * protected by the same lock. Thus no data race can + * occur for this instance. + * + * Compilation within the papi/src/ctests directory: + * make filter_helgrind + * + */ + +#include +#include +#include + +int main(int argc, char** args) { + (void) argc; + (void) args; + + char myLine[16384]; + int state, size; + char type1, type2; + char fname1[256], fname2[256]; + char *paren1, *paren2; + + FILE *HELOUT = fopen("helgrind_out.txt", "r"); // Read the file. + if (HELOUT == NULL) { + fprintf(stderr, "Could not open helgrind_out.txt.\n"); + exit(-1); + } + + char PDRR[]="Possible data race during read"; + char PDRW[]="Possible data race during write"; + char TCWW[]="This conflicts with a previous write"; + char TCWR[]="This conflicts with a previous read"; + char atSTR[]=" at "; + + // State machine: + // State 0: We are looking for a line with PDRR or PDRW. + // We don't exit until we find it, or run out of lines. + // if we find it, we remember which and go to state 1. + // State 1: Looking for " at " in column 11. + // When found, we extract the string betweeen '(' and ')' + // which is program name:line. go to state 2. + // State 2: We are searching for TCWW, TCWR, PDRW, PDRR. + // If we find the first two: + // Remember which, and go to state 3. + // If we find either of the second two, go back to State 1. + // State 3: Looking for " at " in column 11. + // When found, extract the string betweeen '(' and ')', + // which is program name:line. + // OUTPUT LINE for an investigation. + // Go to State 0. + + state = 0; // looking for PDRR, PDRW. + while (fgets(myLine, 16384, HELOUT) != NULL) { + if (strlen(myLine) < 20) continue; + switch (state) { + case 0: // Looking for PDRR or PRDW. + if (strstr(myLine, PDRR) != NULL) { + type1='R'; + state=1; + continue; + } + + if (strstr(myLine, PDRW) != NULL) { + type1='W'; + state=1; + continue; + } + + continue; + break; + + case 1: // Looking for atSTR in column 11. + if (strncmp(myLine+10, atSTR, 6) != 0) continue; + paren1=strchr(myLine, '('); + paren2=strchr(myLine, ')'); + if (paren1 == NULL || paren2 == NULL || + paren1 > paren2) { + state=0; // Abort, found something I don't understand. + continue; + } + + size = paren2-paren1-1; // compute length of name. + strncpy(fname1, paren1+1, size); // Copy the name. + fname1[size]=0; // install z-terminator. + state=2; + continue; + break; + + case 2: // Looking for TCWW, TCWR, PDRR, PDRW. + if (strstr(myLine, TCWR) != NULL) { + type2='R'; + state=3; + continue; + } + + if (strstr(myLine, TCWW) != NULL) { + type2='W'; + state=3; + continue; + } + + if (strstr(myLine, PDRR) != NULL) { + type1='R'; + state=1; + continue; + } + + if (strstr(myLine, PDRW) != NULL) { + type1='W'; + state=1; + continue; + } + + continue; + break; + + case 3: // Looking for atSTR in column 11. + if (strncmp(myLine+10, atSTR, 6) != 0) continue; + paren1=strchr(myLine, '('); + paren2=strchr(myLine, ')'); + if (paren1 == NULL || paren2 == NULL || + paren1 > paren2) { + state=0; // Abort, found something I don't understand. + continue; + } + + size = paren2-paren1-1; // compute length of name. + strncpy(fname2, paren1+1, size); // Copy the name. + fname2[size]=0; // install z-terminator. + fprintf(stdout, "%c@%-32s %c@%-32s\n", type1, fname1, type2, fname2); + state=0; + continue; + break; + } // end switch. + } // end while. + + fclose(HELOUT); + exit(0); +} diff --git a/src/ctests/krentel_pthreads_race.c b/src/ctests/krentel_pthreads_race.c new file mode 100644 index 0000000..0ebfb50 --- /dev/null +++ b/src/ctests/krentel_pthreads_race.c @@ -0,0 +1,236 @@ +/* + * Test PAPI with multiple threads. + * This code is a modification of krentel_pthreads.c by William Cohen + * , on Sep 10 2019, to exercise and test for the race + * condition in papi_internal.c involving the formerly static variables + * papi_event_code and papi_event_code_changed. This code should be run with + * "valgrind --tool=helgrind" to show any data races. If run with: + * "valgrind --tool=helgrind --log-file=helgrind_out.txt" + * The output will be captured in helgrind_out.txt and can then be processed + * with the program filter_helgrind.c; see commentary at the top of that file. + */ + +#define MAX_THREADS 256 + +#include +#include +#include +#include + +#include "papi.h" +#include "papi_test.h" + +#define EVENT PAPI_TOT_CYC + +static int program_time = 5; +static int threshold = 20000000; +static int num_threads = 3; + +static long count[MAX_THREADS]; +static long iter[MAX_THREADS]; +static struct timeval last[MAX_THREADS]; + +static pthread_key_t key; + +static struct timeval start; + +static void +my_handler( int EventSet, void *pc, long long ovec, void *context ) +{ + ( void ) EventSet; + ( void ) pc; + ( void ) ovec; + ( void ) context; + + long num = ( long ) pthread_getspecific( key ); + + if ( num < 0 || num > num_threads ) + test_fail( __FILE__, __LINE__, "getspecific failed", 1 ); + count[num]++; +} + +static void +print_rate( long num ) +{ + struct timeval now; + long st_secs; + double last_secs; + + gettimeofday( &now, NULL ); + st_secs = now.tv_sec - start.tv_sec; + last_secs = ( double ) ( now.tv_sec - last[num].tv_sec ) + + ( ( double ) ( now.tv_usec - last[num].tv_usec ) ) / 1000000.0; + if ( last_secs <= 0.001 ) + last_secs = 0.001; + + if (!TESTS_QUIET) { + printf( "[%ld] time = %ld, count = %ld, iter = %ld, " + "rate = %.1f/Kiter\n", + num, st_secs, count[num], iter[num], + ( 1000.0 * ( double ) count[num] ) / ( double ) iter[num] ); + } + + count[num] = 0; + iter[num] = 0; + last[num] = now; +} + +static void +do_cycles( long num, int len ) +{ + struct timeval start, now; + double x, sum; + + gettimeofday( &start, NULL ); + + for ( ;; ) { + sum = 1.0; + for ( x = 1.0; x < 250000.0; x += 1.0 ) + sum += x; + if ( sum < 0.0 ) + printf( "==>> SUM IS NEGATIVE !! <<==\n" ); + + iter[num]++; + + gettimeofday( &now, NULL ); + if ( now.tv_sec >= start.tv_sec + len ) + break; + } +} + +static void * +my_thread( void *v ) +{ + long num = ( long ) v; + int n; + int EventSet = PAPI_NULL; + int event_code; + long long value; + + int retval; + + retval = PAPI_register_thread( ); + if ( retval != PAPI_OK ) { + test_fail( __FILE__, __LINE__, "PAPI_register_thread", retval ); + } + pthread_setspecific( key, v ); + + count[num] = 0; + iter[num] = 0; + last[num] = start; + + retval = PAPI_create_eventset( &EventSet ); + if ( retval != PAPI_OK ) { + test_fail( __FILE__, __LINE__, "PAPI_create_eventset failed", retval ); + } + + retval = PAPI_event_name_to_code("PAPI_TOT_CYC", &event_code); + if (retval != PAPI_OK ) { + if (!TESTS_QUIET) printf("Trouble creating event name\n"); + test_fail( __FILE__, __LINE__, "PAPI_event_name_to_code failed", retval ); + } + + retval = PAPI_add_event( EventSet, EVENT ); + if (retval != PAPI_OK ) { + if (!TESTS_QUIET) printf("Trouble adding event\n"); + test_fail( __FILE__, __LINE__, "PAPI_add_event failed", retval ); + } + + if ( PAPI_overflow( EventSet, EVENT, threshold, 0, my_handler ) != PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_overflow failed", 1 ); + + if ( PAPI_start( EventSet ) != PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_start failed", 1 ); + + if (!TESTS_QUIET) printf( "launched timer in thread %ld\n", num ); + + for ( n = 1; n <= program_time; n++ ) { + do_cycles( num, 1 ); + print_rate( num ); + } + + PAPI_stop( EventSet, &value ); + + retval = PAPI_overflow( EventSet, EVENT, 0, 0, my_handler); + if ( retval != PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_overflow failed to reset the overflow handler", retval ); + + if ( PAPI_remove_event( EventSet, EVENT ) != PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_remove_event", 1 ); + + if ( PAPI_destroy_eventset( &EventSet ) != PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_destroy_eventset", 1 ); + + if ( PAPI_unregister_thread( ) != PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_unregister_thread", 1 ); + + return ( NULL ); +} + +int +main( int argc, char **argv ) +{ + pthread_t *td = NULL; + long n; + int quiet,retval; + + /* Set TESTS_QUIET variable */ + quiet=tests_quiet( argc, argv ); + + if ( argc < 2 || sscanf( argv[1], "%d", &program_time ) < 1 ) + program_time = 6; + if ( argc < 3 || sscanf( argv[2], "%d", &threshold ) < 1 ) + threshold = 20000000; + if ( argc < 4 || sscanf( argv[3], "%d", &num_threads ) < 1 ) + num_threads = 32; + + td = malloc((num_threads+1) * sizeof(pthread_t)); + if (!td) { + test_fail( __FILE__, __LINE__, "td malloc failed", 1 ); + } + + if (!quiet) { + printf( "program_time = %d, threshold = %d, num_threads = %d\n\n", + program_time, threshold, num_threads ); + } + + if ( PAPI_library_init( PAPI_VER_CURRENT ) != PAPI_VER_CURRENT ) + test_fail( __FILE__, __LINE__, "PAPI_library_init failed", 1 ); + + /* Test to be sure we can add events */ + retval = PAPI_query_event( EVENT ); + if (retval!=PAPI_OK) { + if (!quiet) printf("Trouble finding event\n"); + test_skip(__FILE__,__LINE__,"Event not available",1); + } + + if ( PAPI_thread_init( ( unsigned long ( * )( void ) ) ( pthread_self ) ) != + PAPI_OK ) + test_fail( __FILE__, __LINE__, "PAPI_thread_init failed", 1 ); + + if ( pthread_key_create( &key, NULL ) != 0 ) + test_fail( __FILE__, __LINE__, "pthread key create failed", 1 ); + + gettimeofday( &start, NULL ); + + for ( n = 1; n <= num_threads; n++ ) { + if ( pthread_create( &(td[n]), NULL, my_thread, ( void * ) n ) != 0 ) + test_fail( __FILE__, __LINE__, "pthread create failed", 1 ); + } + + my_thread( ( void * ) 0 ); + + /* wait for all the threads */ + for ( n = 1; n <= num_threads; n++ ) { + if ( pthread_join( td[n], NULL)) + test_fail( __FILE__, __LINE__, "pthread join failed", 1 ); + } + + free(td); + + if (!quiet) printf( "done\n" ); + + test_pass( __FILE__ ); + + return 0; +} diff --git a/src/papi.c b/src/papi.c index 431d807..92c84bd 100644 --- a/src/papi.c +++ b/src/papi.c @@ -607,32 +607,26 @@ PAPI_library_init( int version ) papi_return( init_retval ); } - /* Initialize component globals */ + /* Initialize thread globals, including the main threads */ - tmp = _papi_hwi_init_global( ); + tmp = _papi_hwi_init_global_threads( ); if ( tmp ) { init_retval = tmp; _papi_hwi_shutdown_global_internal( ); - _in_papi_library_init_cnt--; + _in_papi_library_init_cnt--; papi_return( init_retval ); } - - /* Initialize thread globals, including the main threads */ - tmp = _papi_hwi_init_global_threads( ); + /* Initialize component globals */ + + tmp = _papi_hwi_init_global( ); if ( tmp ) { - int i; init_retval = tmp; _papi_hwi_shutdown_global_internal( ); - for ( i = 0; i < papi_num_components; i++ ) { - if (!_papi_hwd[i]->cmp_info.disabled) { - _papi_hwd[i]->shutdown_component( ); - } - } _in_papi_library_init_cnt--; papi_return( init_retval ); } - + init_level = PAPI_LOW_LEVEL_INITED; _in_papi_library_init_cnt--; diff --git a/src/papi_internal.c b/src/papi_internal.c index 44a3bd5..3828aed 100644 --- a/src/papi_internal.c +++ b/src/papi_internal.c @@ -111,31 +111,28 @@ _papi_hwi_free_papi_event_string() { } return; } -// A place to keep the current papi event code so some component functions can fetch its value -// The current event code can be stored here prior to component calls and cleared after the component returns -static unsigned int papi_event_code = -1; -static int papi_event_code_changed = -1; + void _papi_hwi_set_papi_event_code (unsigned int event_code, int update_flag) { INTDBG("new event_code: %#x, update_flag: %d, previous event_code: %#x\n", event_code, update_flag, papi_event_code); // if call is just to reset and start over, set both flags to show nothing saved yet if (update_flag < 0) { - papi_event_code_changed = -1; - papi_event_code = -1; + _papi_hwi_my_thread->tls_papi_event_code_changed = -1; + _papi_hwi_my_thread->tls_papi_event_code = -1; return; } // if 0, it is being set prior to calling a component, if >0 it is being changed by the component - papi_event_code_changed = update_flag; + _papi_hwi_my_thread->tls_papi_event_code_changed = update_flag; // save the event code passed in - papi_event_code = event_code; + _papi_hwi_my_thread->tls_papi_event_code = event_code; return; } unsigned int _papi_hwi_get_papi_event_code () { INTDBG("papi_event_code: %#x\n", papi_event_code); - return papi_event_code; + return _papi_hwi_my_thread->tls_papi_event_code; } /* Get the index into the ESI->NativeInfoArray for the current PAPI event code */ int @@ -555,7 +552,7 @@ _papi_hwi_native_to_eventcode(int cidx, int event_code, int ntv_idx, const char int result; - if (papi_event_code_changed > 0) { + if (_papi_hwi_my_thread->tls_papi_event_code_changed > 0) { result = _papi_hwi_get_papi_event_code(); INTDBG("EXIT: papi_event_code: %#x set by the component\n", result); return result; diff --git a/src/threads.c b/src/threads.c index 4dd0cf4..9f586c4 100644 --- a/src/threads.c +++ b/src/threads.c @@ -286,6 +286,10 @@ _papi_hwi_initialize_thread( ThreadInfo_t ** dest, int tid ) return PAPI_ENOMEM; } + /* init event memory variables, used by papi_internal.c */ + thread->tls_papi_event_code = -1; + thread->tls_papi_event_code_changed = -1; + /* Call the component to fill in anything special. */ for ( i = 0; i < papi_num_components; i++ ) { @@ -421,6 +425,11 @@ _papi_hwi_shutdown_thread( ThreadInfo_t * thread, int force_shutdown ) unsigned long tid; int i, failure = 0; + /* Clear event memory variables */ + thread->tls_papi_event_code = -1; + thread->tls_papi_event_code_changed = -1; + + /* Get thread id */ if ( _papi_hwi_thread_id_fn ) tid = ( *_papi_hwi_thread_id_fn ) ( ); else diff --git a/src/threads.h b/src/threads.h index cd33690..264d9f3 100644 --- a/src/threads.h +++ b/src/threads.h @@ -30,6 +30,11 @@ typedef struct _ThreadInfo EventSetInfo_t **running_eventset; EventSetInfo_t *from_esi; /* ESI used for last update this control state */ int wants_signal; + + // The current event code can be stored here prior to + // component calls and cleared after the component returns. + unsigned int tls_papi_event_code; + int tls_papi_event_code_changed; } ThreadInfo_t; /** The list of threads, gets initialized to master process with TID of getpid()