From e2b3f0b8e4e50e40637361fe095ca2cfef914430 Mon Sep 17 00:00:00 2001 From: Yu-hsin Wang Date: Wed, 6 Dec 2023 13:25:02 +0800 Subject: [PATCH 1/2] mem: Add a flag on AbstractMemory to control statistics collection The stats initialization in the AbstractMemory allocates the space according to the max requestors of the System. This may cause issues in multiple system simulation. Given there are two system A and B. A has one requestor and a memory, while B has two requestors. When the requestor with requestor id 2 sending requests to the meomry in A, the simulator would crash because requestor id 2 is out of the allocated space. Current solution is adding a SysBridge between across A and B which would rewrite the requestor id to a valid one. This solution works but it needs to the bridge at the correct boundary which may not easy. In addition, the stats would record a mapped data which may not accurate. To reduce the complexity, we add an flag to AbstractMemory to control the stats. If users don't want the statistics and want to solve the cross system issue simply, users can disable the statistics collection. We also makes the flag by default True to not disturb current users. Change-Id: Ibb46a63d216d4f310b3e920815a295073496ea6e --- src/mem/AbstractMemory.py | 2 ++ src/mem/abstract_mem.cc | 23 ++++++++++++++--------- src/mem/abstract_mem.hh | 3 +++ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/mem/AbstractMemory.py b/src/mem/AbstractMemory.py index 57e47adcb1..32d18adab2 100644 --- a/src/mem/AbstractMemory.py +++ b/src/mem/AbstractMemory.py @@ -76,3 +76,5 @@ class AbstractMemory(ClockedObject): ) writeable = Param.Bool(True, "Allow writes to this memory") + + collect_stats = Param.Bool(True, "Collect traffic statistics") diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc index 9340f7e96f..f22ea35527 100644 --- a/src/mem/abstract_mem.cc +++ b/src/mem/abstract_mem.cc @@ -63,8 +63,8 @@ AbstractMemory::AbstractMemory(const Params &p) : MemBackdoor::Readable | MemBackdoor::Writeable : MemBackdoor::Readable)), confTableReported(p.conf_table_reported), inAddrMap(p.in_addr_map), - kvmMap(p.kvm_map), writeable(p.writeable), _system(NULL), - stats(*this) + kvmMap(p.kvm_map), writeable(p.writeable), collectStats(p.collect_stats), + _system(NULL), stats(*this) { panic_if(!range.valid() || !range.size(), "Memory range %s must be valid with non-zero size.", @@ -433,7 +433,8 @@ AbstractMemory::access(PacketPtr pkt) assert(!pkt->req->isInstFetch()); TRACE_PACKET("Read/Write"); - stats.numOther[pkt->req->requestorId()]++; + if (collectStats) + stats.numOther[pkt->req->requestorId()]++; } } else if (pkt->isRead()) { assert(!pkt->isWrite()); @@ -447,10 +448,12 @@ AbstractMemory::access(PacketPtr pkt) pkt->setData(host_addr); } TRACE_PACKET(pkt->req->isInstFetch() ? "IFetch" : "Read"); - stats.numReads[pkt->req->requestorId()]++; - stats.bytesRead[pkt->req->requestorId()] += pkt->getSize(); - if (pkt->req->isInstFetch()) - stats.bytesInstRead[pkt->req->requestorId()] += pkt->getSize(); + if (collectStats) { + stats.numReads[pkt->req->requestorId()]++; + stats.bytesRead[pkt->req->requestorId()] += pkt->getSize(); + if (pkt->req->isInstFetch()) + stats.bytesInstRead[pkt->req->requestorId()] += pkt->getSize(); + } } else if (pkt->isInvalidate() || pkt->isClean()) { assert(!pkt->isWrite()); // in a fastmem system invalidating and/or cleaning packets @@ -466,8 +469,10 @@ AbstractMemory::access(PacketPtr pkt) } assert(!pkt->req->isInstFetch()); TRACE_PACKET("Write"); - stats.numWrites[pkt->req->requestorId()]++; - stats.bytesWritten[pkt->req->requestorId()] += pkt->getSize(); + if (collectStats) { + stats.numWrites[pkt->req->requestorId()]++; + stats.bytesWritten[pkt->req->requestorId()] += pkt->getSize(); + } } } else { panic("Unexpected packet %s", pkt->print()); diff --git a/src/mem/abstract_mem.hh b/src/mem/abstract_mem.hh index 7f12487421..8c85f4503e 100644 --- a/src/mem/abstract_mem.hh +++ b/src/mem/abstract_mem.hh @@ -132,6 +132,9 @@ class AbstractMemory : public ClockedObject // Are writes allowed to this memory const bool writeable; + // Should collect traffic statistics + const bool collectStats; + std::list lockedAddrList; // helper function for checkLockedAddrs(): we really want to From 5de50cc9dd6c6e8dc2c305d991e73e1c43fb5880 Mon Sep 17 00:00:00 2001 From: Yu-hsin Wang Date: Thu, 7 Dec 2023 10:21:28 +0800 Subject: [PATCH 2/2] mem: update flag description and the if-block style Change-Id: Iac727d38b88f1818aeeccf0d8de639fe18759074 --- src/mem/AbstractMemory.py | 9 ++++++++- src/mem/abstract_mem.cc | 6 ++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/mem/AbstractMemory.py b/src/mem/AbstractMemory.py index 32d18adab2..f00e2868da 100644 --- a/src/mem/AbstractMemory.py +++ b/src/mem/AbstractMemory.py @@ -77,4 +77,11 @@ class AbstractMemory(ClockedObject): writeable = Param.Bool(True, "Allow writes to this memory") - collect_stats = Param.Bool(True, "Collect traffic statistics") + collect_stats = Param.Bool( + True, + "Collect statistics per requestor for " + "each type of access. Set this to `False` if " + "requestors may be unknown or when running " + "with multiple `System` objects without a " + "`SysBridge`.", + ) diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc index f22ea35527..461fd4c1fe 100644 --- a/src/mem/abstract_mem.cc +++ b/src/mem/abstract_mem.cc @@ -433,8 +433,9 @@ AbstractMemory::access(PacketPtr pkt) assert(!pkt->req->isInstFetch()); TRACE_PACKET("Read/Write"); - if (collectStats) + if (collectStats) { stats.numOther[pkt->req->requestorId()]++; + } } } else if (pkt->isRead()) { assert(!pkt->isWrite()); @@ -451,8 +452,9 @@ AbstractMemory::access(PacketPtr pkt) if (collectStats) { stats.numReads[pkt->req->requestorId()]++; stats.bytesRead[pkt->req->requestorId()] += pkt->getSize(); - if (pkt->req->isInstFetch()) + if (pkt->req->isInstFetch()) { stats.bytesInstRead[pkt->req->requestorId()] += pkt->getSize(); + } } } else if (pkt->isInvalidate() || pkt->isClean()) { assert(!pkt->isWrite());