changeset 204a0f53035e in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=204a0f53035e
description:
mem: Clarify cache behaviour for pending dirty responses
This patch adds a bit of clarification around the assumptions made in
the cache when packets are sent out, and dirty responses are
pending. As part of the change, the marking of an MSHR as in service
is simplified slightly, and comments are added to explain what
assumptions are made.
diffstat:
src/mem/cache/base.hh | 4 ++--
src/mem/cache/cache.hh | 11 ++++++-----
src/mem/cache/cache_impl.hh | 36 ++++++++++++++++++++++++++----------
src/mem/cache/mshr.cc | 6 ++----
src/mem/cache/mshr.hh | 2 +-
src/mem/cache/mshr_queue.cc | 4 ++--
src/mem/cache/mshr_queue.hh | 7 +++++--
7 files changed, 44 insertions(+), 26 deletions(-)
diffs (185 lines):
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/base.hh
--- a/src/mem/cache/base.hh Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/base.hh Tue Feb 03 14:25:59 2015 -0500
@@ -218,11 +218,11 @@
return mshr;
}
- void markInServiceInternal(MSHR *mshr, PacketPtr pkt)
+ void markInServiceInternal(MSHR *mshr, bool pending_dirty_resp)
{
MSHRQueue *mq = mshr->queue;
bool wasFull = mq->isFull();
- mq->markInService(mshr, pkt);
+ mq->markInService(mshr, pending_dirty_resp);
if (wasFull && !mq->isFull()) {
clearBlocked((BlockedCause)mq->index);
}
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/cache.hh
--- a/src/mem/cache/cache.hh Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/cache.hh Tue Feb 03 14:25:59 2015 -0500
@@ -375,12 +375,13 @@
PacketPtr getTimingPacket();
/**
- * Marks a request as in service (sent on the bus). This can have side
- * effect since storage for no response commands is deallocated once they
- * are successfully sent.
- * @param pkt The request that was sent on the bus.
+ * Marks a request as in service (sent on the bus). This can have
+ * side effect since storage for no response commands is
+ * deallocated once they are successfully sent. Also remember if
+ * we are expecting a dirty response from another cache,
+ * effectively making this MSHR the ordering point.
*/
- void markInService(MSHR *mshr, PacketPtr pkt = NULL);
+ void markInService(MSHR *mshr, bool pending_dirty_resp);
/**
* Return whether there are any outstanding misses.
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/cache_impl.hh Tue Feb 03 14:25:59 2015 -0500
@@ -256,11 +256,9 @@
template<class TagStore>
void
-Cache<TagStore>::markInService(MSHR *mshr, PacketPtr pkt)
+Cache<TagStore>::markInService(MSHR *mshr, bool pending_dirty_resp)
{
- // packet can be either a request or response
-
- markInServiceInternal(mshr, pkt);
+ markInServiceInternal(mshr, pending_dirty_resp);
#if 0
if (mshr->originalCmd == MemCmd::HardPFReq) {
DPRINTF(HWPrefetch, "%s:Marking a HW_PF in service\n",
@@ -1745,7 +1743,7 @@
if (pkt->isInvalidate()) {
// Invalidation trumps our writeback... discard here
- markInService(mshr);
+ markInService(mshr, false);
delete wb_pkt;
}
} // writebacks.size()
@@ -1910,9 +1908,10 @@
snoop_pkt.senderState = mshr;
cpuSidePort->sendTimingSnoopReq(&snoop_pkt);
- // Check to see if the prefetch was squashed by an upper cache
- // Or if a writeback arrived between the time the prefetch was
- // placed in the MSHRs and when it was selected to send.
+ // Check to see if the prefetch was squashed by an upper
+ // cache (to prevent us from grabbing the line) or if a
+ // writeback arrived between the time the prefetch was
+ // placed in the MSHRs and when it was selected to be sent.
if (snoop_pkt.prefetchSquashed() || blk != NULL) {
DPRINTF(Cache, "Prefetch squashed by cache. "
"Deallocating mshr target %#x.\n", mshr->addr);
@@ -1926,8 +1925,13 @@
return NULL;
}
+ // Check if the prefetch hit a writeback in an upper cache
+ // and if so we will eventually get a HardPFResp from
+ // above
if (snoop_pkt.memInhibitAsserted()) {
- markInService(mshr, &snoop_pkt);
+ // If we are getting a non-shared response it is dirty
+ bool pending_dirty_resp = !snoop_pkt.sharedAsserted();
+ markInService(mshr, pending_dirty_resp);
DPRINTF(Cache, "Upward snoop of prefetch for addr"
" %#x (%s) hit\n",
tgt_pkt->getAddr(), tgt_pkt->isSecure()? "s": "ns");
@@ -2148,7 +2152,19 @@
// care about this packet and might override it before
// it gets retried
} else {
- cache.markInService(mshr, pkt);
+ // As part of the call to sendTimingReq the packet is
+ // forwarded to all neighbouring caches (and any
+ // caches above them) as a snoop. The packet is also
+ // sent to any potential cache below as the
+ // interconnect is not allowed to buffer the
+ // packet. Thus at this point we know if any of the
+ // neighbouring, or the downstream cache is
+ // responding, and if so, if it is with a dirty line
+ // or not.
+ bool pending_dirty_resp = !pkt->sharedAsserted() &&
+ pkt->memInhibitAsserted();
+
+ cache.markInService(mshr, pending_dirty_resp);
}
}
}
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/mshr.cc
--- a/src/mem/cache/mshr.cc Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/mshr.cc Tue Feb 03 14:25:59 2015 -0500
@@ -238,7 +238,7 @@
}
bool
-MSHR::markInService(PacketPtr pkt)
+MSHR::markInService(bool pending_dirty_resp)
{
assert(!inService);
if (isForwardNoResponse()) {
@@ -249,10 +249,8 @@
return true;
}
- assert(pkt != NULL);
inService = true;
- pendingDirty = targets.needsExclusive ||
- (!pkt->sharedAsserted() && pkt->memInhibitAsserted());
+ pendingDirty = targets.needsExclusive || pending_dirty_resp;
postInvalidate = postDowngrade = false;
if (!downstreamPending) {
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/mshr.hh
--- a/src/mem/cache/mshr.hh Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/mshr.hh Tue Feb 03 14:25:59 2015 -0500
@@ -225,7 +225,7 @@
void allocate(Addr addr, int size, PacketPtr pkt,
Tick when, Counter _order);
- bool markInService(PacketPtr pkt);
+ bool markInService(bool pending_dirty_resp);
void clearDownstreamPending();
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/mshr_queue.cc
--- a/src/mem/cache/mshr_queue.cc Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/mshr_queue.cc Tue Feb 03 14:25:59 2015 -0500
@@ -214,9 +214,9 @@
}
void
-MSHRQueue::markInService(MSHR *mshr, PacketPtr pkt)
+MSHRQueue::markInService(MSHR *mshr, bool pending_dirty_resp)
{
- if (mshr->markInService(pkt)) {
+ if (mshr->markInService(pending_dirty_resp)) {
deallocate(mshr);
} else {
readyList.erase(mshr->readyIter);
diff -r d95e81d44e36 -r 204a0f53035e src/mem/cache/mshr_queue.hh
--- a/src/mem/cache/mshr_queue.hh Tue Feb 03 14:25:58 2015 -0500
+++ b/src/mem/cache/mshr_queue.hh Tue Feb 03 14:25:59 2015 -0500
@@ -183,10 +183,13 @@
/**
* Mark the given MSHR as in service. This removes the MSHR from the
- * readyList. Deallocates the MSHR if it does not expect a response.
+ * readyList or deallocates the MSHR if it does not expect a response.
+ *
* @param mshr The MSHR to mark in service.
+ * @param pending_dirty_resp Whether we expect a dirty response
+ * from another cache
*/
- void markInService(MSHR *mshr, PacketPtr pkt);
+ void markInService(MSHR *mshr, bool pending_dirty_resp);
/**
* Mark an in service entry as pending, used to resend a request.
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
