kernel/sem: Drop GRAB macros in favor of SpinLockers.

No functional change intended.

Change-Id: I24f6f8c55b18f8d031d0b5a87672cc969e141577
Reviewed-on: https://review.haiku-os.org/c/1636
Reviewed-by: waddlesplash <[email protected]>

src/system/kernel/sem.cpp

@@ -129,10 +129,6 @@ static struct sem_entry	*sFreeSemsHead = NULL;
 static struct sem_entry	*sFreeSemsTail = NULL;
 
 static spinlock sSemsSpinlock = B_SPINLOCK_INITIALIZER;
-#define GRAB_SEM_LIST_LOCK()     acquire_spinlock(&sSemsSpinlock)
-#define RELEASE_SEM_LIST_LOCK()  release_spinlock(&sSemsSpinlock)
-#define GRAB_SEM_LOCK(s)         acquire_spinlock(&(s).lock)
-#define RELEASE_SEM_LOCK(s)      release_spinlock(&(s).lock)
 
 
 static int
@@ -322,7 +318,7 @@ fill_sem_info(struct sem_entry* sem, sem_info* info, size_t size)
 	will return that one in \a name.
 */
 static void
-uninit_sem_locked(struct sem_entry& sem, char** _name)
+uninit_sem_locked(struct sem_entry& sem, char** _name, SpinLocker& locker)
 {
 	KTRACE("delete_sem(sem: %ld)", sem.u.used.id);
 
@@ -340,13 +336,12 @@ uninit_sem_locked(struct sem_entry& sem, char** _name)
 	*_name = sem.u.used.name;
 	sem.u.used.name = NULL;
 
-	RELEASE_SEM_LOCK(sem);
+	locker.Unlock();
 
 	// append slot to the free list
-	GRAB_SEM_LIST_LOCK();
+	SpinLocker _(&sSemsSpinlock);
 	free_sem_slot(id % sMaxSems, id + sMaxSems);
 	atomic_add(&sUsedSems, -1);
-	RELEASE_SEM_LIST_LOCK();
 }
 
 
@@ -360,23 +355,17 @@ delete_sem_internal(sem_id id, bool checkPermission)
 
 	int32 slot = id % sMaxSems;
 
-	cpu_status state = disable_interrupts();
+	InterruptsLocker _;
-	GRAB_SEM_LIST_LOCK();
+	SpinLocker listLocker(sSemsSpinlock);
-	GRAB_SEM_LOCK(sSems[slot]);
+	SpinLocker semLocker(sSems[slot].lock);
 
 	if (sSems[slot].id != id) {
-		RELEASE_SEM_LOCK(sSems[slot]);
-		RELEASE_SEM_LIST_LOCK();
-		restore_interrupts(state);
 		TRACE(("delete_sem: invalid sem_id %ld\n", id));
 		return B_BAD_SEM_ID;
 	}
 
 	if (checkPermission
 		&& sSems[slot].u.used.owner == team_get_kernel_team_id()) {
-		RELEASE_SEM_LOCK(sSems[slot]);
-		RELEASE_SEM_LIST_LOCK();
-		restore_interrupts(state);
 		dprintf("thread %" B_PRId32 " tried to delete kernel semaphore "
 			"%" B_PRId32 ".\n", thread_get_current_thread_id(), id);
 		return B_NOT_ALLOWED;
@@ -388,17 +377,15 @@ delete_sem_internal(sem_id id, bool checkPermission)
 	} else
 		panic("sem %" B_PRId32 " has no owner", id);
 
-	RELEASE_SEM_LIST_LOCK();
+	listLocker.Unlock();
 
 	char* name;
-	uninit_sem_locked(sSems[slot], &name);
+	uninit_sem_locked(sSems[slot], &name, semLocker);
 
 	SpinLocker schedulerLocker(thread_get_current_thread()->scheduler_lock);
 	scheduler_reschedule_if_necessary_locked();
 	schedulerLocker.Unlock();
 
-	restore_interrupts(state);
-
 	free(name);
 	return B_OK;
 }
