[haiku-commits] BRANCH pdziepak-github.scheduler [c8dd9f7] in src/system/kernel: . locks posix vm fs
- From: pdziepak-github.scheduler <community@xxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 30 Oct 2013 04:00:33 +0100 (CET)
added 3 changesets to branch 'refs/remotes/pdziepak-github/scheduler'
old head: 9c0ff0eed12150e4b27b266af581b7d4758019a3
new head: c8dd9f7780c426e592a3ccb231e6bfab51f15eb9
overview: https://github.com/pdziepak/Haiku/compare/9c0ff0e...c8dd9f7
----------------------------------------------------------------------------
d54a9e0: kernel: Do not use gSchedulerLock when accesing UID and GID
Reads and writes to uid_t and gid_t are atomic anyway. The only real
problem that may happen here is inconsistent state of triples
effective_{u, g}id, saved_set_{u, g}id, real_{u, g}id, but team locks
protect us against that.
d70728f: kernel/lock: Do not use *_locked() functions when not needed
c8dd9f7: kernel: Add thread_unblock() and use it where possible
[ Pawel Dziepak <pdziepak@xxxxxxxxxxx> ]
----------------------------------------------------------------------------
13 files changed, 46 insertions(+), 90 deletions(-)
headers/private/kernel/thread.h | 1 +
src/system/kernel/debug/system_profiler.cpp | 3 +-
src/system/kernel/fs/Vnode.cpp | 3 +-
src/system/kernel/fs/fifo.cpp | 3 +-
src/system/kernel/locks/lock.cpp | 39 +++++++----------------
src/system/kernel/posix/xsi_message_queue.cpp | 12 +++----
src/system/kernel/posix/xsi_semaphore.cpp | 11 +++----
src/system/kernel/sem.cpp | 4 +--
src/system/kernel/signal.cpp | 15 +++------
src/system/kernel/thread.cpp | 13 ++++++++
src/system/kernel/usergroup.cpp | 26 ++-------------
src/system/kernel/vm/VMCache.cpp | 3 +-
src/system/kernel/vm/vm_page.cpp | 3 +-
############################################################################
Commit: d54a9e0a4194f6843406bc67f68045f59f9a190d
Author: Pawel Dziepak <pdziepak@xxxxxxxxxxx>
Date: Wed Oct 30 01:57:45 2013 UTC
kernel: Do not use gSchedulerLock when accesing UID and GID
Reads and writes to uid_t and gid_t are atomic anyway. The only real
problem that may happen here is inconsistent state of triples
effective_{u, g}id, saved_set_{u, g}id, real_{u, g}id, but team locks
protect us against that.
----------------------------------------------------------------------------
diff --git a/src/system/kernel/signal.cpp b/src/system/kernel/signal.cpp
index a28533f..1d61b15 100644
--- a/src/system/kernel/signal.cpp
+++ b/src/system/kernel/signal.cpp
@@ -315,7 +315,6 @@ Signal::SetTo(uint32 number)
fErrorCode = 0;
fSendingProcess = team->id;
fSendingUser = team->effective_uid;
- // assuming scheduler lock is being held
fStatus = 0;
fPollBand = 0;
fAddress = NULL;
@@ -1322,19 +1321,13 @@ has_signals_pending(Thread* thread)
/*! Checks whether the current user has permission to send a signal to the
given
target team.
- The caller must hold the scheduler lock or \a team's lock.
-
\param team The target team.
- \param schedulerLocked \c true, if the caller holds the scheduler lock,
- \c false otherwise.
*/
static bool
-has_permission_to_signal(Team* team, bool schedulerLocked)
+has_permission_to_signal(Signal* signal, Team* team)
{
// get the current user
- uid_t currentUser = schedulerLocked
- ? thread_get_current_thread()->team->effective_uid
- : geteuid();
+ uid_t currentUser = signal->SendingUser();
// root is omnipotent -- in the other cases the current user must match
the
// target team's
@@ -1372,7 +1365,7 @@ send_signal_to_thread_locked(Thread* thread, uint32
signalNumber,
BReference<Signal> signalReference(signal, true);
if ((flags & B_CHECK_PERMISSION) != 0) {
- if (!has_permission_to_signal(thread->team, true))
+ if (!has_permission_to_signal(signal, thread->team))
return EPERM;
}
@@ -1566,7 +1559,7 @@ send_signal_to_team_locked(Team* team, uint32
signalNumber, Signal* signal,
BReference<Signal> signalReference(signal, true);
if ((flags & B_CHECK_PERMISSION) != 0) {
- if (!has_permission_to_signal(team, true))
+ if (!has_permission_to_signal(signal, team))
return EPERM;
}
diff --git a/src/system/kernel/usergroup.cpp b/src/system/kernel/usergroup.cpp
index 5a54ea0..9be52f5 100644
--- a/src/system/kernel/usergroup.cpp
+++ b/src/system/kernel/usergroup.cpp
@@ -54,7 +54,6 @@ common_setregid(gid_t rgid, gid_t egid, bool
setAllIfPrivileged, bool kernel)
// setgid() semantics: If privileged set both, real,
effective and
// saved set-gid, otherwise set the effective gid.
if (privileged) {
- InterruptsSpinLocker
schedulerLocker(gSchedulerLock);
team->saved_set_gid = rgid;
team->real_gid = rgid;
team->effective_gid = rgid;
@@ -91,7 +90,6 @@ common_setregid(gid_t rgid, gid_t egid, bool
setAllIfPrivileged, bool kernel)
}
// Getting here means all checks were successful -- set the gids.
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->real_gid = rgid;
team->effective_gid = egid;
team->saved_set_gid = ssgid;
@@ -119,7 +117,6 @@ common_setreuid(uid_t ruid, uid_t euid, bool
setAllIfPrivileged, bool kernel)
// setuid() semantics: If privileged set both, real,
effective and
// saved set-uid, otherwise set the effective uid.
if (privileged) {
- InterruptsSpinLocker
schedulerLocker(gSchedulerLock);
team->saved_set_uid = ruid;
team->real_uid = ruid;
team->effective_uid = ruid;
@@ -156,7 +153,6 @@ common_setreuid(uid_t ruid, uid_t euid, bool
setAllIfPrivileged, bool kernel)
}
// Getting here means all checks were successful -- set the uids.
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->real_uid = ruid;
team->effective_uid = euid;
team->saved_set_uid = ssuid;
@@ -253,8 +249,6 @@ common_setgroups(int groupCount, const gid_t* groupList,
bool kernel)
void
inherit_parent_user_and_group(Team* team, Team* parent)
{
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
-
team->saved_set_uid = parent->saved_set_uid;
team->real_uid = parent->real_uid;
team->effective_uid = parent->effective_uid;
@@ -262,8 +256,6 @@ inherit_parent_user_and_group(Team* team, Team* parent)
team->real_gid = parent->real_gid;
team->effective_gid = parent->effective_gid;
- schedulerLocker.Unlock();
-
malloc_referenced_acquire(parent->supplementary_groups);
team->supplementary_groups = parent->supplementary_groups;
team->supplementary_group_count = parent->supplementary_group_count;
@@ -279,7 +271,6 @@ update_set_id_user_and_group(Team* team, const char* file)
return status;
TeamLocker teamLocker(team);
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
if ((st.st_mode & S_ISUID) != 0) {
team->saved_set_uid = st.st_uid;
@@ -300,8 +291,6 @@ _kern_getgid(bool effective)
{
Team* team = thread_get_current_thread()->team;
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
-
return effective ? team->effective_gid : team->real_gid;
}
@@ -311,8 +300,6 @@ _kern_getuid(bool effective)
{
Team* team = thread_get_current_thread()->team;
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
-
return effective ? team->effective_uid : team->real_uid;
}
@@ -353,8 +340,6 @@ _user_getgid(bool effective)
{
Team* team = thread_get_current_thread()->team;
- TeamLocker teamLocker(team);
-
return effective ? team->effective_gid : team->real_gid;
}
@@ -364,8 +349,6 @@ _user_getuid(bool effective)
{
Team* team = thread_get_current_thread()->team;
- TeamLocker teamLocker(team);
-
return effective ? team->effective_uid : team->real_uid;
}
@@ -395,12 +378,9 @@ ssize_t
_user_setgroups(int groupCount, const gid_t* groupList)
{
// check privilege
- {
- Team* team = thread_get_current_thread()->team;
- TeamLocker teamLocker(team);
- if (!is_privileged(team))
- return EPERM;
- }
+ Team* team = thread_get_current_thread()->team;
+ if (!is_privileged(team))
+ return EPERM;
return common_setgroups(groupCount, groupList, false);
}
############################################################################
Commit: d70728f54da5b22311b6f271a60fe2067ae4a47b
Author: Pawel Dziepak <pdziepak@xxxxxxxxxxx>
Date: Wed Oct 30 02:26:13 2013 UTC
kernel/lock: Do not use *_locked() functions when not needed
----------------------------------------------------------------------------
diff --git a/src/system/kernel/locks/lock.cpp b/src/system/kernel/locks/lock.cpp
index 3b1d069..7f4094f 100644
--- a/src/system/kernel/locks/lock.cpp
+++ b/src/system/kernel/locks/lock.cpp
@@ -168,9 +168,7 @@ rw_lock_wait(rw_lock* lock, bool writer,
InterruptsSpinLocker& locker)
thread_prepare_to_block(waiter.thread, 0, THREAD_BLOCK_TYPE_RW_LOCK,
lock);
locker.Unlock();
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- status_t result = thread_block_locked(thread_get_current_thread());
- schedulerLocker.Unlock();
+ status_t result = thread_block();
locker.Lock();
return result;
@@ -386,10 +384,7 @@ _rw_lock_read_lock_with_timeout(rw_lock* lock, uint32
timeoutFlags,
thread_prepare_to_block(waiter.thread, 0, THREAD_BLOCK_TYPE_RW_LOCK,
lock);
locker.Unlock();
- InterruptsSpinLocker schedulerLock(gSchedulerLock);
- status_t error = thread_block_with_timeout_locked(timeoutFlags,
timeout);
- schedulerLock.Unlock();
-
+ status_t error = thread_block_with_timeout(timeoutFlags, timeout);
if (error == B_OK || waiter.thread == NULL) {
// We were unblocked successfully -- potentially our unblocker
overtook
// us after we already failed. In either case, we've got the
lock, now.
@@ -752,10 +747,7 @@ _mutex_lock(mutex* lock, void* _locker)
thread_prepare_to_block(waiter.thread, 0, THREAD_BLOCK_TYPE_MUTEX,
lock);
locker->Unlock();
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- status_t error = thread_block_locked(waiter.thread);
- schedulerLocker.Unlock();
-
+ status_t error = thread_block();
#if KDEBUG
if (error == B_OK)
atomic_set(&lock->holder, waiter.thread->id);
@@ -875,16 +867,15 @@ _mutex_lock_with_timeout(mutex* lock, uint32
timeoutFlags, bigtime_t timeout)
thread_prepare_to_block(waiter.thread, 0, THREAD_BLOCK_TYPE_MUTEX,
lock);
locker.Unlock();
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- status_t error = thread_block_with_timeout_locked(timeoutFlags,
timeout);
- schedulerLocker.Unlock();
+ status_t error = thread_block_with_timeout(timeoutFlags, timeout);
- locker.Lock();
if (error == B_OK) {
#if KDEBUG
lock->holder = waiter.thread->id;
#endif
} else {
+ locker.Lock();
+
// If the timeout occurred, we must remove our waiter structure
from
// the queue.
mutex_waiter* previousWaiter = NULL;
############################################################################
Commit: c8dd9f7780c426e592a3ccb231e6bfab51f15eb9
Author: Pawel Dziepak <pdziepak@xxxxxxxxxxx>
Date: Wed Oct 30 02:58:36 2013 UTC
kernel: Add thread_unblock() and use it where possible
----------------------------------------------------------------------------
diff --git a/headers/private/kernel/thread.h b/headers/private/kernel/thread.h
index 469e339..ecb6245 100644
--- a/headers/private/kernel/thread.h
+++ b/headers/private/kernel/thread.h
@@ -132,6 +132,7 @@ status_t thread_block();
status_t thread_block_with_timeout(uint32 timeoutFlags, bigtime_t timeout);
status_t thread_block_with_timeout_locked(uint32 timeoutFlags,
bigtime_t timeout);
+void thread_unblock(Thread* thread, status_t status);
// used in syscalls.c
status_t _user_set_thread_priority(thread_id thread, int32 newPriority);
diff --git a/src/system/kernel/debug/system_profiler.cpp
b/src/system/kernel/debug/system_profiler.cpp
index c2d7549..d0d3e22 100644
--- a/src/system/kernel/debug/system_profiler.cpp
+++ b/src/system/kernel/debug/system_profiler.cpp
@@ -298,8 +298,7 @@ SystemProfiler::~SystemProfiler()
// inactive.
InterruptsSpinLocker locker(fLock);
if (fWaitingProfilerThread != NULL) {
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(fWaitingProfilerThread, B_OK);
+ thread_unblock(fWaitingProfilerThread, B_OK);
fWaitingProfilerThread = NULL;
}
fProfilingActive = false;
diff --git a/src/system/kernel/fs/Vnode.cpp b/src/system/kernel/fs/Vnode.cpp
index 890e29b..cc71aae 100644
--- a/src/system/kernel/fs/Vnode.cpp
+++ b/src/system/kernel/fs/Vnode.cpp
@@ -88,6 +88,5 @@ vnode::_WakeUpLocker()
atomic_and(&fFlags, ~kFlagsWaitingLocker);
// and wake it up
- InterruptsSpinLocker threadLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_OK);
+ thread_unblock(waiter->thread, B_OK);
}
diff --git a/src/system/kernel/fs/fifo.cpp b/src/system/kernel/fs/fifo.cpp
index 9c771ff..fe2ffb6 100644
--- a/src/system/kernel/fs/fifo.cpp
+++ b/src/system/kernel/fs/fifo.cpp
@@ -100,8 +100,7 @@ public:
TRACE("ReadRequest %p::Notify(), fNotified %d\n", this,
fNotified);
if (!fNotified) {
- SpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(fThread, status);
+ thread_unblock(fThread, status);
fNotified = true;
}
}
diff --git a/src/system/kernel/locks/lock.cpp b/src/system/kernel/locks/lock.cpp
index 7f4094f..6b3ac5f 100644
--- a/src/system/kernel/locks/lock.cpp
+++ b/src/system/kernel/locks/lock.cpp
@@ -197,9 +197,7 @@ rw_lock_unblock(rw_lock* lock)
lock->holder = waiter->thread->id;
// unblock thread
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_OK);
- schedulerLocker.Unlock();
+ thread_unblock(waiter->thread, B_OK);
waiter->thread = NULL;
return RW_LOCK_WRITER_COUNT_BASE;
@@ -216,9 +214,7 @@ rw_lock_unblock(rw_lock* lock)
readerCount++;
// unblock thread
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_OK);
- schedulerLocker.Unlock();
+ thread_unblock(waiter->thread, B_OK);
waiter->thread = NULL;
} while ((waiter = lock->waiters) != NULL && !waiter->writer);
@@ -293,8 +289,7 @@ rw_lock_destroy(rw_lock* lock)
lock->waiters = waiter->next;
// unblock thread
- InterruptsSpinLocker _(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_ERROR);
+ thread_unblock(waiter->thread, B_ERROR);
}
lock->name = NULL;
@@ -637,8 +632,7 @@ mutex_destroy(mutex* lock)
lock->waiters = waiter->next;
// unblock thread
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_ERROR);
+ thread_unblock(waiter->thread, B_ERROR);
}
lock->name = NULL;
@@ -783,9 +777,7 @@ _mutex_unlock(mutex* lock)
lock->waiters->last = waiter->last;
// unblock thread
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_OK);
- schedulerLocker.Unlock();
+ thread_unblock(waiter->thread, B_OK);
#if KDEBUG
// Already set the holder to the unblocked thread. Besides that
this
diff --git a/src/system/kernel/posix/xsi_message_queue.cpp
b/src/system/kernel/posix/xsi_message_queue.cpp
index 278c241..f0141b1 100644
--- a/src/system/kernel/posix/xsi_message_queue.cpp
+++ b/src/system/kernel/posix/xsi_message_queue.cpp
@@ -246,8 +246,6 @@ public:
void WakeUpThread(bool waitForMessage)
{
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
-
if (waitForMessage) {
// Wake up all waiting thread for a message
// TODO: this can cause starvation for any
@@ -255,14 +253,14 @@ public:
while (queued_thread *entry =
fWaitingToReceive.RemoveHead()) {
entry->queued = false;
fThreadsWaitingToReceive--;
- thread_unblock_locked(entry->thread, 0);
+ thread_unblock(entry->thread, 0);
}
} else {
// Wake up only one thread waiting to send
if (queued_thread *entry = fWaitingToSend.RemoveHead())
{
entry->queued = false;
fThreadsWaitingToSend--;
- thread_unblock_locked(entry->thread, 0);
+ thread_unblock(entry->thread, 0);
}
}
}
@@ -400,15 +398,13 @@ XsiMessageQueue::~XsiMessageQueue()
// Wake up any threads still waiting
if (fThreadsWaitingToSend || fThreadsWaitingToReceive) {
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
-
while (queued_thread *entry = fWaitingToReceive.RemoveHead()) {
entry->queued = false;
- thread_unblock_locked(entry->thread, EIDRM);
+ thread_unblock(entry->thread, EIDRM);
}
while (queued_thread *entry = fWaitingToSend.RemoveHead()) {
entry->queued = false;
- thread_unblock_locked(entry->thread, EIDRM);
+ thread_unblock(entry->thread, EIDRM);
}
}
diff --git a/src/system/kernel/posix/xsi_semaphore.cpp
b/src/system/kernel/posix/xsi_semaphore.cpp
index 212ee1b..4c76c79 100644
--- a/src/system/kernel/posix/xsi_semaphore.cpp
+++ b/src/system/kernel/posix/xsi_semaphore.cpp
@@ -101,15 +101,13 @@ public:
{
// For some reason the semaphore is getting destroyed.
// Wake up any remaing awaiting threads
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
-
while (queued_thread *entry =
fWaitingToIncreaseQueue.RemoveHead()) {
entry->queued = false;
- thread_unblock_locked(entry->thread, EIDRM);
+ thread_unblock(entry->thread, EIDRM);
}
while (queued_thread *entry =
fWaitingToBeZeroQueue.RemoveHead()) {
entry->queued = false;
- thread_unblock_locked(entry->thread, EIDRM);
+ thread_unblock(entry->thread, EIDRM);
}
// No need to remove any sem_undo request still
// hanging. When the process exit and doesn't found
@@ -218,20 +216,19 @@ public:
void WakeUpThread(bool waitingForZero)
{
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
if (waitingForZero) {
// Wake up all threads waiting on zero
while (queued_thread *entry =
fWaitingToBeZeroQueue.RemoveHead()) {
entry->queued = false;
fThreadsWaitingToBeZero--;
- thread_unblock_locked(entry->thread, 0);
+ thread_unblock(entry->thread, 0);
}
} else {
// Wake up all threads even though they might go back
to sleep
while (queued_thread *entry =
fWaitingToIncreaseQueue.RemoveHead()) {
entry->queued = false;
fThreadsWaitingToIncrease--;
- thread_unblock_locked(entry->thread, 0);
+ thread_unblock(entry->thread, 0);
}
}
}
diff --git a/src/system/kernel/sem.cpp b/src/system/kernel/sem.cpp
index 0b761c2..7f01069 100644
--- a/src/system/kernel/sem.cpp
+++ b/src/system/kernel/sem.cpp
@@ -330,12 +330,10 @@ uninit_sem_locked(struct sem_entry& sem, char** _name)
sem.u.used.select_infos = NULL;
// free any threads waiting for this semaphore
- SpinLocker schedulerLocker(gSchedulerLock);
while (queued_thread* entry = sem.queue.RemoveHead()) {
entry->queued = false;
- thread_unblock_locked(entry->thread, B_BAD_SEM_ID);
+ thread_unblock(entry->thread, B_BAD_SEM_ID);
}
- schedulerLocker.Unlock();
int32 id = sem.id;
sem.id = -1;
diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp
index 210492d..20bf8f7 100644
--- a/src/system/kernel/thread.cpp
+++ b/src/system/kernel/thread.cpp
@@ -2889,6 +2889,19 @@ thread_block_with_timeout_locked(uint32 timeoutFlags,
bigtime_t timeout)
}
+/*! Unblocks a thread.
+
+ Acquires the scheduler lock and calls thread_unblock_locked().
+ See there for more information.
+*/
+void
+thread_unblock(Thread* thread, status_t status)
+{
+ InterruptsSpinLocker _(gSchedulerLock);
+ thread_unblock_locked(thread, status);
+}
+
+
/*! Unblocks a userland-blocked thread.
The caller must not hold any locks.
*/
diff --git a/src/system/kernel/vm/VMCache.cpp b/src/system/kernel/vm/VMCache.cpp
index d9efe30..a1d1667 100644
--- a/src/system/kernel/vm/VMCache.cpp
+++ b/src/system/kernel/vm/VMCache.cpp
@@ -1383,8 +1383,7 @@ VMCache::_NotifyPageEvents(vm_page* page, uint32 events)
if (waiter->page == page && (waiter->events & events) != 0) {
// remove from list and unblock
*it = waiter->next;
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_OK);
+ thread_unblock(waiter->thread, B_OK);
} else
it = &waiter->next;
}
diff --git a/src/system/kernel/vm/vm_page.cpp b/src/system/kernel/vm/vm_page.cpp
index 35d1a8b..dd9b9b3 100644
--- a/src/system/kernel/vm/vm_page.cpp
+++ b/src/system/kernel/vm/vm_page.cpp
@@ -1447,8 +1447,7 @@ wake_up_page_reservation_waiters()
sPageReservationWaiters.Remove(waiter);
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
- thread_unblock_locked(waiter->thread, B_OK);
+ thread_unblock(waiter->thread, B_OK);
}
}
Other related posts:
- » [haiku-commits] BRANCH pdziepak-github.scheduler [c8dd9f7] in src/system/kernel: . locks posix vm fs - pdziepak-github . scheduler
