From c23c6311c7c35002bb2bf91c24db7751ce867c37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89der=20F=2E=20Zulian?= Date: Mon, 16 Nov 2015 20:36:03 +0100 Subject: [PATCH] FIFO strict scheduler is now working properly. Summary: - The code was simplified - Using a deque instead of a vector to implement the FIFO. - Fixed a bug related to the removal of requests (payloads) from the FIFO. It was possible that old requests were not properly removed and then processed again after the memory manager had free()d the payload (and its extensions) generating a segmentation fault. --- .../resources/simulations/sim-batch.xml | 2 +- DRAMSys/simulator/src/controller/Controller.h | 13 +- .../src/controller/scheduler/FifoStrict.cpp | 128 +++++++++--------- .../src/controller/scheduler/FifoStrict.h | 23 ++-- 4 files changed, 78 insertions(+), 88 deletions(-) diff --git a/DRAMSys/simulator/resources/simulations/sim-batch.xml b/DRAMSys/simulator/resources/simulations/sim-batch.xml index c3ed7cbd..fbd50bf4 100644 --- a/DRAMSys/simulator/resources/simulations/sim-batch.xml +++ b/DRAMSys/simulator/resources/simulations/sim-batch.xml @@ -34,7 +34,7 @@ - + diff --git a/DRAMSys/simulator/src/controller/Controller.h b/DRAMSys/simulator/src/controller/Controller.h index 9889a697..01cf7c2a 100644 --- a/DRAMSys/simulator/src/controller/Controller.h +++ b/DRAMSys/simulator/src/controller/Controller.h @@ -151,7 +151,7 @@ void Controller::buildScheduler() } else if (selectedScheduler == "FIFO_STRICT") { - scheduler = new FifoStrict(*this, *controllerCore); + scheduler = new FifoStrict(*controllerCore); } else if (selectedScheduler == "FR_FCFS") { @@ -316,16 +316,7 @@ void Controller::controllerCorePEQCallback(tlm_generic_payload &payloa if (phase == BEGIN_RD || phase == BEGIN_WR) { - if(Configuration::getInstance().Scheduler == "FIFO_STRICT") - { - // special case for the Fifo_strict scheduler, because it may have to unblock - // the current front element - dynamic_cast(scheduler)->NotifyBeginRDWR(); - } - else - { - scheduleNextFromScheduler(DramExtension::getBank(payload)); - } + scheduleNextFromScheduler(DramExtension::getBank(payload)); } else if (phase == BEGIN_REFB) printDebugMessage("Entering REFB on bank " + to_string(bank.ID())); diff --git a/DRAMSys/simulator/src/controller/scheduler/FifoStrict.cpp b/DRAMSys/simulator/src/controller/scheduler/FifoStrict.cpp index 6b6244b4..4897b796 100644 --- a/DRAMSys/simulator/src/controller/scheduler/FifoStrict.cpp +++ b/DRAMSys/simulator/src/controller/scheduler/FifoStrict.cpp @@ -37,77 +37,83 @@ #include "FifoStrict.h" -void FifoStrict::schedule(gp *payload) +void FifoStrict::schedule(tlm::tlm_generic_payload *payload) { - buffer.push_back(payload); - printDebugMessage("New payload scheduled. Total number " + std::to_string(buffer.size()) + "\n"); -} - -void FifoStrict::printFrontElementState() -{ - if(buffer.size() > 0) - { - printDebugMessage("Front element state - Bank: " + DramExtension::getBank(buffer.front()).toString() + " Operation: " + - commandToString(getNextCommand(*buffer.front())) + "\n"); - } - else - { - printDebugMessage("No fron element. Buffer is empty \n "); - } -} - -void FifoStrict::NotifyBeginRDWR() -{ - buffer.erase(buffer.begin()); - printDebugMessage("Front element finished. New front Element: "); - printFrontElementState(); - - if(buffer.size() > 0) - { - // If the new front element was orginally blocked, because it had to wait on the old front element, we have to call - // controller.scheduleNextFromScheduler explicitly, otherwise there will be a deadlock in the system - controller.scheduleNextFromScheduler(DramExtension::getBank(buffer.front())); - } + Bank bank = DramExtension::getExtension(payload).getBank(); + buffer.push_back(std::pair(bank, payload)); } std::pair FifoStrict::getNextRequest(Bank bank) { - if(buffer.empty()) - { - return pair(Command::NOP, NULL); - } + if (!buffer.empty()) { - else if(DramExtension::getBank(buffer.front()) == bank) - { - Command command = getNextCommand(*buffer.front()); - pair result(command, buffer.front()); - printDebugMessage("Operation for front Element scheduled. "); - printFrontElementState(); + if (buffer.front().first == bank) { + // The next request in the FIFO is for the bank passed as parameter - return result; - } - else - { - for(unsigned int i = 1; i < buffer.size(); ++i) - { - if(DramExtension::getBank(buffer[i]) == bank) - { - Command command = getNextCommand(*buffer[i]); + tlm::tlm_generic_payload *payload = buffer.front().second; - // RD/WR operations have to be strictly in order across banks, so only allow progress on commands - // other than RD/WR commands. - if(commandIsIn(command, {Command::Read, Command::Write, Command::ReadA, Command::WriteA})) - { - printDebugMessage("Current queue element " + std::to_string(i) + " blocked. Waiting for earlier rd/wr to finish\n"); - return pair(Command::NOP, NULL); - } - else - { - return pair(command, buffer[i]); + // For a given request (a given payload) one or more commands will + // be sent to the DRAM in order to process it. + // + // getNextRequest() may be called more than once for the same + // enqueued request until the appropriate sequence of commands is + // sent to the DRAM. + // + // Every time getNextRequest() it is called it calls + // getNextCommand() that returns the suitable command to be sent + // to the DRAM. + // + // getNextCommand() returns the proper command based on the + // internal status of the DRAM. + // + // In the worst case getNextCommand() need to be called three + // times for a given element in the requests queue: first for + // precharge, second for activate and finally for read or write + // (accordingly the nature of the request). In contrast, for the + // case of an already open row (due to a previous request) the + // command itself could be directly issued. + // + Command command = IScheduler::getNextCommand(*payload); + + if(commandIsIn(command, {Command::Read, Command::Write, Command::ReadA, Command::WriteA})) { + buffer.pop_front(); + } + + return pair(command, payload); + + } else { + // The next request in the FIFO is NOT for the bank passed as parameter. + + // Search for the next request related to the bank passed as parameter. + for (auto req = buffer.begin(); req != buffer.end(); req++) { + if (req->first == bank) { + // Found a request to this bank in the queue + tlm::tlm_generic_payload *payload = req->second; + + // Get the appropriate command to be sent to the DRAM + // regarding this request. + // + // Commands other than read and write will be issued. + // Reads and writes will not be issued since this + // scheduler executes all read and writes in a strict + // order. + Command command = getNextCommand(*payload); + if(commandIsIn(command, {Command::Read, Command::Write, Command::ReadA, Command::WriteA})) { + // Reads and writes must be executed in order. Then if + // the next command for this request is read or write + // NOP will be returned and no operation will be + // performed. + return pair(Command::NOP, NULL); + } else { + // Commands other than read and write are issued normally. + return pair(command, payload); + } } } } - - return pair(Command::NOP, NULL); } + + // The FIFO is empty + return pair(Command::NOP, NULL); } + diff --git a/DRAMSys/simulator/src/controller/scheduler/FifoStrict.h b/DRAMSys/simulator/src/controller/scheduler/FifoStrict.h index e46add42..43f45d73 100644 --- a/DRAMSys/simulator/src/controller/scheduler/FifoStrict.h +++ b/DRAMSys/simulator/src/controller/scheduler/FifoStrict.h @@ -38,33 +38,26 @@ #ifndef FIFOSTRICT_H #define FIFOSTRICT_H -#include "../core/ControllerCore.h" -#include "../Command.h" -#include "../../common/dramExtension.h" -#include "IScheduler.h" #include #include #include +#include "../core/ControllerCore.h" +#include "../Command.h" +#include "IScheduler.h" + class FifoStrict : public IScheduler { public: - FifoStrict(IController &controller,ControllerCore &controllerCore) : - IScheduler(controllerCore), controller(controller) - {} - virtual ~FifoStrict() - {} + FifoStrict(ControllerCore &controllerCore) : IScheduler(controllerCore) {} + virtual ~FifoStrict() {} void schedule(gp* payload) override; std::pair getNextRequest(Bank bank) override; - void NotifyBeginRDWR(); - void NotifyBeginRD(); private: - std::vector buffer; - IController &controller; - void printFrontElementState(); + std::deque> buffer; }; +#endif /* FIFOSTRICT_H */ -#endif // FIFOSTRICT_H