@@ -477,7 +464,6 @@ sem_id
 create_sem_etc(int32 count, const char* name, team_id owner)
 {
 	struct sem_entry* sem = NULL;
-	cpu_status state;
 	sem_id id = B_NO_MORE_SEMS;
 	char* tempName;
 	size_t nameLength;
@@ -503,8 +489,7 @@ create_sem_etc(int32 count, const char* name, team_id owner)
 
 	strlcpy(tempName, name, nameLength);
 
-	state = disable_interrupts();
+	InterruptsSpinLocker _(&sSemsSpinlock);
-	GRAB_SEM_LIST_LOCK();
 
 	// get the first slot from the free list
 	sem = sFreeSemsHead;
@@ -515,7 +500,7 @@ create_sem_etc(int32 count, const char* name, team_id owner)
 			sFreeSemsTail = NULL;
 
 		// init the slot
-		GRAB_SEM_LOCK(*sem);
+		SpinLocker semLocker(sem->lock);
 		sem->id = sem->u.unused.next_id;
 		sem->u.used.count = count;
 		sem->u.used.net_count = count;
@@ -527,7 +512,7 @@ create_sem_etc(int32 count, const char* name, team_id owner)
 
 		list_add_item(&team->sem_list, &sem->u.used.team_link);
 
-		RELEASE_SEM_LOCK(*sem);
+		semLocker.Unlock();
 
 		atomic_add(&sUsedSems, 1);
 
@@ -539,9 +524,6 @@ create_sem_etc(int32 count, const char* name, team_id owner)
 			name);
 	}
 
-	RELEASE_SEM_LIST_LOCK();
-	restore_interrupts(state);
-
 	if (sem == NULL)
 		free(tempName);
 
@@ -552,25 +534,20 @@ create_sem_etc(int32 count, const char* name, team_id owner)
 status_t
 select_sem(int32 id, struct select_info* info, bool kernel)
 {
-	cpu_status state;
-	int32 slot;
-	status_t error = B_OK;
-
 	if (id < 0)
 		return B_BAD_SEM_ID;
 
-	slot = id % sMaxSems;
+	int32 slot = id % sMaxSems;
 
-	state = disable_interrupts();
+	InterruptsSpinLocker _(&sSems[slot].lock);
-	GRAB_SEM_LOCK(sSems[slot]);
 
 	if (sSems[slot].id != id) {
 		// bad sem ID
-		error = B_BAD_SEM_ID;
+		return B_BAD_SEM_ID;
 	} else if (!kernel
 		&& sSems[slot].u.used.owner == team_get_kernel_team_id()) {
 		// kernel semaphore, but call from userland
-		error = B_NOT_ALLOWED;
+		return B_NOT_ALLOWED;
 	} else {
 		info->selected_events &= B_EVENT_ACQUIRE_SEMAPHORE | B_EVENT_INVALID;
 
@@ -583,29 +560,22 @@ select_sem(int32 id, struct select_info* info, bool kernel)
 		}
 	}
 
-	RELEASE_SEM_LOCK(sSems[slot]);
+	return B_OK;
-	restore_interrupts(state);
-
-	return error;
 }
 
 
 status_t
 deselect_sem(int32 id, struct select_info* info, bool kernel)
 {
-	cpu_status state;
-	int32 slot;
-
 	if (id < 0)
 		return B_BAD_SEM_ID;
 
 	if (info->selected_events == 0)
 		return B_OK;
 
-	slot = id % sMaxSems;
+	int32 slot = id % sMaxSems;
 
-	state = disable_interrupts();
+	InterruptsSpinLocker _(&sSems[slot].lock);
-	GRAB_SEM_LOCK(sSems[slot]);
 
 	if (sSems[slot].id == id) {
 		select_info** infoLocation = &sSems[slot].u.used.select_infos;
@@ -616,9 +586,6 @@ deselect_sem(int32 id, struct select_info* info, bool kernel)
 			*infoLocation = info->next;
 	}
 
-	RELEASE_SEM_LOCK(sSems[slot]);
-	restore_interrupts(state);
-
 	return B_OK;
 }
 
