diff options
| -rw-r--r-- | src/lib/sync.C | 83 | ||||
| -rw-r--r-- | src/runtime/rt_sync.C | 8 | ||||
| -rw-r--r-- | src/usr/targeting/common/targetservice.C | 15 | ||||
| -rwxr-xr-x | src/usr/targeting/common/xmltohb/attribute_types_hb.xml | 12 | ||||
| -rw-r--r-- | src/usr/targeting/common/xmltohb/target_types_hb.xml | 3 | ||||
| -rwxr-xr-x | src/usr/targeting/common/xmltohb/xmltohb.pl | 89 | ||||
| -rwxr-xr-x | src/usr/targeting/targetservicestart.C | 47 | ||||
| -rw-r--r-- | src/usr/targeting/test/testtargeting.H | 176 | ||||
| -rw-r--r-- | src/usr/testcore/lib/makefile | 2 | ||||
| -rw-r--r-- | src/usr/testcore/lib/synctest.H | 15 |
10 files changed, 394 insertions, 56 deletions
diff --git a/src/lib/sync.C b/src/lib/sync.C index c318856a1..210790074 100644 --- a/src/lib/sync.C +++ b/src/lib/sync.C @@ -31,6 +31,10 @@ using namespace Systemcalls; +// The size of mutexes are hardcoded in xmltohb.pl script and must be that +// size. +const int MUTEX_SIZE = 24; + //----------------------------------------------------------------------------- int futex_wait(uint64_t * i_addr, uint64_t i_val) @@ -181,6 +185,16 @@ void mutex_lock(mutex_t * i_mutex) // __sync_lock_test_and_set have an implied isync. crit_assert(!i_mutex->iv_recursive); + static_assert(sizeof(mutex_t) == MUTEX_SIZE, + "mutex_t must be size of 24 bytes"); + + // The mutex is only allowed to take on three values: 0, 1, or 2. + // 0: Lock is not held. + // 1: Lock is held by a task with no contention. + // 2: Lock is held by a task and there is contention. + // Any other values indicates that outside influences have messed with the + // mutex and we shouldn't continue. + crit_assert(i_mutex->iv_val <= 2); uint64_t l_lockStatus = __sync_val_compare_and_swap(&(i_mutex->iv_val),0,1); @@ -216,11 +230,22 @@ void mutex_unlock(mutex_t * i_mutex) // context synchronizing nature of the 'sc' instruction. crit_assert(!i_mutex->iv_recursive); + static_assert(sizeof(mutex_t) == MUTEX_SIZE, + "mutex_t must be size of 24 bytes"); + + // The mutex is only allowed to take on three values: 0, 1, or 2. + // 0: Lock is not held. + // 1: Lock is held by a task with no contention. + // 2: Lock is held by a task and there is contention. + // Any other values indicates that outside influences have messed with the + // mutex and we shouldn't continue. + crit_assert(i_mutex->iv_val <= 2); uint64_t l_lockStatus = __sync_fetch_and_sub(&(i_mutex->iv_val), 1); - if(unlikely(2 <= l_lockStatus)) + if(unlikely(l_lockStatus == 2)) { + // Fully release the lock and let another task grab it. i_mutex->iv_val = 0; futex_wake(&(i_mutex->iv_val), 1); // wake one task } @@ -231,23 +256,57 @@ void mutex_unlock(mutex_t * i_mutex) void recursive_mutex_lock(mutex_t * i_mutex) { crit_assert(i_mutex->iv_recursive); - + static_assert(sizeof(mutex_t) == MUTEX_SIZE, + "mutex_t must be size of 24 bytes"); + + // The mutex is only allowed to take on three values: 0, 1, or 2. + // 0: Lock is not held. + // 1: Lock is held by a task with no contention. + // 2: Lock is held by a task and there is contention. + // Any other values indicates that outside influences have messed with the + // mutex and we shouldn't continue. + crit_assert(i_mutex->iv_val <= 2); + + // Check the contents of the mutex's iv_val and if it's equal to 0, then + // assign it the value of 1. l_lockStatus is initialized with the value of + // iv_val prior to the assignment. uint64_t l_lockStatus = __sync_val_compare_and_swap(&(i_mutex->iv_val),0,1); + do { if(unlikely(l_lockStatus != 0)) { + // There may be contention for the lock. Since this is a recursive + // mutex we first need to check if the owner has called for the lock + // again. if (task_gettid() == i_mutex->iv_owner) { + // The owner called for the lock. There isn't contention at this + // point and so subsequent calls to this function will still + // initialize l_lockStatus to 1. + // + // Increment the owner's lock count to keep track of how much + // unwinding will be necessary but the lock can be safely + // released. ++i_mutex->iv_ownerLockCount; + + // Return now so that the owner doesn't set the lock to be + // contended and enter the waiting queue, thus causing a + // deadlock. break; } + // By reaching this point there is certainly contention for the + // lock. Ensure that the lock status reflects this if it doesn't + // already. if (likely(l_lockStatus != 2)) { + // mutex's iv_val will be set to 2 and the previous value will + // be assigned to l_lockStatus. l_lockStatus = __sync_lock_test_and_set(&(i_mutex->iv_val), 2); } + // Send caller to the wait queue. while( l_lockStatus != 0 ) { futex_wait( &(i_mutex->iv_val), 2); @@ -257,6 +316,7 @@ void recursive_mutex_lock(mutex_t * i_mutex) } } + // Set the new owner of the lock and increment the lock count. i_mutex->iv_owner = task_gettid(); ++i_mutex->iv_ownerLockCount; @@ -268,22 +328,39 @@ void recursive_mutex_lock(mutex_t * i_mutex) void recursive_mutex_unlock(mutex_t * i_mutex) { crit_assert(i_mutex->iv_recursive); + static_assert(sizeof(mutex_t) == MUTEX_SIZE, + "mutex_t must be size of 24 bytes"); + + // The mutex is only allowed to take on three values: 0, 1, or 2. + // 0: Lock is not held. + // 1: Lock is held by a task with no contention. + // 2: Lock is held by a task and there is contention. + // Any other values indicates that outside influences have messed with the + // mutex and we shouldn't continue. + crit_assert(i_mutex->iv_val <= 2); uint64_t l_lockStatus = 0; + if (i_mutex->iv_ownerLockCount <= 1) { + // Owner is finished with the lock. So reset owner variables of mutex. i_mutex->iv_ownerLockCount = 0; i_mutex->iv_owner = 0; + + // Decrements iv_val by 1 and assigns the value prior to the operation + // to l_lockStatus. l_lockStatus = __sync_fetch_and_sub(&(i_mutex->iv_val),1); - if(unlikely(2 <= l_lockStatus)) + if(unlikely(l_lockStatus == 2)) { + // Fully release the lock to allow the next task to grab it. i_mutex->iv_val = 0; futex_wake(&(i_mutex->iv_val), 1); // wake one task } } else { + // Unwind the recursive lock one step. --i_mutex->iv_ownerLockCount; } diff --git a/src/runtime/rt_sync.C b/src/runtime/rt_sync.C index 86d1b426b..0b7cc027e 100644 --- a/src/runtime/rt_sync.C +++ b/src/runtime/rt_sync.C @@ -5,7 +5,9 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* COPYRIGHT International Business Machines Corp. 2013,2014 */ +/* Contributors Listed Below - COPYRIGHT 2013,2018 */ +/* [+] International Business Machines Corp. */ +/* */ /* */ /* Licensed under the Apache License, Version 2.0 (the "License"); */ /* you may not use this file except in compliance with the License. */ @@ -22,6 +24,10 @@ /* IBM_PROLOG_END_TAG */ #include <sys/sync.h> +void recursive_mutex_init(mutex_t*) {}; +void recursive_mutex_destroy(mutex_t*) {}; +void recursive_mutex_lock(mutex_t*) {}; +void recursive_mutex_unlock(mutex_t*) {}; void mutex_init(mutex_t*) {}; void mutex_destroy(mutex_t*) {}; void mutex_lock(mutex_t*) {}; diff --git a/src/usr/targeting/common/targetservice.C b/src/usr/targeting/common/targetservice.C index c7f002f87..e3c3dde92 100644 --- a/src/usr/targeting/common/targetservice.C +++ b/src/usr/targeting/common/targetservice.C @@ -1793,14 +1793,23 @@ uint32_t TargetService::resetMutexAttributes(const Target* i_pTarget) for ( uint32_t l_attrIndex = 0; l_attrIndex < l_attrCount; l_attrIndex++) { const ATTRIBUTE_ID l_attrId = l_pAttrIds[l_attrIndex]; - for( const auto mutexId : hbMutexAttrIds) + for( const auto mutex : hbMutexAttrIds) { - if(l_attrId == mutexId) + if(l_attrId == mutex.id) { mutex_t* l_mutex; if(i_pTarget->_tryGetHbMutexAttr(l_attrId, l_mutex)) { - mutex_init(l_mutex); +#ifdef __HOSTBOOT_MODULE + if (mutex.isRecursive) + { + recursive_mutex_init(l_mutex); + } + else +#endif + { + mutex_init(l_mutex); + } l_numberMutexAttrsReset++; } else diff --git a/src/usr/targeting/common/xmltohb/attribute_types_hb.xml b/src/usr/targeting/common/xmltohb/attribute_types_hb.xml index 5f88bbce1..1c574ebaa 100755 --- a/src/usr/targeting/common/xmltohb/attribute_types_hb.xml +++ b/src/usr/targeting/common/xmltohb/attribute_types_hb.xml @@ -330,6 +330,18 @@ </attribute> <attribute> + <id>HB_RECURSIVE_MUTEX_TEST_LOCK</id> + <description>Host boot recursive mutex for testing</description> + <simpleType> + <hbrecursivemutex/> + </simpleType> + <persistency>volatile-zeroed</persistency> + <readable/> + <writeable/> + <hbOnly/> + </attribute> + + <attribute> <id>HB_RSV_MEM_NEXT_SECTION</id> <description> The next HB reserved memory section available to assign diff --git a/src/usr/targeting/common/xmltohb/target_types_hb.xml b/src/usr/targeting/common/xmltohb/target_types_hb.xml index 7ac29598f..eaf0ca0b6 100644 --- a/src/usr/targeting/common/xmltohb/target_types_hb.xml +++ b/src/usr/targeting/common/xmltohb/target_types_hb.xml @@ -260,6 +260,9 @@ <id>HB_MUTEX_TEST_LOCK</id> </attribute> <attribute> + <id>HB_RECURSIVE_MUTEX_TEST_LOCK</id> + </attribute> + <attribute> <id>HB_RSV_MEM_NEXT_SECTION</id> </attribute> <attribute> diff --git a/src/usr/targeting/common/xmltohb/xmltohb.pl b/src/usr/targeting/common/xmltohb/xmltohb.pl index e92bf598e..29727c403 100755 --- a/src/usr/targeting/common/xmltohb/xmltohb.pl +++ b/src/usr/targeting/common/xmltohb/xmltohb.pl @@ -2330,7 +2330,7 @@ namespace TARGETING * Array defining all attribute ids found that are of type hbMutex. * This file is autogenerated and should not be altered. */ -const uint32_t hbMutexAttrIds[] = { +const struct {uint32_t id; bool isRecursive;} hbMutexAttrIds[] = { VERBATIM my @mutexAttrIds; @@ -2340,22 +2340,30 @@ VERBATIM #check if hbmutex tag is present #check that attr is readable/writeable if( (exists $attribute->{simpleType}) - && (exists $attribute->{simpleType}->{hbmutex}) + && (exists $attribute->{simpleType}->{hbmutex} + || exists $attribute->{simpleType}->{hbrecursivemutex}) && (exists $attribute->{readable}) && (exists $attribute->{writeable})) { - push @mutexAttrIds, $attribute->{id}; + my $recursiveType = "false"; + if (exists $attribute->{simpleType}->{hbrecursivemutex}) + { + $recursiveType = "true"; + } + + push @mutexAttrIds, ([ $attribute->{id}, $recursiveType ]); } } # variables that can be used for writing the enums to the file my $attrId; my $hexVal; + my $recursiveVal; # Format below intentionally > 80 chars for clarity format ATTRMUTEXFORMAT = - @>>>>>>>>>> - $hexVal ."," + @>>>>>>>>>>>>>>>>>>>> + "{ ". $hexVal .", ". $recursiveVal ." }," . select($outFile); $~ = 'ATTRMUTEXFORMAT'; @@ -2368,7 +2376,9 @@ VERBATIM $attrId = $enumerator->{name}; foreach my $mutexAttrId (@mutexAttrIds) { - if( $mutexAttrId eq $attrId ) + $recursiveVal = $mutexAttrId->[1]; + + if( $mutexAttrId->[0] eq $attrId ) { write; last; @@ -2494,7 +2504,8 @@ sub writeTraitFileTraits { # Mark the attribute as being a host boot mutex or non-host boot mutex if( (exists $attribute->{simpleType}) - && (exists $attribute->{simpleType}->{hbmutex}) ) + && (exists $attribute->{simpleType}->{hbmutex} + || exists $attribute->{simpleType}->{hbrecursivemutex}) ) { $traits .= " hbMutex,"; } @@ -2751,6 +2762,7 @@ sub writeAttrErrlCFile { !(exists $attribute->{writeable}) || # read-only attributes (exists $attribute->{simpleType} && ( (exists $attribute->{simpleType}->{hbmutex}) || + (exists $attribute->{simpleType}->{hbrecrusivemutex}) || (exists $attribute->{simpleType}->{fspmutex}))) # mutex attributes ) { print $outFile " case (ATTR_",$attribute->{id},"): { break; }\n"; @@ -2923,6 +2935,7 @@ sub writeAttrErrlCFile { !(exists $attribute->{writeable}) || # read-only attributes (exists $attribute->{simpleType} && ( (exists $attribute->{simpleType}->{hbmutex}) || + (exists $attribute->{simpleType}->{hbrecursivemutex}) || (exists $attribute->{simpleType}->{fspmutex}))) # mutex attributes ) { next; @@ -3040,6 +3053,7 @@ sub writeAttrErrlHFile { !(exists $attribute->{writeable}) || # read-only attributes (exists $attribute->{simpleType} && ( (exists $attribute->{simpleType}->{hbmutex}) || + (exists $attribute->{simpleType}->{hbrecursivemutex}) || (exists $attribute->{simpleType}->{fspmutex}))) # mutex attributes ) { print $outFile " //not readable\n"; @@ -3069,7 +3083,7 @@ sub writeAttrErrlHFile { } } # makes no sense to dump mutex attributes, so skipping - elsif(exists $attribute->{simpleType} && (exists $attribute->{simpleType}->{hbmutex}) ) { + elsif(exists $attribute->{simpleType} && (exists $attribute->{simpleType}->{hbmutex}) || (exists $attribute->{simpleType}->{hbrecursivemutex}) ) { print $outFile " //Mutex attributes - skipping\n"; } # makes no sense to dump fsp mutex attributes, so skipping @@ -4880,6 +4894,7 @@ sub simpleTypeProperties { $typesHoH{"uint64_t"} = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 1, typeName => "uint64_t" , bytes => 8, bits => 64, default => \&defaultZero , alignment => 1, specialPolicies =>\&null, packfmt =>\&pack8byte}; $typesHoH{"enumeration"} = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 0, typeName => "XMLTOHB_USE_PARENT_ATTR_ID" , bytes => 0, bits => 0 , default => \&defaultEnum , alignment => 1, specialPolicies =>\&null, packfmt => "packEnumeration"}; $typesHoH{"hbmutex"} = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 0, typeName => "mutex_t*" , bytes => 24, bits => 192, default => \&defaultZero , alignment => 8, specialPolicies =>\&enforceHbMutex, packfmt =>\&packMutex}; + $typesHoH{"hbrecursivemutex"} = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 0, typeName => "mutex_t*" , bytes => 24, bits => 192, default => \&defaultZero , alignment => 8, specialPolicies =>\&enforceHbMutex, packfmt =>\&packMutex}; $typesHoH{"Target_t"} = { supportsArray => 0, canBeHex => 1, complexTypeSupport => 0, typeName => "TARGETING::Target*" , bytes => 8, bits => 64, default => \&defaultZero , alignment => 8, specialPolicies =>\&null, packfmt =>\&pack8byte}; $typesHoH{"fspmutex"} = { supportsArray => 1, canBeHex => 1, complexTypeSupport => 0, typeName => "util::Mutex*" , bytes => 8, bits => 64, default => \&defaultZero , alignment => 8, specialPolicies =>\&enforceFspMutex, packfmt =>\&pack8byte}; @@ -5038,33 +5053,45 @@ sub mergeComplexAttributeFields { # the two fields. my $mergedFields = dclone $currentAttrFields; - # Iterate over the fields of $newField and look for their corresponding id - # in $currentAttrFields. All the fields in $newField should exist in - # $currentAttrFields, if not, then there is a problem - foreach my $newField (@{$newAttrFields->{default}->{field}}) + # If the merged field's (an alias for the current attribute fields) + # hash is empty, then just assign it the new attribute field's value - + # no merge necessary + if ($mergedFields->{default} == 0) { - my $foundField = 0; - - # Iterate over $mergedFields (really $currentAttrFields) looking - # for the $newField of $newAttrFields - foreach my $currentField (@{$mergedFields->{default}->{field}}) + $mergedFields->{default} = $newAttrFields->{default}; + } + # Only proceed if both attribute pairs have values to merge + elsif ($newAttrFields->{default} != 0) + { + # Iterate over the fields of $newField and look for their corresponding + # id in $currentAttrFields. All the fields in $newField should exist in + # $currentAttrFields, if not, then there is a problem + foreach my $newField (@{$newAttrFields->{default}->{field}}) { - # Found the field in question - if ($currentField->{id} eq $newField->{id}) + my $foundField = 0; + + # Iterate over $mergedFields (really $currentAttrFields) looking + # for the $newField of $newAttrFields + foreach my $currentField (@{$mergedFields->{default}->{field}}) { - # Merge in the new value from $newField - $currentField->{value} = $newField->{value}; - $foundField = 1; - last; - } - } # end foreach my $currentField ... + # Found the field in question + if ($currentField->{id} eq $newField->{id}) + { + # Merge in the new value from $newField + $currentField->{value} = $newField->{value}; + $foundField = 1; + last; + } + } # end foreach my $currentField ... - # A field was not found ... halt execution - if ($foundField == 0) - { - croak("Field $newField is not supported.") - } - } # end foreach my $newField ... + # A field was not found ... halt execution + if ($foundField == 0) + { + croak("Field $newField is not supported.") + } + } # end foreach my $newField ... + } + # else new attribute fields is empty - nothing to merge return $mergedFields; } diff --git a/src/usr/targeting/targetservicestart.C b/src/usr/targeting/targetservicestart.C index 20dce1e48..795bd4072 100755 --- a/src/usr/targeting/targetservicestart.C +++ b/src/usr/targeting/targetservicestart.C @@ -221,29 +221,40 @@ static void initTargeting(errlHndl_t& io_pError) } } - initializeAttributes(l_targetService, l_isMpipl, l_isIstepMode, l_scratch); - //Ensure all mutex attributes are reset on MPIPL + initializeAttributes(l_targetService, l_isMpipl, l_isIstepMode, + l_scratch); + + + uint32_t l_peerTargetsAdjusted = 0; + uint32_t l_numberMutexAttrsReset = 0; + if(l_isMpipl) { - // updatePeerTargets will write to read-only attribute pages - // to get around vmm fails we need to allow writes to readonly - // memory for the duration of this loop (unlocking is done above) - uint32_t l_peerTargetsAdjusted = 0; - uint32_t l_numberMutexAttrsReset = 0; + + for( auto targ_iter = l_targetService.begin(); targ_iter != l_targetService.end(); targ_iter++) { const Target* l_pTarget = *targ_iter; - // Check if there any mutex attributes we need to reset on this target - l_numberMutexAttrsReset += l_targetService.resetMutexAttributes(l_pTarget); + + // Ensure all mutex attributes are setup correctly for MPIPL + // Check if there any mutex attributes + // we need to reset on this target + l_numberMutexAttrsReset += + l_targetService.resetMutexAttributes(l_pTarget); // Update any peer target addresses if necessary + // updatePeerTargets will write to read-only attribute pages. + // To get around vmm fails we need to allow writes to readonly + // memory for the duration of this loop (unlocking is done + // above). if(l_targetService.updatePeerTarget(l_pTarget)) { l_peerTargetsAdjusted++; } } + // Now that the loop is complete we can re-apply // the read only permissions to the read only attr pages l_targetService.modifyReadOnlyPagePermissions(false); @@ -252,6 +263,24 @@ static void initTargeting(errlHndl_t& io_pError) TARG_INF("Number of mutex attributes reset: %d", l_numberMutexAttrsReset); } + else + { + + for( auto targ_iter = l_targetService.begin(); + targ_iter != l_targetService.end(); + targ_iter++) + { + const Target* l_pTarget = *targ_iter; + + // Ensure all mutex attributes are setup correctly for IPL + // Recursive Mutexes need to be setup since they are defaulted + // to all zero. + // Check if there any mutex attributes + // we need to reset on this target + l_targetService.resetMutexAttributes(l_pTarget); + } + } + checkProcessorTargeting(l_targetService); diff --git a/src/usr/targeting/test/testtargeting.H b/src/usr/targeting/test/testtargeting.H index c87d599e9..16d2fc99d 100644 --- a/src/usr/targeting/test/testtargeting.H +++ b/src/usr/targeting/test/testtargeting.H @@ -90,6 +90,63 @@ void* funcTestMutex(void* i_pData) return NULL; } +/** + * @brief Function which attempts to recursively access a critical section with + * a recursive mutex. + * + * @param[in] i_pData Pointer to mutex pointer/value pointer structure + * + * @return nullptr + */ +void* funcTestRecursiveMutex(void* i_pData) +{ + MutexTestData_t* l_pData = static_cast<MutexTestData_t*>(i_pData); + + // The parent thread holds the lock initially. So the child thread will be + // blocked when it attempts to access the critical section. + TS_INFO("Child Thread: Attempting to access the Critical Section"); + recursive_mutex_lock(l_pData->pMutex); + + TS_INFO("Child Thread: Entering the Critical Section"); + if (*(l_pData->pVar) > 0) + { + *(l_pData->pVar) = 0; + funcTestRecursiveMutex(l_pData); + } + + TS_INFO("Child Thread: Leaving the Critical Section"); + recursive_mutex_unlock(l_pData->pMutex); + return nullptr; +} + +/** + * @brief Function that serves as an entry point to the recursive function + * it calls. This is so that the barrier_wait calls work without the + * recursion causing a deadlock. + * + * @param[in] i_pData Pointer to mutex pointer/value pointer structure + * + * @return nullptr + */ +void* funcTestRecursiveMutexEntry(void* i_pData) +{ + MutexTestData_t* l_pData = static_cast<MutexTestData_t*>(i_pData); + + // Signal to the parent thread that the child thread is about to access the + // lock which is held by it. That way the main thread can verify that + // this thread hasn't started recursion yet. + barrier_wait(l_pData->pBarrier); + + funcTestRecursiveMutex(l_pData); + + // Wake the parent thread so that it can verify that this thread accessed + // the protected value which proves that the child thread got + // the lock recursively without a deadlock occuring. + barrier_wait(l_pData->pBarrier); + + return nullptr; +} + class TargetingTestSuite : public CxxTest::TestSuite { public: @@ -97,6 +154,122 @@ class TargetingTestSuite : public CxxTest::TestSuite /** * @brief Test Hostboot specific mutex attribute support */ + void testHbRecursiveMutexAttr() + { + TS_TRACE(ENTER_MRK "testHbRecursiveMutexAttr" ); + + using namespace TARGETING; + + do { + + // Get a reference to the target service + TargetService& l_targetService = targetService(); + + // Get the system target containing the test mutex + TARGETING::Target* l_pTarget = nullptr; + (void) l_targetService.getTopLevelTarget(l_pTarget); + if (l_pTarget == nullptr) + { + TS_FAIL("Top level target handle is NULL"); + break; + } + + // Get the mutex attribute (actually a mutex_t* which points to + // a mutex) + HB_RECURSIVE_MUTEX_TEST_LOCK_ATTR l_pLock = + l_pTarget->getHbMutexAttr + <TARGETING::ATTR_HB_RECURSIVE_MUTEX_TEST_LOCK>(); + + // Verify the recursive mutex was initialized correctly + mutex_t* l_recursiveMutex = reinterpret_cast<mutex_t*>(l_pLock); + + if ((l_recursiveMutex->iv_val != 0) + || (l_recursiveMutex->iv_ownerLockCount != 0) + || (l_recursiveMutex->iv_owner != 0) + || (l_recursiveMutex->iv_recursive != true)) + { + TS_FAIL("Mutex attribute must be initialized as recursive."); + break; + } + + // Try to get the attribute, and ensure it's the same + HB_RECURSIVE_MUTEX_TEST_LOCK_ATTR l_pLockTry = nullptr; + if(l_pTarget->tryGetHbMutexAttr + <TARGETING::ATTR_HB_RECURSIVE_MUTEX_TEST_LOCK>(l_pLockTry)) + { + if(l_pLockTry != l_pLock) + { + TS_FAIL("Mutex attributes should match, but dont. " + "l_pLockTry = %p, l_pLock = %p",l_pLockTry, + l_pLock); + break; + } + } + else + { + TS_FAIL("Mutex attribute tryGet failed, even though it exists"); + break; + } + + // Create a structure holding pointers to + // the mutex and a protected value + volatile uint32_t l_var = 1; + TS_INFO("Parent Thread: Acquiring recursive lock"); + (void)recursive_mutex_lock(l_pLock); + barrier_t l_barrier; + (void)barrier_init(&l_barrier, 2); + MutexTestData_t l_mutexTestData = { l_pLock, &l_barrier, &l_var }; + + // Spawn off a function which tries to write the protected value to + // something unexpected. If the mutex is working, the for loop will + // always poll the expected value. + TS_INFO("Parent Thread: Creating Child thread"); + task_create(funcTestRecursiveMutexEntry, + static_cast<void*>(&l_mutexTestData)); + + // Guarantee the child process runs and blocks on the mutex prior to + // modifying the protected value. isync to ensure the processor doesn't + // speculatively perform the comparison prior to the sleep completing + TS_INFO("Parent Thread: Waiting for Child to attempt to grab the lock"); + barrier_wait(&l_barrier); + nanosleep(0,TEN_CTX_SWITCHES_NS); isync(); + + if(l_var != 1) + { + TS_FAIL("Protected value must be 1, was %d instead",l_var); + break; + } + + // Now unlock the mutex, allowing the other thread to overwrite the + // protected value; which should happen within 100,000 reads of the + // var. This will confirm the other thread was actively trying to + // write the controlled value + TS_INFO( + "Parent Thread: Releasing lock to Child to test recursive mutex."); + (void)recursive_mutex_unlock(l_pLock); + + // Guarantee the child process acquires the mutex and modifies the + // protected value. + TS_INFO("Parent Thread: Waiting for Child to modify the value"); + barrier_wait(&l_barrier); + + if(l_var != 0) + { + TS_FAIL("Protected value must now be 0, was %d instead",l_var); + break; + } + + TS_INFO("Parent Thread: Child completed successfully."); + barrier_destroy(&l_barrier); + + } while(0); + + TS_TRACE(EXIT_MRK "testHbRecursiveMutexAttr"); + } + + /** + * @brief Test Hostboot specific mutex attribute support + */ void testHbMutexAttr() { TS_TRACE(ENTER_MRK "testHbMutexAttr" ); @@ -149,7 +322,8 @@ class TargetingTestSuite : public CxxTest::TestSuite break; } - // Create a structue holding pointers to the mutex and a protected value + // Create a structure holding pointers to + // the mutex and a protected value volatile uint32_t l_var = 0; (void)mutex_lock(l_pLock); barrier_t l_barrier; diff --git a/src/usr/testcore/lib/makefile b/src/usr/testcore/lib/makefile index 9bf180042..ce80dc0df 100644 --- a/src/usr/testcore/lib/makefile +++ b/src/usr/testcore/lib/makefile @@ -28,7 +28,7 @@ MODULE = testsyslib #@TODO-RTC:151185-Turn enable all test cases #TESTS = *.H TESTS = stltest.H -#TESTS += synctest.H +TESTS += synctest.H SUBDIRS += runtime.d diff --git a/src/usr/testcore/lib/synctest.H b/src/usr/testcore/lib/synctest.H index 2662f020d..ea71dac53 100644 --- a/src/usr/testcore/lib/synctest.H +++ b/src/usr/testcore/lib/synctest.H @@ -50,10 +50,11 @@ class SyncTest: public CxxTest::TestSuite // deadlocks occur. // Case 1: Single thread locks recursive mutex three times in a row. { - recursive_mutex_init(&mutex); + mutex_t recursive_mutex; + recursive_mutex_init(&recursive_mutex); l_status = TASK_STATUS_EXITED_CLEAN; - tid_t l_task1 = task_create(entry_func, this); + tid_t l_task1 = task_create(entry_func, &recursive_mutex); if ((l_task1 != task_wait_tid(l_task1, &l_status, nullptr)) || (l_status == TASK_STATUS_EXITED_CLEAN)) @@ -66,11 +67,12 @@ class SyncTest: public CxxTest::TestSuite // grab it (hangs) and then the first thread grabs it again // without issue. { - recursive_mutex_init(&mutex); + mutex_t recursive_mutex; + recursive_mutex_init(&recursive_mutex); l_status = TASK_STATUS_EXITED_CLEAN; - tid_t l_task1 = task_create(entry_func, this); - tid_t l_task2 = task_create(entry_func, this); + tid_t l_task1 = task_create(entry_func, &recursive_mutex); + tid_t l_task2 = task_create(entry_func, &recursive_mutex); if ((l_task2 != task_wait_tid(l_task2, &l_status, nullptr)) || (l_status == TASK_STATUS_EXITED_CLEAN)) @@ -205,8 +207,7 @@ class SyncTest: public CxxTest::TestSuite static void* entry_func(void* i_p) { - SyncTest* my = (SyncTest*) i_p; - mutex_t* myMutex = &(my->mutex); + mutex_t* myMutex = (mutex_t*) i_p; // Call the recursive function. recursive_func(myMutex, 2); |
