os: bug fixes after re-review of rwlock code

This commit is contained in:
Michael Scire 2020-04-21 22:40:45 -07:00
parent 97cba5e881
commit 4f50f57bb7
3 changed files with 14 additions and 16 deletions

View file

@ -204,9 +204,9 @@ namespace ams::kern {
u8 irq_access_flags[IrqFlagCount]{}; u8 irq_access_flags[IrqFlagCount]{};
u64 core_mask{}; u64 core_mask{};
u64 priority_mask{}; u64 priority_mask{};
util::BitPack32 debug_capabilities; util::BitPack32 debug_capabilities{0};
s32 handle_table_size{}; s32 handle_table_size{};
util::BitPack32 intended_kernel_version; util::BitPack32 intended_kernel_version{0};
u32 program_type{}; u32 program_type{};
private: private:
static constexpr ALWAYS_INLINE void SetSvcAllowedImpl(u8 *data, u32 id) { static constexpr ALWAYS_INLINE void SetSvcAllowedImpl(u8 *data, u32 id) {
@ -254,7 +254,7 @@ namespace ams::kern {
Result SetCapability(const util::BitPack32 cap, u32 &set_flags, u32 &set_svc, KProcessPageTable *page_table); Result SetCapability(const util::BitPack32 cap, u32 &set_flags, u32 &set_svc, KProcessPageTable *page_table);
Result SetCapabilities(const u32 *caps, s32 num_caps, KProcessPageTable *page_table); Result SetCapabilities(const u32 *caps, s32 num_caps, KProcessPageTable *page_table);
public: public:
constexpr KCapabilities() : debug_capabilities(0), intended_kernel_version(0) { /* ... */ } constexpr KCapabilities() = default;
Result Initialize(const u32 *caps, s32 num_caps, KProcessPageTable *page_table); Result Initialize(const u32 *caps, s32 num_caps, KProcessPageTable *page_table);

View file

@ -87,6 +87,8 @@ namespace ams::os::impl {
expected = GetLockCount(rw_lock); expected = GetLockCount(rw_lock);
do { do {
lock_count = expected; lock_count = expected;
AMS_ASSERT(GetWriteLocked(lock_count) == 0 || GetThreadHandle(lock_count) != svc::InvalidHandle);
if (GetWriteLocked(lock_count) != 0 || GetWriteLockWaiterCount(lock_count) != 0) { if (GetWriteLocked(lock_count) != 0 || GetWriteLockWaiterCount(lock_count) != 0) {
if (!arbitrated) { if (!arbitrated) {
IncReadLockWaiterCount(lock_count); IncReadLockWaiterCount(lock_count);
@ -113,7 +115,7 @@ namespace ams::os::impl {
const u32 new_handle = GetThreadHandle(GetLockCount(rw_lock)); const u32 new_handle = GetThreadHandle(GetLockCount(rw_lock));
if ((new_handle | svc::HandleWaitMask) == (cur_handle | svc::HandleWaitMask)) { if ((new_handle | svc::HandleWaitMask) == (cur_handle | svc::HandleWaitMask)) {
SetThreadHandle(lock_count, new_handle); lock_count = GetLockCount(rw_lock);
break; break;
} }
@ -135,7 +137,7 @@ namespace ams::os::impl {
IncReadLockCount(lock_count); IncReadLockCount(lock_count);
SetThreadHandle(lock_count, (GetThreadHandle(lock_count) & svc::HandleWaitMask) ? GetThreadHandle(lock_count) : svc::InvalidHandle); SetThreadHandle(lock_count, (GetThreadHandle(lock_count) & svc::HandleWaitMask) ? GetThreadHandle(lock_count) : svc::InvalidHandle);
} while(!ATOMIC_COMPARE_EXCHANGE_LOCK_COUNT(GetLockCount(rw_lock), expected, lock_count, __ATOMIC_RELEASE, __ATOMIC_RELAXED)); } while(!ATOMIC_COMPARE_EXCHANGE_LOCK_COUNT(GetLockCount(rw_lock), expected, lock_count, __ATOMIC_RELEASE, __ATOMIC_RELAXED));
if (GetThreadHandle(lock_count)) { if (GetThreadHandle(lock_count) & svc::HandleWaitMask) {
R_ABORT_UNLESS(svc::ArbitrateUnlock(GetThreadHandleAddress(GetLockCount(rw_lock)))); R_ABORT_UNLESS(svc::ArbitrateUnlock(GetThreadHandleAddress(GetLockCount(rw_lock))));
} }
} }
@ -202,7 +204,7 @@ namespace ams::os::impl {
R_ABORT_UNLESS(svc::ArbitrateLock(GetThreadHandle(lock_count) & ~svc::HandleWaitMask, GetThreadHandleAddress(GetLockCount(rw_lock)), cur_handle)); R_ABORT_UNLESS(svc::ArbitrateLock(GetThreadHandle(lock_count) & ~svc::HandleWaitMask, GetThreadHandleAddress(GetLockCount(rw_lock)), cur_handle));
expected = GetLockCount(rw_lock); expected = GetLockCount(rw_lock);
} while ((GetThreadHandle(GetLockCount(rw_lock)) | svc::HandleWaitMask) != (GetThreadHandle(lock_count) | svc::HandleWaitMask)); } while ((GetThreadHandle(GetLockCount(rw_lock)) | svc::HandleWaitMask) != (cur_handle | svc::HandleWaitMask));
do { do {
lock_count = expected; lock_count = expected;
@ -228,11 +230,12 @@ namespace ams::os::impl {
AMS_ASSERT(::ams::svc::GetThreadLocalRegion()->disable_count == 0); AMS_ASSERT(::ams::svc::GetThreadLocalRegion()->disable_count == 0);
auto *cur_thread = impl::GetCurrentThread(); auto *cur_thread = impl::GetCurrentThread();
const u32 cur_handle = cur_thread->thread_impl->handle;
if (rw_lock->owner_thread == cur_thread) { if (rw_lock->owner_thread == cur_thread) {
AMS_ASSERT(GetWriteLocked(GetLockCount(rw_lock)) == 1); AMS_ASSERT(GetWriteLocked(GetLockCount(rw_lock)) == 1);
AMS_ASSERT((GetThreadHandle(GetLockCount(rw_lock)) | svc::HandleWaitMask) == (cur_handle | svc::HandleWaitMask));
IncWriteLockCount(*rw_lock); IncWriteLockCount(*rw_lock);
} else { } else {
const u32 cur_handle = cur_thread->thread_impl->handle;
alignas(alignof(u64)) LockCount expected; alignas(alignof(u64)) LockCount expected;
alignas(alignof(u64)) LockCount lock_count; alignas(alignof(u64)) LockCount lock_count;
bool arbitrated = false; bool arbitrated = false;
@ -269,7 +272,7 @@ namespace ams::os::impl {
R_ABORT_UNLESS(svc::ArbitrateLock(GetThreadHandle(lock_count) & ~svc::HandleWaitMask, GetThreadHandleAddress(GetLockCount(rw_lock)), cur_handle)); R_ABORT_UNLESS(svc::ArbitrateLock(GetThreadHandle(lock_count) & ~svc::HandleWaitMask, GetThreadHandleAddress(GetLockCount(rw_lock)), cur_handle));
arbitrated = true; arbitrated = true;
expected = GetLockCount(rw_lock); expected = GetLockCount(rw_lock);
if ((GetThreadHandle(lock_count) | svc::HandleWaitMask) != (cur_handle | svc::HandleWaitMask)) { if ((GetThreadHandle(expected) | svc::HandleWaitMask) != (cur_handle | svc::HandleWaitMask)) {
continue; continue;
} }
} }
@ -282,13 +285,8 @@ namespace ams::os::impl {
GetReference(rw_lock->cv_write_lock._storage).Wait(GetPointer(GetLockCount(rw_lock).cs_storage)); GetReference(rw_lock->cv_write_lock._storage).Wait(GetPointer(GetLockCount(rw_lock).cs_storage));
expected = GetLockCount(rw_lock); expected = GetLockCount(rw_lock);
lock_count = expected; lock_count = expected;
if (GetWriteLocked(lock_count) == 1) {
break;
}
}
if (GetWriteLocked(lock_count) == 1) {
continue;
} }
AMS_ASSERT(GetWriteLocked(lock_count) == 0);
DecWriteLockWaiterCount(lock_count); DecWriteLockWaiterCount(lock_count);
SetWriteLocked(lock_count); SetWriteLocked(lock_count);

View file

@ -222,7 +222,7 @@ namespace ams::ldr::caps {
} }
static constexpr util::BitPack32 Encode(u32 app_type) { static constexpr util::BitPack32 Encode(u32 app_type) {
util::BitPack32 encoded(IdBitsValue); util::BitPack32 encoded{IdBitsValue};
encoded.Set<ApplicationType>(app_type); encoded.Set<ApplicationType>(app_type);
return encoded; return encoded;
} }
@ -275,7 +275,7 @@ namespace ams::ldr::caps {
} }
static constexpr util::BitPack32 Encode(bool allow_debug, bool force_debug) { static constexpr util::BitPack32 Encode(bool allow_debug, bool force_debug) {
util::BitPack32 encoded(IdBitsValue); util::BitPack32 encoded{IdBitsValue};
encoded.Set<AllowDebug>(allow_debug); encoded.Set<AllowDebug>(allow_debug);
encoded.Set<ForceDebug>(force_debug); encoded.Set<ForceDebug>(force_debug);
return encoded; return encoded;