@@ -680,16 +647,16 @@ sem_delete_owned_sems(Team* team)
 
 		{
 			// get the next semaphore from the team's sem list
-			InterruptsLocker locker;
+			InterruptsLocker _;
 			SpinLocker semListLocker(sSemsSpinlock);
 			sem_entry* sem = (sem_entry*)list_remove_head_item(&team->sem_list);
 			if (sem == NULL)
 				break;
 
 			// delete the semaphore
-			GRAB_SEM_LOCK(*sem);
+			SpinLocker semLocker(sem->lock);
 			semListLocker.Unlock();
-			uninit_sem_locked(*sem, &name);
+			uninit_sem_locked(*sem, &name, semLocker);
 		}
 
 		free(name);
@@ -756,8 +723,6 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 	uint32 flags, bigtime_t timeout)
 {
 	int slot = id % sMaxSems;
-	int state;
-	status_t status = B_OK;
 
 	if (gKernelStartup)
 		return B_OK;
@@ -777,13 +742,12 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 		return B_BAD_VALUE;
 	}
 
-	state = disable_interrupts();
+	InterruptsLocker _;
-	GRAB_SEM_LOCK(sSems[slot]);
+	SpinLocker semLocker(sSems[slot].lock);
 
 	if (sSems[slot].id != id) {
 		TRACE(("switch_sem_etc: bad sem %ld\n", id));
-		status = B_BAD_SEM_ID;
+		return B_BAD_SEM_ID;
-		goto err;
 	}
 
 	// TODO: the B_CHECK_PERMISSION flag should be made private, as it
@@ -792,19 +756,19 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 		&& sSems[slot].u.used.owner == team_get_kernel_team_id()) {
 		dprintf("thread %" B_PRId32 " tried to acquire kernel semaphore "
 			"%" B_PRId32 ".\n", thread_get_current_thread_id(), id);
-		status = B_NOT_ALLOWED;
+#if 0
-		goto err;
+		_user_debugger("Thread tried to acquire kernel semaphore.");
+#endif
+		return B_NOT_ALLOWED;
 	}
 
 	if (sSems[slot].u.used.count - count < 0) {
 		if ((flags & B_RELATIVE_TIMEOUT) != 0 && timeout <= 0) {
 			// immediate timeout
-			status = B_WOULD_BLOCK;
+			return B_WOULD_BLOCK;
-			goto err;
 		} else if ((flags & B_ABSOLUTE_TIMEOUT) != 0 && timeout < 0) {
 			// absolute negative timeout
-			status = B_TIMED_OUT;
+			return B_TIMED_OUT;
-			goto err;
 		}
 	}
 
@@ -825,9 +789,12 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 		if (thread_is_interrupted(thread, flags)) {
 			schedulerLocker.Unlock();
 			sSems[slot].u.used.count += count;
-			status = B_INTERRUPTED;
+			if (semToBeReleased >= B_OK) {
-				// the other semaphore will be released later
+				// we need to still release the semaphore to always
-			goto err;
+				// leave in a consistent state
+				release_sem_etc(semToBeReleased, 1, B_DO_NOT_RESCHEDULE);
+			}
+			return B_INTERRUPTED;
 		}
 
 		schedulerLocker.Unlock();
@@ -843,7 +810,7 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 		thread_prepare_to_block(thread, flags, THREAD_BLOCK_TYPE_SEMAPHORE,
 			(void*)(addr_t)id);
 
-		RELEASE_SEM_LOCK(sSems[slot]);
+		semLocker.Unlock();
 
 		// release the other semaphore, if any
 		if (semToBeReleased >= 0) {
@@ -854,7 +821,7 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 		status_t acquireStatus = timeout == B_INFINITE_TIMEOUT
 			? thread_block() : thread_block_with_timeout(flags, timeout);
 
-		GRAB_SEM_LOCK(sSems[slot]);
+		semLocker.Lock();
 
 		// If we're still queued, this means the acquiration failed, and we
 		// need to remove our entry and (potentially) wake up other threads.
@@ -868,8 +835,7 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 #endif
 		}
 
