gem5-dev@gem5.org

The gem5 Developer List

View all threads

[S] Change in gem5/gem5[develop]: mem: Handle DRAM write queue drain and disabled power down

MP
Matthew Poremba (Gerrit)
Thu, May 25, 2023 7:14 PM

Matthew Poremba has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/69917?usp=email )

Change subject: mem: Handle DRAM write queue drain and disabled power down
......................................................................

mem: Handle DRAM write queue drain and disabled power down

Write queue drain logic seems off currently. An event is scheduled if
the write queue is empty instead of non-empty. There is no check to see
if draining is complete when bus is in write mode. Finally the power
down check on drain always fails if DRAM powerdown is disabled.

This changeset reverses the drain conditional for the write queue to
schedule an event if the write queue is not empty and checks in the
event processing method that the queues are all empty so that
signalDrainDone can be called. Lastly the powerdown state is ignored if
DRAM powerdown is disabled. Powerdown is disabled in the GPU_VIPER
protocol by default. This changeset successfully drains and checkpoints
a GPUFS simulation using GPU_VIPER protocol.

Change-Id: I5459856a694c9054b28677049a06b99b9ad91bbb
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/69917
Tested-by: kokoro noreply+kokoro@google.com
Maintainer: Jason Lowe-Power power.jg@gmail.com
Reviewed-by: Jason Lowe-Power power.jg@gmail.com

M src/mem/dram_interface.hh
M src/mem/mem_ctrl.cc
2 files changed, 23 insertions(+), 4 deletions(-)

Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass

