Lv2 Rwlock rewritten

rpcs3/Emu/SysCalls/lv2/sys_event.cpp

@@ -35,10 +35,8 @@ s32 sys_event_queue_create(vm::ptr<u32> equeue_id, vm::ptr<sys_event_queue_attr>
 
 	switch (protocol)
 	{
-	case SYS_SYNC_PRIORITY: break;
-	case SYS_SYNC_RETRY: sys_event.Error("sys_event_queue_create(): invalid protocol (SYS_SYNC_RETRY)"); return CELL_EINVAL;
-	case SYS_SYNC_PRIORITY_INHERIT: sys_event.Error("sys_event_queue_create(): invalid protocol (SYS_SYNC_PRIORITY_INHERIT)"); return CELL_EINVAL;
 	case SYS_SYNC_FIFO: break;
+	case SYS_SYNC_PRIORITY: break;
 	default: sys_event.Error("sys_event_queue_create(): unknown protocol (0x%x)", protocol); return CELL_EINVAL;
 	}
 

rpcs3/Emu/SysCalls/lv2/sys_event_flag.cpp

@@ -27,10 +27,10 @@ s32 sys_event_flag_create(vm::ptr<u32> id, vm::ptr<sys_event_flag_attr> attr, u6
 
 	switch (protocol)
 	{
-	case SYS_SYNC_PRIORITY: break;
-	case SYS_SYNC_RETRY: sys_event_flag.Todo("sys_event_flag_create(): SYS_SYNC_RETRY"); break;
-	case SYS_SYNC_PRIORITY_INHERIT: sys_event_flag.Todo("sys_event_flag_create(): SYS_SYNC_PRIORITY_INHERIT"); break;
 	case SYS_SYNC_FIFO: break;
+	case SYS_SYNC_RETRY: break;
+	case SYS_SYNC_PRIORITY: break;
+	case SYS_SYNC_PRIORITY_INHERIT: break;
 	default: sys_event_flag.Error("sys_event_flag_create(): unknown protocol (0x%x)", attr->protocol); return CELL_EINVAL;
 	}
 

rpcs3/Emu/SysCalls/lv2/sys_mutex.cpp

@@ -29,8 +29,7 @@ s32 sys_mutex_create(vm::ptr<u32> mutex_id, vm::ptr<sys_mutex_attribute_t> attr)
 	{
 	case SYS_SYNC_FIFO: break;
 	case SYS_SYNC_PRIORITY: break;
-	case SYS_SYNC_PRIORITY_INHERIT: sys_mutex.Todo("sys_mutex_create(): SYS_SYNC_PRIORITY_INHERIT"); break;
+	case SYS_SYNC_PRIORITY_INHERIT: break;
-	case SYS_SYNC_RETRY: sys_mutex.Error("sys_mutex_create(): invalid protocol (SYS_SYNC_RETRY)"); return CELL_EINVAL;
 	default: sys_mutex.Error("sys_mutex_create(): unknown protocol (0x%x)", protocol); return CELL_EINVAL;
 	}
 

rpcs3/Emu/SysCalls/lv2/sys_rwlock.cpp

@@ -13,35 +13,33 @@ SysCallBase sys_rwlock("sys_rwlock");
 
 s32 sys_rwlock_create(vm::ptr<u32> rw_lock_id, vm::ptr<sys_rwlock_attribute_t> attr)
 {
-	sys_rwlock.Warning("sys_rwlock_create(rw_lock_id_addr=0x%x, attr_addr=0x%x)", rw_lock_id.addr(), attr.addr());
+	sys_rwlock.Warning("sys_rwlock_create(rw_lock_id=*0x%x, attr=*0x%x)", rw_lock_id, attr);
 
-	if (!attr)
+	if (!rw_lock_id || !attr)
 	{
-		sys_rwlock.Error("sys_rwlock_create(): null attr address");
 		return CELL_EFAULT;
 	}
 
-	switch (attr->protocol.data())
+	const u32 protocol = attr->protocol;
+
+	switch (protocol)
 	{
-	case se32(SYS_SYNC_PRIORITY): break;
+	case SYS_SYNC_FIFO: break;
-	case se32(SYS_SYNC_RETRY): sys_rwlock.Error("Invalid protocol (SYS_SYNC_RETRY)"); return CELL_EINVAL;
+	case SYS_SYNC_PRIORITY: break;
-	case se32(SYS_SYNC_PRIORITY_INHERIT): sys_rwlock.Todo("SYS_SYNC_PRIORITY_INHERIT"); break;
+	case SYS_SYNC_PRIORITY_INHERIT: break;
-	case se32(SYS_SYNC_FIFO): break;
+	default: sys_rwlock.Error("sys_rwlock_create(): unknown protocol (0x%x)", protocol); return CELL_EINVAL;
-	default: sys_rwlock.Error("Unknown protocol (0x%x)", attr->protocol); return CELL_EINVAL;
 	}
 
-	if (attr->pshared.data() != se32(0x200))
+	if (attr->pshared.data() != se32(0x200) || attr->ipc_key.data() || attr->flags.data())
 	{
-		sys_rwlock.Error("Unknown pshared attribute (0x%x)", attr->pshared);
+		sys_rwlock.Error("sys_rwlock_create(): unknown attributes (pshared=0x%x, ipc_key=0x%llx, flags=0x%x)", attr->pshared, attr->ipc_key, attr->flags);
 		return CELL_EINVAL;
 	}
 
-	std::shared_ptr<RWLock> rw(new RWLock(attr->protocol, attr->name_u64));
+	std::shared_ptr<rwlock_t> rwlock(new rwlock_t(attr->protocol, attr->name_u64));
-	const u32 id = Emu.GetIdManager().GetNewID(rw, TYPE_RWLOCK);
+
-	*rw_lock_id = id;
+	*rw_lock_id = Emu.GetIdManager().GetNewID(rwlock, TYPE_RWLOCK);
-	rw->wqueue.set_full_name(fmt::Format("Rwlock(%d)", id));
 
-	sys_rwlock.Warning("*** rwlock created [%s] (protocol=0x%x): id = %d", std::string(attr->name, 8).c_str(), rw->protocol, id);
 	return CELL_OK;
 }
 
@@ -49,52 +47,40 @@ s32 sys_rwlock_destroy(u32 rw_lock_id)
 {
 	sys_rwlock.Warning("sys_rwlock_destroy(rw_lock_id=%d)", rw_lock_id);
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	if (!rw->sync.compare_and_swap_test({ 0, 0 }, { ~0u, ~0u })) // check if locked and make unusable
+	if (rwlock.use_count() > 2 || rwlock->readers || rwlock->writer || rwlock->waiters)
 	{
 		return CELL_EBUSY;
 	}
 
 	Emu.GetIdManager().RemoveID(rw_lock_id);
+
 	return CELL_OK;
 }
 
 s32 sys_rwlock_rlock(u32 rw_lock_id, u64 timeout)
 {
-	sys_rwlock.Log("sys_rwlock_rlock(rw_lock_id=%d, timeout=%lld)", rw_lock_id, timeout);
+	sys_rwlock.Log("sys_rwlock_rlock(rw_lock_id=%d, timeout=0x%llx)", rw_lock_id, timeout);
 
 	const u64 start_time = get_system_time();
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	while (true)
+	while (rwlock->writer || rwlock->waiters)
 	{
-		bool succeeded;
-		rw->sync.atomic_op_sync([&succeeded](RWLock::sync_var_t& sync)
-		{
-			assert(~sync.readers);
-			if ((succeeded = !sync.writer))
-			{
-				sync.readers++;
-			}
-		});
-
-		if (succeeded)
-		{
-			break;
-		}
-
-		std::this_thread::sleep_for(std::chrono::milliseconds(1)); // hack
-
 		if (timeout && get_system_time() - start_time > timeout)
 		{
 			return CELL_ETIMEDOUT;
@@ -105,8 +91,12 @@ s32 sys_rwlock_rlock(u32 rw_lock_id, u64 timeout)
 			sys_rwlock.Warning("sys_rwlock_rlock(id=%d) aborted", rw_lock_id);
 			return CELL_OK;
 		}
+
+		rwlock->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
 	}
 
+	rwlock->readers++;
+
 	return CELL_OK;
 }
 
@@ -114,100 +104,76 @@ s32 sys_rwlock_tryrlock(u32 rw_lock_id)
 {
 	sys_rwlock.Log("sys_rwlock_tryrlock(rw_lock_id=%d)", rw_lock_id);
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	bool succeeded;
+	if (rwlock->writer || rwlock->waiters)
-	rw->sync.atomic_op_sync([&succeeded](RWLock::sync_var_t& sync)
 	{
-		assert(~sync.readers);
+		return CELL_EBUSY;
-		if ((succeeded = !sync.writer))
-		{
-			sync.readers++;
-		}
-	});
-
-	if (succeeded)
-	{
-		return CELL_OK;
 	}
 
-	return CELL_EBUSY;
+	rwlock->readers++;
+
+	return CELL_OK;
 }
 
 s32 sys_rwlock_runlock(u32 rw_lock_id)
 {
 	sys_rwlock.Log("sys_rwlock_runlock(rw_lock_id=%d)", rw_lock_id);
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	bool succeeded;
+	if (!rwlock->readers)
-	rw->sync.atomic_op_sync([&succeeded](RWLock::sync_var_t& sync)
 	{
-		if ((succeeded = sync.readers != 0))
+		return CELL_EPERM;
-		{
-			assert(!sync.writer);
-			sync.readers--;
-		}
-	});
-
-	if (succeeded)
-	{
-		return CELL_OK;
 	}
 
-	return CELL_EPERM;
+	if (!--rwlock->readers)
+	{
+		rwlock->cv.notify_one();
+	}
+
+	return CELL_OK;
 }
 
 s32 sys_rwlock_wlock(PPUThread& CPU, u32 rw_lock_id, u64 timeout)
 {
-	sys_rwlock.Log("sys_rwlock_wlock(rw_lock_id=%d, timeout=%lld)", rw_lock_id, timeout);
+	sys_rwlock.Log("sys_rwlock_wlock(rw_lock_id=%d, timeout=0x%llx)", rw_lock_id, timeout);
 
 	const u64 start_time = get_system_time();
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	const u32 tid = CPU.GetId();
+	if (rwlock->writer == CPU.GetId())
-
-	if (rw->sync.compare_and_swap_test({ 0, 0 }, { 0, tid }))
-	{
-		return CELL_OK;
-	}
-
-	if (rw->sync.read_relaxed().writer == tid)
 	{
 		return CELL_EDEADLK;
 	}
 
-	rw->wqueue.push(tid, rw->protocol);
+	// protocol is ignored in current implementation
+	rwlock->waiters++; assert(rwlock->waiters > 0);
 
-	while (true)
+	while (rwlock->readers || rwlock->writer)
 	{
-		auto old_sync = rw->sync.compare_and_swap({ 0, 0 }, { 0, tid });
-		if (!old_sync.readers && (!old_sync.writer || old_sync.writer == tid))
-		{
-			break;
-		}
-
-		std::this_thread::sleep_for(std::chrono::milliseconds(1)); // hack
-
 		if (timeout && get_system_time() - start_time > timeout)
 		{
-			if (!rw->wqueue.invalidate(tid, rw->protocol))
+			rwlock->waiters--; assert(rwlock->waiters >= 0);
-			{
-				assert(!"sys_rwlock_wlock() failed (timeout)");
-			}
 			return CELL_ETIMEDOUT;
 		}
 
@@ -216,12 +182,13 @@ s32 sys_rwlock_wlock(PPUThread& CPU, u32 rw_lock_id, u64 timeout)
 			sys_rwlock.Warning("sys_rwlock_wlock(id=%d) aborted", rw_lock_id);
 			return CELL_OK;
 		}
+
+		rwlock->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
 	}
 
-	if (!rw->wqueue.invalidate(tid, rw->protocol) && !rw->wqueue.pop(tid, rw->protocol))
+	rwlock->writer = CPU.GetId();
-	{
+	rwlock->waiters--; assert(rwlock->waiters >= 0);
-		assert(!"sys_rwlock_wlock() failed (locking)");
+
-	}
 	return CELL_OK;
 }
 
@@ -229,48 +196,48 @@ s32 sys_rwlock_trywlock(PPUThread& CPU, u32 rw_lock_id)
 {
 	sys_rwlock.Log("sys_rwlock_trywlock(rw_lock_id=%d)", rw_lock_id);
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	const u32 tid = CPU.GetId();
+	if (rwlock->writer == CPU.GetId())
-
-	if (rw->sync.compare_and_swap_test({ 0, 0 }, { 0, tid }))
-	{
-		return CELL_OK;
-	}
-
-	if (rw->sync.read_relaxed().writer == tid)
 	{
 		return CELL_EDEADLK;
 	}
 
-	return CELL_EBUSY;
+	if (rwlock->readers || rwlock->writer || rwlock->waiters)
+	{
+		return CELL_EBUSY;
+	}
+
+	rwlock->writer = CPU.GetId();
+
+	return CELL_OK;
 }
 
 s32 sys_rwlock_wunlock(PPUThread& CPU, u32 rw_lock_id)
 {
 	sys_rwlock.Log("sys_rwlock_wunlock(rw_lock_id=%d)", rw_lock_id);
 
-	std::shared_ptr<RWLock> rw;
+	LV2_LOCK;
-	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rw))
+
+	std::shared_ptr<rwlock_t> rwlock;
+	if (!Emu.GetIdManager().GetIDData(rw_lock_id, rwlock))
 	{
 		return CELL_ESRCH;
 	}
 
