From 4f0a4e201a03ea9ecfed2c7c074e596b44ec5707 Mon Sep 17 00:00:00 2001 From: Terraneo Federico <fede.tft@miosix.org> Date: Mon, 1 May 2023 00:44:22 +0200 Subject: [PATCH] Refactored Queue, removed waitUntilNotFull() and waitUntilNotEmpty() --- miosix/_tools/testsuite/testsuite.cpp | 35 ++--- miosix/arch/common/drivers/serial_lpc2000.cpp | 21 ++- miosix/arch/common/drivers/serial_lpc2000.h | 3 +- miosix/kernel/queue.h | 131 ++++++++++-------- 4 files changed, 101 insertions(+), 89 deletions(-) diff --git a/miosix/_tools/testsuite/testsuite.cpp b/miosix/_tools/testsuite/testsuite.cpp index 3122583b..f5f21ab1 100644 --- a/miosix/_tools/testsuite/testsuite.cpp +++ b/miosix/_tools/testsuite/testsuite.cpp @@ -1688,9 +1688,9 @@ Queue::put() Queue::get() Queue::reset() Queue::IRQput() -Queue::waitUntilNotFull() +Queue::IRQputBlocking() Queue::IRQget() -Queue::waitUntilNotEmpty() +Queue::IRQgetBlocking() FIXME: The overloaded versions of IRQput and IRQget are not tested */ @@ -1772,52 +1772,41 @@ static void test_8() read++; } } - //Test waitUntilNotFull and IRQput + //Test IRQputBlocking t8_q1.reset(); t8_q2.reset(); write='A'; read='A'; - disableInterrupts();// - for(j=0;j<8;j++) { - if(t8_q1.isFull()) + FastInterruptDisableLock dLock; + for(j=0;j<8;j++) { - enableInterrupts();// - t8_q1.waitUntilNotFull(); - disableInterrupts();// + t8_q1.IRQputBlocking(write,dLock); + write++;//Advance to next char, to check order } - if(t8_q1.isFull()) fail("waitUntilNotFull()"); - if(t8_q1.IRQput(write)==false) fail("IRQput (1)"); - write++;//Advance to next char, to check order } - enableInterrupts();// for(j=0;j<8;j++) { char c; t8_q2.get(c); - if(c!=read) fail("IRQput (2)"); + if(c!=read) fail("IRQputBlocking"); read++; } - //Test waitUntilNotEmpty and IRQget + //Test IRQgetBlocking t8_q1.reset(); t8_q2.reset(); write='A'; read='A'; for(j=0;j<8;j++) { - disableInterrupts();// + FastInterruptDisableLock dLock; t8_q1.IRQput(write); write++; if(t8_q2.isEmpty()==false) fail("Unexpected"); - enableInterrupts();// - t8_q2.waitUntilNotEmpty(); - disableInterrupts();// - if(t8_q2.isEmpty()==true) fail("waitUntilNotEmpty()"); char c='\0'; - if(t8_q2.IRQget(c)==false) fail("IRQget (1)"); - if(c!=read) fail("IRQget (2)"); + t8_q2.IRQgetBlocking(c,dLock); + if(c!=read) fail("IRQgetBlocking"); read++; - enableInterrupts();// } p->terminate(); t8_q1.put('0'); diff --git a/miosix/arch/common/drivers/serial_lpc2000.cpp b/miosix/arch/common/drivers/serial_lpc2000.cpp index cb0f3732..a7f9dd8f 100644 --- a/miosix/arch/common/drivers/serial_lpc2000.cpp +++ b/miosix/arch/common/drivers/serial_lpc2000.cpp @@ -87,7 +87,8 @@ void __attribute__ ((interrupt("IRQ"),naked)) usart1irq() // 20ms of full data rate. In the 8N1 format one char is made of 10 bits. // So (baudrate/10)*0.02=baudrate/500 LPC2000Serial::LPC2000Serial(int id, int baudrate) : Device(Device::TTY), - rxQueue(hwRxQueueLen+baudrate/500), rxWaiting(0), idle(true) + txQueue(swTxQueue), rxQueue(hwRxQueueLen+baudrate/500), + txWaiting(nullptr), rxWaiting(nullptr), idle(true) { InterruptDisableLock dLock; if(id<0 || id>1 || ports[id]!=0) errorHandler(UNEXPECTED); @@ -177,13 +178,13 @@ ssize_t LPC2000Serial::writeBlock(const void *buffer, size_t size, off_t where) if(len==0) break; } } else { - if(txQueue.IRQput(*buf)==true) + if(txQueue.tryPut(*buf)) { buf++; len--; } else { - FastInterruptEnableLock eLock(dLock); - txQueue.waitUntilNotFull(); + txWaiting=Thread::IRQgetCurrentThread(); + while(txWaiting) Thread::IRQenableIrqAndWait(dLock); } } } @@ -256,8 +257,14 @@ void LPC2000Serial::IRQhandleInterrupt() case 0x2: //THRE for(int i=0;i<hwTxQueueLen;i++) { - //If software queue empty, stop - if(txQueue.IRQget(c,hppw)==false) break; + if(txWaiting) + { + txWaiting->IRQwakeup(); + if(txWaiting->IRQgetPriority()> + Thread::IRQgetCurrentThread()->IRQgetPriority()) hppw=true; + txWaiting=nullptr; + } + if(txQueue.tryGet(c)==false) break; //If software queue empty, stop serial->THR=c; } break; @@ -267,7 +274,7 @@ void LPC2000Serial::IRQhandleInterrupt() rxWaiting->IRQwakeup(); if(rxWaiting->IRQgetPriority()> Thread::IRQgetCurrentThread()->IRQgetPriority()) hppw=true; - rxWaiting=0; + rxWaiting=nullptr; } if(hppw) Scheduler::IRQfindNextThread(); } diff --git a/miosix/arch/common/drivers/serial_lpc2000.h b/miosix/arch/common/drivers/serial_lpc2000.h index 67a41cf0..7a624cf8 100644 --- a/miosix/arch/common/drivers/serial_lpc2000.h +++ b/miosix/arch/common/drivers/serial_lpc2000.h @@ -171,8 +171,9 @@ private: FastMutex txMutex;///< Mutex used to guard the tx queue FastMutex rxMutex;///< Mutex used to guard the rx queue - Queue<char,swTxQueue> txQueue;///< Tx software queue + DynUnsyncQueue<char> txQueue;///< Rx software queue DynUnsyncQueue<char> rxQueue;///< Rx software queue + Thread *txWaiting; ///< Thread waiting on rx queue Thread *rxWaiting; ///< Thread waiting on rx queue bool idle; ///< Receiver idle diff --git a/miosix/kernel/queue.h b/miosix/kernel/queue.h index a870da2d..cf8a82d8 100644 --- a/miosix/kernel/queue.h +++ b/miosix/kernel/queue.h @@ -46,6 +46,11 @@ namespace miosix { * Dynamically creating a queue with new or on the stack must be done with care, * to avoid deleting a queue with a waiting thread, and to avoid situations * where a thread tries to access a deleted queue. + * + * \warning the type T most not have a copy constructor or operator= that + * allocate memory, as allocating memory in FastInterruptDisableLock context + * and within interrupt handlers is not possible. + * * \tparam T the type of elements in the queue * \tparam len the length of the Queue. Value 0 is forbidden */ @@ -77,35 +82,36 @@ public: * \return the maximum number of elements the queue can hold */ unsigned int capacity() const { return len; } - - /** - * If a queue is empty, waits until the queue is not empty. - */ - void waitUntilNotEmpty(); - - /** - * If a queue is full, waits until the queue is not full - */ - void waitUntilNotFull(); /** - * Put an element to the queue. If the queue is full, then sleep until a + * Put an element to the queue. If the queue is full, then wait until a * place becomes available. * \param elem element to add to the queue */ void put(const T& elem); /** - * Put an element to the queue, only if th queue is not full.<br> + * Put an element to the queue. If the queue is full, then use the dLock + * parameter to enable interrupts and wait until a place becomes available. + * Being a blocking call, it cannot be called inside an IRQ, it can only be + * called when interrupts are disabled. + * \param elem element to add to the queue + * \param dLock the FastInterruptDisableLock object that was used to disable + * interrupts in the current context. + */ + void IRQputBlocking(const T& elem, FastInterruptDisableLock& dLock); + + /** + * Put an element to the queue, only if the queue is not full.<br> * Can ONLY be used inside an IRQ, or when interrupts are disabled. * \param elem element to add. The element has been added only if the * return value is true * \return true if the queue was not full. */ - bool IRQput(const T& elem); + bool IRQput(const T& elem) { return IRQput(elem,nullptr); } /** - * Put an element to the queue, only if th queue is not full.<br> + * Put an element to the queue, only if the queue is not full.<br> * Can ONLY be used inside an IRQ, or when interrupts are disabled. * \param elem element to add. The element has been added only if the * return value is true @@ -114,7 +120,7 @@ public: * set to true * \return true if the queue was not full. */ - bool IRQput(const T& elem, bool& hppw); + bool IRQput(const T& elem, bool& hppw) { return IRQput(elem,&hppw); } /** * Get an element from the queue. If the queue is empty, then sleep until @@ -123,6 +129,17 @@ public: */ void get(T& elem); + /** + * Get an element from the queue. If the queue is empty, then use the dLock + * parameter to enable interrupts and wait until a place becomes available. + * Being a blocking call, it cannot be called inside an IRQ, it can only be + * called when interrupts are disabled. + * \param elem an element from the queue + * \param dLock the FastInterruptDisableLock object that was used to disable + * interrupts in the current context. + */ + void IRQgetBlocking(T& elem, FastInterruptDisableLock& dLock); + /** * Get an element from the queue, only if the queue is not empty.<br> * Can ONLY be used inside an IRQ, or when interrupts are disabled. @@ -130,7 +147,7 @@ public: * return value is true * \return true if the queue was not empty */ - bool IRQget(T& elem); + bool IRQget(T& elem) { return IRQget(elem,nullptr); } /** * Get an element from the queue, only if the queue is not empty.<br> @@ -142,7 +159,7 @@ public: * set to true * \return true if the queue was not empty */ - bool IRQget(T& elem, bool& hppw); + bool IRQget(T& elem, bool& hppw) { return IRQget(elem,&hppw); } /** * Clear all items in the queue.<br> @@ -169,6 +186,28 @@ public: Queue& operator= (const Queue& s) = delete; private: + /** + * Put an element to the queue, only if the queue is not full. + * \param elem element to add. The element has been added only if the + * return value is true + * \param hppw is not modified if nullptr or no thread is woken or if the + * woken thread has a lower or equal priority than the currently running + * thread, else is set to true + * \return true if the queue was not full + */ + bool IRQput(const T& elem, bool *hppw); + + /** + * Get an element from the queue, only if the queue is not empty. + * \param elem an element from the queue. The element is valid only if the + * return value is true + * \param hppw is not modified if nullptr or no thread is woken or if the + * woken thread has a lower or equal priority than the currently running + * thread, else is set to true + * \return true if the queue was not empty + */ + bool IRQget(T& elem, bool *hppw); + /** * Wake an eventual waiting thread. * Must be called when interrupts are disabled @@ -189,10 +228,10 @@ private: }; template <typename T, unsigned int len> -void Queue<T,len>::waitUntilNotEmpty() +void Queue<T,len>::put(const T& elem) { FastInterruptDisableLock dLock; - while(isEmpty()) + while(IRQput(elem)==false) { waiting=Thread::IRQgetCurrentThread(); Thread::IRQenableIrqAndWait(dLock); @@ -200,10 +239,9 @@ void Queue<T,len>::waitUntilNotEmpty() } template <typename T, unsigned int len> -void Queue<T,len>::waitUntilNotFull() +void Queue<T,len>::IRQputBlocking(const T& elem, FastInterruptDisableLock& dLock) { - FastInterruptDisableLock dLock; - while(isFull()) + while(IRQput(elem)==false) { waiting=Thread::IRQgetCurrentThread(); Thread::IRQenableIrqAndWait(dLock); @@ -211,10 +249,10 @@ void Queue<T,len>::waitUntilNotFull() } template <typename T, unsigned int len> -void Queue<T,len>::put(const T& elem) +void Queue<T,len>::get(T& elem) { FastInterruptDisableLock dLock; - while(IRQput(elem)==false) + while(IRQget(elem)==false) { waiting=Thread::IRQgetCurrentThread(); Thread::IRQenableIrqAndWait(dLock); @@ -222,33 +260,8 @@ void Queue<T,len>::put(const T& elem) } template <typename T, unsigned int len> -bool Queue<T,len>::IRQput(const T& elem) +void Queue<T,len>::IRQgetBlocking(T& elem, FastInterruptDisableLock& dLock) { - IRQwakeWaitingThread(); - if(isFull()) return false; - numElem++; - buffer[putPos]=elem; - if(++putPos==len) putPos=0; - return true; -} - -template <typename T, unsigned int len> -bool Queue<T,len>::IRQput(const T& elem, bool& hppw) -{ - if(waiting && (Thread::IRQgetCurrentThread()->IRQgetPriority() < - waiting->IRQgetPriority())) hppw=true; - IRQwakeWaitingThread(); - if(isFull()) return false; - numElem++; - buffer[putPos]=elem; - if(++putPos==len) putPos=0; - return true; -} - -template <typename T, unsigned int len> -void Queue<T,len>::get(T& elem) -{ - FastInterruptDisableLock dLock; while(IRQget(elem)==false) { waiting=Thread::IRQgetCurrentThread(); @@ -257,21 +270,23 @@ void Queue<T,len>::get(T& elem) } template <typename T, unsigned int len> -bool Queue<T,len>::IRQget(T& elem) +bool Queue<T,len>::IRQput(const T& elem, bool *hppw) { + if(hppw && waiting && (Thread::IRQgetCurrentThread()->IRQgetPriority() < + waiting->IRQgetPriority())) *hppw=true; IRQwakeWaitingThread(); - if(isEmpty()) return false; - numElem--; - elem=buffer[getPos]; - if(++getPos==len) getPos=0; + if(isFull()) return false; + numElem++; + buffer[putPos]=elem; + if(++putPos==len) putPos=0; return true; } template <typename T, unsigned int len> -bool Queue<T,len>::IRQget(T& elem, bool& hppw) +bool Queue<T,len>::IRQget(T& elem, bool *hppw) { - if(waiting && (Thread::IRQgetCurrentThread()->IRQgetPriority()) < - waiting->IRQgetPriority()) hppw=true; + if(hppw && waiting && (Thread::IRQgetCurrentThread()->IRQgetPriority()) < + waiting->IRQgetPriority()) *hppw=true; IRQwakeWaitingThread(); if(isEmpty()) return false; numElem--; -- GitLab