-		RELEASE_SEM_LOCK(sSems[slot]);
+		semLocker.Unlock();
-		restore_interrupts(state);
 
 		TRACE(("switch_sem_etc(sem %ld): exit block name %s, "
 			"thread %ld (%s)\n", id, sSems[slot].u.used.name, thread->id,
@@ -884,25 +850,9 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count,
 #endif
 	}
 
-err:
-	RELEASE_SEM_LOCK(sSems[slot]);
-	restore_interrupts(state);
-
-	if (status == B_INTERRUPTED && semToBeReleased >= B_OK) {
-		// depending on when we were interrupted, we need to still
-		// release the semaphore to always leave in a consistent
-		// state
-		release_sem_etc(semToBeReleased, 1, B_DO_NOT_RESCHEDULE);
-	}
-
-#if 0
-	if (status == B_NOT_ALLOWED)
-	_user_debugger("Thread tried to acquire kernel semaphore.");
-#endif
-
 	KTRACE("switch_sem_etc() done: 0x%lx", status);
 
-	return status;
+	return B_OK;
 }
 
 
@@ -1028,9 +978,6 @@ release_sem_etc(sem_id id, int32 count, uint32 flags)
 status_t
 get_sem_count(sem_id id, int32 *_count)
 {
-	int slot;
-	int state;
-
 	if (sSemsActive == false)
 		return B_NO_MORE_SEMS;
 	if (id < 0)
@@ -1038,23 +985,17 @@ get_sem_count(sem_id id, int32 *_count)
 	if (_count == NULL)
 		return B_BAD_VALUE;
 
-	slot = id % sMaxSems;
+	int slot = id % sMaxSems;
 
-	state = disable_interrupts();
+	InterruptsSpinLocker _(sSems[slot].lock);
-	GRAB_SEM_LOCK(sSems[slot]);
 
 	if (sSems[slot].id != id) {
-		RELEASE_SEM_LOCK(sSems[slot]);
-		restore_interrupts(state);
 		TRACE(("sem_get_count: invalid sem_id %ld\n", id));
 		return B_BAD_SEM_ID;
 	}
 
 	*_count = sSems[slot].u.used.count;
 
-	RELEASE_SEM_LOCK(sSems[slot]);
-	restore_interrupts(state);
-
 	return B_OK;
 }
 
@@ -1063,10 +1004,6 @@ get_sem_count(sem_id id, int32 *_count)
 status_t
 _get_sem_info(sem_id id, struct sem_info *info, size_t size)
 {
-	status_t status = B_OK;
-	int state;
-	int slot;
-
 	if (!sSemsActive)
 		return B_NO_MORE_SEMS;
 	if (id < 0)
@@ -1074,21 +1011,17 @@ _get_sem_info(sem_id id, struct sem_info *info, size_t size)
 	if (info == NULL || size != sizeof(sem_info))
 		return B_BAD_VALUE;
 
-	slot = id % sMaxSems;
+	int slot = id % sMaxSems;
 
-	state = disable_interrupts();
+	InterruptsSpinLocker _(sSems[slot].lock);
-	GRAB_SEM_LOCK(sSems[slot]);
 
 	if (sSems[slot].id != id) {
-		status = B_BAD_SEM_ID;
 		TRACE(("get_sem_info: invalid sem_id %ld\n", id));
+		return B_BAD_SEM_ID;
 	} else
 		fill_sem_info(&sSems[slot], info, size);
 
-	RELEASE_SEM_LOCK(sSems[slot]);
+	return B_OK;
-	restore_interrupts(state);
-
-	return status;
 }
 
 
@@ -1127,7 +1060,7 @@ _get_next_sem_info(team_id teamID, int32 *_cookie, struct sem_info *info,
 		if (sem == NULL)
 			return B_BAD_VALUE;
 
-		GRAB_SEM_LOCK(*sem);
+		SpinLocker _(sem->lock);
 
 		if (sem->id != -1 && sem->u.used.owner == team->id) {
 			// found one!
@@ -1136,8 +1069,6 @@ _get_next_sem_info(team_id teamID, int32 *_cookie, struct sem_info *info,
 			found = true;
 		} else
 			newIndex++;
-
-		RELEASE_SEM_LOCK(*sem);
 	}
 
 	if (!found)