-	const u32 tid = CPU.GetId();
+	if (rwlock->writer != CPU.GetId())
-	const u32 target = rw->wqueue.signal(rw->protocol);
-
-	if (rw->sync.compare_and_swap_test({ 0, tid }, { 0, target }))
 	{
-		if (!target)
+		return CELL_EPERM;
-		{
-			// TODO: signal readers
-		}
-		return CELL_OK;
 	}
 
-	return CELL_EPERM;
+	rwlock->writer = 0;
+	rwlock->cv.notify_all();
+
+	return CELL_OK;
 }

rpcs3/Emu/SysCalls/lv2/sys_rwlock.h

@@ -7,6 +7,7 @@ struct sys_rwlock_attribute_t
 	be_t<u64> ipc_key;
 	be_t<s32> flags;
 	be_t<u32> pad;
+
 	union
 	{
 		char name[8];
@@ -14,24 +15,25 @@ struct sys_rwlock_attribute_t
 	};
 };
 
-struct RWLock
+struct rwlock_t
 {
-	struct sync_var_t
-	{
-		u32 readers; // reader count
-		u32 writer; // writer thread id
-	};
-
-	sleep_queue_t wqueue;
-	atomic_le_t<sync_var_t> sync;
-
 	const u32 protocol;
+	const u64 name;
 
-	RWLock(u32 protocol, u64 name)
+	std::atomic<u32> readers; // reader count
+	std::atomic<u32> writer; // writer id
+
+	// TODO: use sleep queue, possibly remove condition variable
+	std::condition_variable cv;
+	std::atomic<s32> waiters;
+
+	rwlock_t(u32 protocol, u64 name)
 		: protocol(protocol)
-		, wqueue(name)
+		, name(name)
+		, readers(0)
+		, writer(0)
+		, waiters(0)
 	{
-		sync.write_relaxed({ 0, 0 });
 	}
 };
 

rpcs3/Emu/SysCalls/lv2/sys_semaphore.cpp

@@ -50,8 +50,7 @@ s32 sys_semaphore_create(vm::ptr<u32> sem, vm::ptr<sys_semaphore_attribute> attr
 	{
 	case se32(SYS_SYNC_FIFO): break;
 	case se32(SYS_SYNC_PRIORITY): break;
-	case se32(SYS_SYNC_PRIORITY_INHERIT): sys_semaphore.Todo("SYS_SYNC_PRIORITY_INHERIT"); break;
+	case se32(SYS_SYNC_PRIORITY_INHERIT): break;
-	case se32(SYS_SYNC_RETRY): sys_semaphore.Error("Invalid protocol (SYS_SYNC_RETRY)"); return CELL_EINVAL;
 	default: sys_semaphore.Error("Unknown protocol attribute (0x%x)", attr->protocol); return CELL_EINVAL;
 	}