hrev53286 adds 1 changeset to branch 'master'
old head: 91ec679f17eac49beb7356ef0eaa39df099fd639
new head: 0a564c2cc0cafa80fb92708f4e13f0bebac14e66
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=0a564c2cc0ca+%5E91ec679f17ea
----------------------------------------------------------------------------
0a564c2cc0ca: 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 <waddlesplash@xxxxxxxxx>
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
Revision: hrev53286
Commit: 0a564c2cc0cafa80fb92708f4e13f0bebac14e66
URL: https://git.haiku-os.org/haiku/commit/?id=0a564c2cc0ca
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Sat Jul 20 15:07:48 2019 UTC
Committer: waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Sun Jul 21 21:14:40 2019 UTC
----------------------------------------------------------------------------
1 file changed, 47 insertions(+), 116 deletions(-)
src/system/kernel/sem.cpp | 163 ++++++++++++------------------------------
----------------------------------------------------------------------------
diff --git a/src/system/kernel/sem.cpp b/src/system/kernel/sem.cpp
index 841df6fe45..8f7be82303 100644
--- a/src/system/kernel/sem.cpp
+++ b/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();
- GRAB_SEM_LIST_LOCK();
- GRAB_SEM_LOCK(sSems[slot]);
+ InterruptsLocker _;
+ SpinLocker listLocker(sSemsSpinlock);
+ 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();
- GRAB_SEM_LIST_LOCK();
+ InterruptsSpinLocker _(&sSemsSpinlock);
// 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();
- GRAB_SEM_LOCK(sSems[slot]);
+ InterruptsSpinLocker _(&sSems[slot].lock);
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]);
- restore_interrupts(state);
-
- return error;
+ return B_OK;
}
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();
- GRAB_SEM_LOCK(sSems[slot]);
+ InterruptsSpinLocker _(&sSems[slot].lock);
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();
- GRAB_SEM_LOCK(sSems[slot]);
+ InterruptsLocker _;
+ SpinLocker semLocker(sSems[slot].lock);
if (sSems[slot].id != id) {
TRACE(("switch_sem_etc: bad sem %ld\n", id));
- status = B_BAD_SEM_ID;
- goto err;
+ return B_BAD_SEM_ID;
}
// 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;
- goto err;
+#if 0
+ _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;
- goto err;
+ return B_WOULD_BLOCK;
} else if ((flags & B_ABSOLUTE_TIMEOUT) != 0 && timeout < 0) {
// absolute negative timeout
- status = B_TIMED_OUT;
- goto err;
+ return B_TIMED_OUT;
}
}
@@ -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;
- // the other semaphore will be released later
- goto err;
+ if (semToBeReleased >= B_OK) {
+ // we need to still release the semaphore to
always
+ // 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]);
- restore_interrupts(state);
+ semLocker.Unlock();
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();
- GRAB_SEM_LOCK(sSems[slot]);
+ InterruptsSpinLocker _(sSems[slot].lock);
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();
- GRAB_SEM_LOCK(sSems[slot]);
+ InterruptsSpinLocker _(sSems[slot].lock);
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]);
- restore_interrupts(state);
-
- return status;
+ return B_OK;
}
@@ -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)
