From 2f6b4f3e1a392bc9dd18659bbd57d61a2eb84fd0 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Fri, 28 Sep 2007 21:37:31 +0000 Subject: [PATCH] * When canonical input processing is enabled reads should only return anything, if a line is available. Fixes bug #1483. * Replaced the request owner waiting semaphore by a condition variable and removed the now no longer needed SemaphorePool. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22357 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/add-ons/kernel/drivers/tty/Jamfile | 3 +- .../kernel/drivers/tty/SemaphorePool.cpp | 100 ------------------ .../kernel/drivers/tty/SemaphorePool.h | 67 ------------ src/add-ons/kernel/drivers/tty/driver.cpp | 17 --- .../kernel/drivers/tty/line_buffer.cpp | 18 ++++ src/add-ons/kernel/drivers/tty/line_buffer.h | 1 + src/add-ons/kernel/drivers/tty/tty.cpp | 97 +++++++++-------- src/add-ons/kernel/drivers/tty/tty_private.h | 31 +++--- 8 files changed, 88 insertions(+), 246 deletions(-) delete mode 100644 src/add-ons/kernel/drivers/tty/SemaphorePool.cpp delete mode 100644 src/add-ons/kernel/drivers/tty/SemaphorePool.h diff --git a/src/add-ons/kernel/drivers/tty/Jamfile b/src/add-ons/kernel/drivers/tty/Jamfile index 9ab336230d..1d025216e8 100644 --- a/src/add-ons/kernel/drivers/tty/Jamfile +++ b/src/add-ons/kernel/drivers/tty/Jamfile @@ -8,6 +8,5 @@ KernelAddon tty : line_buffer.cpp master.cpp slave.cpp - SemaphorePool.cpp - ; +; diff --git a/src/add-ons/kernel/drivers/tty/SemaphorePool.cpp b/src/add-ons/kernel/drivers/tty/SemaphorePool.cpp deleted file mode 100644 index 947ced244c..0000000000 --- a/src/add-ons/kernel/drivers/tty/SemaphorePool.cpp +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2005-2007, Ingo Weinhold, bonefish@users.sf.net. - * Distributed under the terms of the MIT License. - */ - -#include - -#include - -#include "SemaphorePool.h" - - -status_t -Semaphore::ZeroCount() -{ - int32 count; - status_t error = get_sem_count(fSemaphore, &count); - if (error != B_OK) - return error; - - if (count > 0) - error = acquire_sem_etc(fSemaphore, count, B_RELATIVE_TIMEOUT, 0); - else if (count < 0) - error = release_sem_etc(fSemaphore, -count, 0); - - return error; -} - - -// #pragma mark - - -SemaphorePool::SemaphorePool(int32 maxCached) - : fSemaphores(), - fCount(0), - fMaxCount(maxCached) -{ - if (fMaxCount < 0) - fMaxCount = 8; -} - - -SemaphorePool::~SemaphorePool() -{ - while (Semaphore *sem = fSemaphores.Head()) { - fSemaphores.Remove(sem); - delete sem; - } - - mutex_destroy(&fLock); -} - - -status_t -SemaphorePool::Init() -{ - return mutex_init(&fLock, "tty semaphore pool"); -} - - -status_t -SemaphorePool::Get(Semaphore *&semaphore) -{ - MutexLocker _(fLock); - - // use a cached one, if available - if (fCount > 0) { - fCount--; - semaphore = fSemaphores.Head(); - fSemaphores.Remove(semaphore); - return B_OK; - } - - // create a new one - Semaphore *sem = new(nothrow) Semaphore(0, "pool semaphore"); - if (!sem) - return B_NO_MEMORY; - - semaphore = sem; - return B_OK; -} - - -void -SemaphorePool::Put(Semaphore *semaphore) -{ - if (!semaphore) - return; - - MutexLocker _(fLock); - - if (fCount >= fMaxCount - || semaphore->InitCheck() != B_OK - || semaphore->ZeroCount() != B_OK) { - delete semaphore; - return; - } - - fSemaphores.Add(semaphore); - fCount++; -} diff --git a/src/add-ons/kernel/drivers/tty/SemaphorePool.h b/src/add-ons/kernel/drivers/tty/SemaphorePool.h deleted file mode 100644 index 74768d10aa..0000000000 --- a/src/add-ons/kernel/drivers/tty/SemaphorePool.h +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright 2005, Ingo Weinhold, bonefish@users.sf.net. - * Distributed under the terms of the MIT License. - */ -#ifndef TTY_SEMAPHORE_POOL_H -#define TTY_SEMAPHORE_POOL_H - -#include -#include - -class Semaphore : public DoublyLinkedListLinkImpl { -public: - Semaphore(sem_id sem) - : fSemaphore(sem) - { - } - - Semaphore(int32 count, const char *name) - { - fSemaphore = create_sem(count, name); - } - - ~Semaphore() - { - if (fSemaphore >= 0) - delete_sem(fSemaphore); - } - - status_t InitCheck() const - { - return (fSemaphore >= 0 ? B_OK : fSemaphore); - } - - sem_id ID() const - { - return fSemaphore; - } - - status_t ZeroCount(); - -private: - sem_id fSemaphore; - -public: - Semaphore *fNext; -}; - -class SemaphorePool { -public: - SemaphorePool(int32 maxCached); - ~SemaphorePool(); - - status_t Init(); - - status_t Get(Semaphore *&semaphore); - void Put(Semaphore *semaphore); - -private: - typedef DoublyLinkedList SemaphoreList; - - struct mutex fLock; - SemaphoreList fSemaphores; - int32 fCount; - int32 fMaxCount; -}; - -#endif // TTY_SEMAPHORE_POOL_H diff --git a/src/add-ons/kernel/drivers/tty/driver.cpp b/src/add-ons/kernel/drivers/tty/driver.cpp index dd7b76f58c..097e81dec3 100644 --- a/src/add-ons/kernel/drivers/tty/driver.cpp +++ b/src/add-ons/kernel/drivers/tty/driver.cpp @@ -11,7 +11,6 @@ #include -#include "SemaphorePool.h" #include "tty_private.h" @@ -34,8 +33,6 @@ static char *sDeviceNames[kNumTTYs * 2 + 1]; struct mutex gGlobalTTYLock; struct mutex gTTYCookieLock; struct recursive_lock gTTYRequestLock; -static char sSemaphorePoolBuffer[sizeof(SemaphorePool)]; -SemaphorePool *gSemaphorePool = 0; status_t @@ -73,15 +70,6 @@ init_driver(void) return error; } - // create the semaphore pool - gSemaphorePool - = new(sSemaphorePoolBuffer) SemaphorePool(kMaxCachedSemaphores); - error = gSemaphorePool->Init(); - if (error != B_OK) { - uninit_driver(); - return error; - } - // create driver name array and initialize basic TTY structures char letter = 'p'; @@ -124,11 +112,6 @@ uninit_driver(void) for (int32 i = 0; i < (int32)kNumTTYs * 2; i++) free(sDeviceNames[i]); - if (gSemaphorePool) { - gSemaphorePool->~SemaphorePool(); - gSemaphorePool = NULL; - } - recursive_lock_destroy(&gTTYRequestLock); mutex_destroy(&gTTYCookieLock); mutex_destroy(&gGlobalTTYLock); diff --git a/src/add-ons/kernel/drivers/tty/line_buffer.cpp b/src/add-ons/kernel/drivers/tty/line_buffer.cpp index fe3d683987..dcd035b523 100644 --- a/src/add-ons/kernel/drivers/tty/line_buffer.cpp +++ b/src/add-ons/kernel/drivers/tty/line_buffer.cpp @@ -49,6 +49,24 @@ line_buffer_readable(struct line_buffer &buffer) } +int32 +line_buffer_readable_line(struct line_buffer &buffer, char eol, char eof) +{ + size_t size = buffer.in; + if (size == 0) + return 0; + + // find EOL or EOF char + for (size_t i = 0; i < size; i++) { + char c = buffer.buffer[(buffer.first + i) % buffer.size]; + if (c == eol || c == '\n' || c == '\r' || c == eof) + return i + 1; + } + + return 0; +} + + int32 line_buffer_writable(struct line_buffer &buffer) { diff --git a/src/add-ons/kernel/drivers/tty/line_buffer.h b/src/add-ons/kernel/drivers/tty/line_buffer.h index 0d5ac31177..018824bbf7 100644 --- a/src/add-ons/kernel/drivers/tty/line_buffer.h +++ b/src/add-ons/kernel/drivers/tty/line_buffer.h @@ -20,6 +20,7 @@ status_t init_line_buffer(struct line_buffer &buffer, size_t size); status_t uninit_line_buffer(struct line_buffer &buffer); status_t clear_line_buffer(struct line_buffer &buffer); int32 line_buffer_readable(struct line_buffer &buffer); +int32 line_buffer_readable_line(struct line_buffer &buffer, char eol, char eof); int32 line_buffer_writable(struct line_buffer &buffer); ssize_t line_buffer_user_read(struct line_buffer &buffer, char *data, size_t length, char eof = 0, bool* hitEOF = NULL); diff --git a/src/add-ons/kernel/drivers/tty/tty.cpp b/src/add-ons/kernel/drivers/tty/tty.cpp index a6556b80a7..64a2494774 100644 --- a/src/add-ons/kernel/drivers/tty/tty.cpp +++ b/src/add-ons/kernel/drivers/tty/tty.cpp @@ -19,7 +19,6 @@ #include -#include "SemaphorePool.h" #include "tty_private.h" @@ -134,7 +133,8 @@ class ReaderLocker : public AbstractLocker { status_t AcquireReader(bool dontBlock); private: - status_t _CheckBackgroundRead() const; + size_t _CheckAvailableBytes() const; + status_t _CheckBackgroundRead() const; struct tty *fTTY; RequestOwner fRequestOwner; @@ -295,7 +295,7 @@ RequestQueue::NotifyError(tty_cookie *cookie, status_t error) RequestOwner::RequestOwner() : - fSemaphore(NULL), + fConditionVariable(NULL), fCookie(NULL), fError(B_OK), fBytesNeeded(1) @@ -364,20 +364,11 @@ RequestOwner::SetBytesNeeded(size_t bytesNeeded) * The request lock MUST NOT be held! */ status_t -RequestOwner::Wait(bool interruptable, Semaphore *sem) +RequestOwner::Wait(bool interruptable) { - TRACE(("%p->RequestOwner::Wait(%d, %p)\n", this, interruptable, sem)); + TRACE(("%p->RequestOwner::Wait(%d)\n", this, interruptable)); - // get a semaphore (if not supplied or invalid) status_t error = B_OK; - bool ownsSem = false; - if (!sem || sem->InitCheck() != B_OK) { - error = gSemaphorePool->Get(sem); - if (error != B_OK) - return error; - - ownsSem = true; - } RecursiveLocker locker(gTTYRequestLock); @@ -386,35 +377,35 @@ RequestOwner::Wait(bool interruptable, Semaphore *sem) && (!fRequests[0].WasNotified() || !fRequests[1].WasNotified())) { // not yet done - // set the semaphore - fSemaphore = sem; + // publish the condition variable + ConditionVariable<> conditionVariable; + conditionVariable.Publish(this, "tty request"); + fConditionVariable = &conditionVariable; + + // add an entry to wait on + ConditionVariableEntry<> entry; + entry.Add(this); locker.Unlock(); // wait - TRACE(("%p->RequestOwner::Wait(): acquiring semaphore...\n", this)); + TRACE(("%p->RequestOwner::Wait(): waiting for condition...\n", this)); - error = acquire_sem_etc(sem->ID(), 1, - (interruptable ? B_CAN_INTERRUPT : 0), 0); + error = entry.Wait(interruptable ? B_CAN_INTERRUPT : 0); - TRACE(("%p->RequestOwner::Wait(): semaphore acquired: %lx\n", this, + TRACE(("%p->RequestOwner::Wait(): condition occurred: %lx\n", this, error)); - // remove the semaphore - locker.SetTo(gTTYRequestLock, false); - fSemaphore = NULL; + // remove the condition variable + locker.Lock(); + fConditionVariable = NULL; + conditionVariable.Unpublish(); } // get the result if (error == B_OK) error = fError; - locker.Unlock(); - - // return the semaphore to the pool - if (ownsSem) - gSemaphorePool->Put(sem); - return error; } @@ -439,18 +430,18 @@ RequestOwner::Notify(Request *request) TRACE(("%p->RequestOwner::Notify(%p)\n", this, request)); if (fError == B_OK && !request->WasNotified()) { - bool releaseSem = false; + bool notify = false; if (&fRequests[0] == request) { - releaseSem = fRequests[1].WasNotified(); + notify = fRequests[1].WasNotified(); } else if (&fRequests[1] == request) { - releaseSem = fRequests[0].WasNotified(); + notify = fRequests[0].WasNotified(); } else { // spurious call } - if (releaseSem && fSemaphore) - release_sem(fSemaphore->ID()); + if (notify && fConditionVariable) + fConditionVariable->NotifyOne(); } } @@ -464,8 +455,8 @@ RequestOwner::NotifyError(Request *request, status_t error) fError = error; if (!fRequests[0].WasNotified() || !fRequests[1].WasNotified()) { - if (fSemaphore) - release_sem(fSemaphore->ID()); + if (fConditionVariable) + fConditionVariable->NotifyOne(); } } } @@ -667,7 +658,7 @@ ReaderLocker::AcquireReader(bool dontBlock) // check, if we're first in queue, and if there is something to read if (fRequestOwner.IsFirstInQueues()) { - fBytes = line_buffer_readable(fTTY->input_buffer); + fBytes = _CheckAvailableBytes(); if (fBytes > 0) return B_OK; } @@ -686,12 +677,27 @@ ReaderLocker::AcquireReader(bool dontBlock) status = _CheckBackgroundRead(); if (status == B_OK) - fBytes = line_buffer_readable(fTTY->input_buffer); + fBytes = _CheckAvailableBytes(); return status; } +size_t +ReaderLocker::_CheckAvailableBytes() const +{ + // Reading from the slave with canonical input processing enabled means + // that we read at max until hitting a line end or EOF. + if (!fTTY->is_master && (fTTY->settings->termios.c_lflag & ICANON) != 0) { + return line_buffer_readable_line(fTTY->input_buffer, + fTTY->settings->termios.c_cc[VEOL], + fTTY->settings->termios.c_cc[VEOF]); + } + + return line_buffer_readable(fTTY->input_buffer); +} + + status_t ReaderLocker::_CheckBackgroundRead() const { @@ -1028,11 +1034,8 @@ tty_close_cookie(struct tty_cookie *cookie) ttyLocker.Unlock(); - // wait for our turn (we reuse the blocking semaphore, so Wait() - // can't fail due to not being able to get a semaphore) - Semaphore sem(cookie->blocking_semaphore); - cookie->blocking_semaphore = -1; - requestOwner.Wait(false, &sem); + // wait for our turn + requestOwner.Wait(false); // re-lock ttyLocker.SetTo(cookie->tty->lock, false); @@ -1402,11 +1405,17 @@ tty_input_read(tty_cookie *cookie, void *buffer, size_t *_length) return status; } + size_t toRead = locker.AvailableBytes(); + if (toRead == 0) + continue; + if (toRead > length) + toRead = length; + bool _hitEOF = false; bool* hitEOF = (tty->pending_eof > 0 ? &_hitEOF : NULL); bytesRead = line_buffer_user_read(tty->input_buffer, (char *)buffer, - length, tty->settings->termios.c_cc[VEOF], hitEOF); + toRead, tty->settings->termios.c_cc[VEOF], hitEOF); if (bytesRead < B_OK) { *_length = 0; return bytesRead; diff --git a/src/add-ons/kernel/drivers/tty/tty_private.h b/src/add-ons/kernel/drivers/tty/tty_private.h index 694417a0ee..ac88ae13aa 100644 --- a/src/add-ons/kernel/drivers/tty/tty_private.h +++ b/src/add-ons/kernel/drivers/tty/tty_private.h @@ -5,15 +5,16 @@ #ifndef TTY_PRIVATE_H #define TTY_PRIVATE_H - -#include -#include -#include -#include -#include - #include +#include +#include + +#include +#include +#include +#include + #include "line_buffer.h" @@ -29,7 +30,6 @@ typedef status_t (*tty_service_func)(struct tty *tty, uint32 op); class RequestOwner; class Semaphore; -class SemaphorePool; struct tty; struct tty_cookie; @@ -87,7 +87,7 @@ class RequestOwner { void SetBytesNeeded(size_t bytesNeeded); size_t BytesNeeded() const { return fBytesNeeded; } - status_t Wait(bool interruptable, Semaphore *sem = NULL); + status_t Wait(bool interruptable); bool IsFirstInQueues(); @@ -97,12 +97,12 @@ class RequestOwner { status_t Error() const { return fError; } private: - Semaphore *fSemaphore; - tty_cookie *fCookie; - status_t fError; - RequestQueue *fRequestQueues[2]; - Request fRequests[2]; - size_t fBytesNeeded; + ConditionVariable<>* fConditionVariable; + tty_cookie* fCookie; + status_t fError; + RequestQueue* fRequestQueues[2]; + Request fRequests[2]; + size_t fBytesNeeded; }; @@ -151,7 +151,6 @@ extern device_hooks gSlaveTTYHooks; extern struct mutex gGlobalTTYLock; extern struct mutex gTTYCookieLock; extern struct recursive_lock gTTYRequestLock; -extern SemaphorePool *gSemaphorePool; // functions available for master/slave TTYs