diff --git a/src/mem/dram_interface.hh b/src/mem/dram_interface.hh
index fa9d319..e20e33f 100644
--- a/src/mem/dram_interface.hh
+++ b/src/mem/dram_interface.hh
@@ -380,7 +380,18 @@
* @param Return true if the rank is idle from a bank
*        and power point of view
*/

  •    bool inPwrIdleState() const { return pwrState == PWR_IDLE; }
    
  •    bool
    
  •    inPwrIdleState() const
    
  •    {
    
  •        // If powerdown is not enabled, then the ranks never go to idle
    
  •        // states. In that case return true here to prevent  
    

checkpointing

  •        // from getting stuck waiting for DRAM to be idle.
    
  •        if (!dram.enableDRAMPowerdown) {
    
  •            return true;
    
  •        }
    
  •        return pwrState == PWR_IDLE;
    
  •    }
    
        /**
         * Trigger a self-refresh exit if there are entries enqueued
    

diff --git a/src/mem/mem_ctrl.cc b/src/mem/mem_ctrl.cc
index 543d637..290db3e 100644
--- a/src/mem/mem_ctrl.cc
+++ b/src/mem/mem_ctrl.cc
@@ -908,6 +908,13 @@
}
}

  • if (drainState() == DrainState::Draining && !totalWriteQueueSize &&
  •    !totalReadQueueSize && respQEmpty() && allIntfDrained()) {
    
  •    DPRINTF(Drain, "MemCtrl controller done draining\n");
    
  •    signalDrainDone();
    
  • }
  • // updates current state
    busState = busStateNext;
    

@@ -1411,8 +1418,8 @@
{
// if there is anything in any of our internal queues, keep track
// of that as well

  • if (!(!totalWriteQueueSize && !totalReadQueueSize && respQueue.empty()
    &&
  •      allIntfDrained())) {
    
  • if (totalWriteQueueSize || totalReadQueueSize || !respQueue.empty() ||
  •      !allIntfDrained()) {
    
        DPRINTF(Drain, "Memory controller not drained, write: %d,  
    

read: %d,"
" resp: %d\n", totalWriteQueueSize, totalReadQueueSize,
@@ -1420,7 +1427,8 @@

      // the only queue that is not drained automatically over time
      // is the write queue, thus kick things into action if needed
  •    if (!totalWriteQueueSize && !nextReqEvent.scheduled()) {
    
  •    if (totalWriteQueueSize && !nextReqEvent.scheduled()) {
    
  •        DPRINTF(Drain,"Scheduling nextReqEvent from drain\n");
            schedule(nextReqEvent, curTick());
        }
    

--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/69917?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I5459856a694c9054b28677049a06b99b9ad91bbb
Gerrit-Change-Number: 69917
Gerrit-PatchSet: 4
Gerrit-Owner: Matthew Poremba matthew.poremba@amd.com
Gerrit-Reviewer: Jason Lowe-Power jason@lowepower.com
Gerrit-Reviewer: Jason Lowe-Power power.jg@gmail.com
Gerrit-Reviewer: Matthew Poremba matthew.poremba@amd.com
Gerrit-Reviewer: Nikos Nikoleris nikos.nikoleris@arm.com
Gerrit-Reviewer: kokoro noreply+kokoro@google.com
Gerrit-CC: Matt Sinclair mattdsinclair.wisc@gmail.com
Gerrit-CC: Matt Sinclair mattdsinclair@gmail.com
Gerrit-CC: Melissa Jost mkjost@ucdavis.edu
Gerrit-CC: VISHNU RAMADAS vramadas@wisc.edu

Matthew Poremba has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/69917?usp=email ) Change subject: mem: Handle DRAM write queue drain and disabled power down ...................................................................... mem: Handle DRAM write queue drain and disabled power down Write queue drain logic seems off currently. An event is scheduled if the write queue is empty instead of non-empty. There is no check to see if draining is complete when bus is in write mode. Finally the power down check on drain always fails if DRAM powerdown is disabled. This changeset reverses the drain conditional for the write queue to schedule an event if the write queue is *not* empty and checks in the event processing method that the queues are all empty so that signalDrainDone can be called. Lastly the powerdown state is ignored if DRAM powerdown is disabled. Powerdown is disabled in the GPU_VIPER protocol by default. This changeset successfully drains and checkpoints a GPUFS simulation using GPU_VIPER protocol. Change-Id: I5459856a694c9054b28677049a06b99b9ad91bbb Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/69917 Tested-by: kokoro <noreply+kokoro@google.com> Maintainer: Jason Lowe-Power <power.jg@gmail.com> Reviewed-by: Jason Lowe-Power <power.jg@gmail.com> --- M src/mem/dram_interface.hh M src/mem/mem_ctrl.cc 2 files changed, 23 insertions(+), 4 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/mem/dram_interface.hh b/src/mem/dram_interface.hh index fa9d319..e20e33f 100644 --- a/src/mem/dram_interface.hh +++ b/src/mem/dram_interface.hh @@ -380,7 +380,18 @@ * @param Return true if the rank is idle from a bank * and power point of view */ - bool inPwrIdleState() const { return pwrState == PWR_IDLE; } + bool + inPwrIdleState() const + { + // If powerdown is not enabled, then the ranks never go to idle + // states. In that case return true here to prevent checkpointing + // from getting stuck waiting for DRAM to be idle. + if (!dram.enableDRAMPowerdown) { + return true; + } + + return pwrState == PWR_IDLE; + } /** * Trigger a self-refresh exit if there are entries enqueued diff --git a/src/mem/mem_ctrl.cc b/src/mem/mem_ctrl.cc index 543d637..290db3e 100644 --- a/src/mem/mem_ctrl.cc +++ b/src/mem/mem_ctrl.cc @@ -908,6 +908,13 @@ } } + if (drainState() == DrainState::Draining && !totalWriteQueueSize && + !totalReadQueueSize && respQEmpty() && allIntfDrained()) { + + DPRINTF(Drain, "MemCtrl controller done draining\n"); + signalDrainDone(); + } + // updates current state busState = busStateNext; @@ -1411,8 +1418,8 @@ { // if there is anything in any of our internal queues, keep track // of that as well - if (!(!totalWriteQueueSize && !totalReadQueueSize && respQueue.empty() && - allIntfDrained())) { + if (totalWriteQueueSize || totalReadQueueSize || !respQueue.empty() || + !allIntfDrained()) { DPRINTF(Drain, "Memory controller not drained, write: %d, read: %d," " resp: %d\n", totalWriteQueueSize, totalReadQueueSize, @@ -1420,7 +1427,8 @@ // the only queue that is not drained automatically over time // is the write queue, thus kick things into action if needed - if (!totalWriteQueueSize && !nextReqEvent.scheduled()) { + if (totalWriteQueueSize && !nextReqEvent.scheduled()) { + DPRINTF(Drain,"Scheduling nextReqEvent from drain\n"); schedule(nextReqEvent, curTick()); } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/69917?usp=email To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: public/gem5 Gerrit-Branch: develop Gerrit-Change-Id: I5459856a694c9054b28677049a06b99b9ad91bbb Gerrit-Change-Number: 69917 Gerrit-PatchSet: 4 Gerrit-Owner: Matthew Poremba <matthew.poremba@amd.com> Gerrit-Reviewer: Jason Lowe-Power <jason@lowepower.com> Gerrit-Reviewer: Jason Lowe-Power <power.jg@gmail.com> Gerrit-Reviewer: Matthew Poremba <matthew.poremba@amd.com> Gerrit-Reviewer: Nikos Nikoleris <nikos.nikoleris@arm.com> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com> Gerrit-CC: Matt Sinclair <mattdsinclair.wisc@gmail.com> Gerrit-CC: Matt Sinclair <mattdsinclair@gmail.com> Gerrit-CC: Melissa Jost <mkjost@ucdavis.edu> Gerrit-CC: VISHNU RAMADAS <vramadas@wisc.edu>