Re: [gem5-users] Ruby functional read fails and potential fix
Hi Ciro,
Thank you for your reply. Looks like this patch does address the problem I
mentioned along with some other ones. Thanks for sharing.
Best Regards,
Shehab
On Thu, Apr 9, 2020 at 1:45 PM Ciro Santilli
wrote:
> Thanks for this Shehab,
>
> Could you compare your changes to this patchset:
> https://gem5-review.googlesource.com/c/public/gem5/+/22022/1
>
> On Thu, Apr 9, 2020 at 6:22 PM Shehab Elsayed
> wrote:
> >
> > Hello All,
> >
> > I was running some experiments and I ran into a problem with ruby where
> a functional read was failing. After some investigation I found that the
> reason was that the functional read was trying to read a line that was in a
> MaybeStale state (no ReadOnly or ReadWrite versions).
> >
> > I implemented a fix which so far seems to be working fine but I am not
> sure if there is a deeper problem that needs fixing or if my fix could
> present future problems.
> >
> > I am running Full System simulations with ARM ISA and MESI_Three_Level.
> >
> > Here is my fix (I have marked new lines with //--NEW--//):
> > Basically what this fix does is perform the functional read from the
> controller that has the line in the MaybeStale state if no ReadOnly or
> ReadWrite versions in any controller.
> >
> > bool
> > RubySystem::functionalRead(PacketPtr pkt)
> > {
> > Addr address(pkt->getAddr());
> > Addr line_address = makeLineAddress(address);
> >
> > AccessPermission access_perm = AccessPermission_NotPresent;
> > int num_controllers = m_abs_cntrl_vec.size();
> >
> > DPRINTF(RubySystem, "Functional Read request for %#x\n", address);
> >
> > unsigned int num_ro = 0;
> > unsigned int num_rw = 0;
> > unsigned int num_busy = 0;
> > unsigned int num_backing_store = 0;
> > unsigned int num_invalid = 0;
> > unsigned int num_maybe_stale = 0;//--NEW--//
> >
> > // In this loop we count the number of controllers that have the
> given
> > // address in read only, read write and busy states.
> > for (unsigned int i = 0; i < num_controllers; ++i) {
> >
> > // Ignore ATD controllers for functional reads
> > if (m_abs_cntrl_vec[i]->getType() == MachineType_ATD) {
> > continue;
> > }
> >
> > access_perm = m_abs_cntrl_vec[i]->
> getAccessPermission(line_address);
> > if (access_perm == AccessPermission_Read_Only)
> > num_ro++;
> > else if (access_perm == AccessPermission_Read_Write)
> > num_rw++;
> > else if (access_perm == AccessPermission_Busy)
> > num_busy++;
> > else if (access_perm == AccessPermission_Backing_Store)
> > // See RubySlicc_Exports.sm for details, but Backing_Store
> is meant
> > // to represent blocks in memory *for Broadcast/Snooping
> protocols*,
> > // where memory has no idea whether it has an exclusive copy
> of data
> > // or not.
> > num_backing_store++;
> > else if (access_perm == AccessPermission_Invalid ||
> > access_perm == AccessPermission_NotPresent)
> > num_invalid++;
> > else if (access_perm == AccessPermission_Maybe_Stale)
> //--NEW--//
> > num_maybe_stale++;
> //--NEW--//
> > }
> >
> > // This if case is meant to capture what happens in a Broadcast/Snoop
> > // protocol where the block does not exist in the cache hierarchy.
> You
> > // only want to read from the Backing_Store memory if there is no
> copy in
> > // the cache hierarchy, otherwise you want to try to read the RO or
> RW
> > // copies existing in the cache hierarchy (covered by the else
> statement).
> > // The reason is because the Backing_Store memory could easily be
> stale, if
> > // there are copies floating around the cache hierarchy, so you want
> to read
> > // it only if it's not in the cache hierarchy at all.
> > if (num_invalid == (num_controllers - 1) && num_backing_store == 1) {
> > DPRINTF(RubySystem, "only copy in Backing_Store memory, read
> from it\n");
> > for (unsigned int i = 0; i < num_controllers; ++i) {
> > access_perm =
> m_abs_cntrl_vec[i]->getAccessPermission(line_address);
> > if (access_perm == AccessPermission_Backing_Store) {
> > m_abs_cntrl_vec[i]->functionalRead(line_address, pkt);
> > return true;
> > }
> > }
> > } else if (num_ro > 0 || num_rw >= 1 || num_maybe_stale > 0) {
> //--NEW--//
> > if (num_rw > 1) {
> > // We iterate over the vector of abstract controllers, and
> return
> > // the first copy found. If we have more than one cache with
> block
> > // in writable permission, the first one found would be
> returned.
> > warn("More than one Abstract Controller with RW permission
> for "
> > "addr: %#x
Re: [gem5-users] Ruby functional read fails and potential fix
Thanks for this Shehab,
Could you compare your changes to this patchset:
https://gem5-review.googlesource.com/c/public/gem5/+/22022/1
On Thu, Apr 9, 2020 at 6:22 PM Shehab Elsayed wrote:
>
> Hello All,
>
> I was running some experiments and I ran into a problem with ruby where a
> functional read was failing. After some investigation I found that the reason
> was that the functional read was trying to read a line that was in a
> MaybeStale state (no ReadOnly or ReadWrite versions).
>
> I implemented a fix which so far seems to be working fine but I am not sure
> if there is a deeper problem that needs fixing or if my fix could present
> future problems.
>
> I am running Full System simulations with ARM ISA and MESI_Three_Level.
>
> Here is my fix (I have marked new lines with //--NEW--//):
> Basically what this fix does is perform the functional read from the
> controller that has the line in the MaybeStale state if no ReadOnly or
> ReadWrite versions in any controller.
>
> bool
> RubySystem::functionalRead(PacketPtr pkt)
> {
> Addr address(pkt->getAddr());
> Addr line_address = makeLineAddress(address);
>
> AccessPermission access_perm = AccessPermission_NotPresent;
> int num_controllers = m_abs_cntrl_vec.size();
>
> DPRINTF(RubySystem, "Functional Read request for %#x\n", address);
>
> unsigned int num_ro = 0;
> unsigned int num_rw = 0;
> unsigned int num_busy = 0;
> unsigned int num_backing_store = 0;
> unsigned int num_invalid = 0;
> unsigned int num_maybe_stale = 0;//--NEW--//
>
> // In this loop we count the number of controllers that have the given
> // address in read only, read write and busy states.
> for (unsigned int i = 0; i < num_controllers; ++i) {
>
> // Ignore ATD controllers for functional reads
> if (m_abs_cntrl_vec[i]->getType() == MachineType_ATD) {
> continue;
> }
>
> access_perm = m_abs_cntrl_vec[i]-> getAccessPermission(line_address);
> if (access_perm == AccessPermission_Read_Only)
> num_ro++;
> else if (access_perm == AccessPermission_Read_Write)
> num_rw++;
> else if (access_perm == AccessPermission_Busy)
> num_busy++;
> else if (access_perm == AccessPermission_Backing_Store)
> // See RubySlicc_Exports.sm for details, but Backing_Store is
> meant
> // to represent blocks in memory *for Broadcast/Snooping
> protocols*,
> // where memory has no idea whether it has an exclusive copy of
> data
> // or not.
> num_backing_store++;
> else if (access_perm == AccessPermission_Invalid ||
> access_perm == AccessPermission_NotPresent)
> num_invalid++;
> else if (access_perm == AccessPermission_Maybe_Stale)
> //--NEW--//
> num_maybe_stale++;
> //--NEW--//
> }
>
> // This if case is meant to capture what happens in a Broadcast/Snoop
> // protocol where the block does not exist in the cache hierarchy. You
> // only want to read from the Backing_Store memory if there is no copy in
> // the cache hierarchy, otherwise you want to try to read the RO or RW
> // copies existing in the cache hierarchy (covered by the else statement).
> // The reason is because the Backing_Store memory could easily be stale,
> if
> // there are copies floating around the cache hierarchy, so you want to
> read
> // it only if it's not in the cache hierarchy at all.
> if (num_invalid == (num_controllers - 1) && num_backing_store == 1) {
> DPRINTF(RubySystem, "only copy in Backing_Store memory, read from
> it\n");
> for (unsigned int i = 0; i < num_controllers; ++i) {
> access_perm =
> m_abs_cntrl_vec[i]->getAccessPermission(line_address);
> if (access_perm == AccessPermission_Backing_Store) {
> m_abs_cntrl_vec[i]->functionalRead(line_address, pkt);
> return true;
> }
> }
> } else if (num_ro > 0 || num_rw >= 1 || num_maybe_stale > 0) {
> //--NEW--//
> if (num_rw > 1) {
> // We iterate over the vector of abstract controllers, and return
> // the first copy found. If we have more than one cache with block
> // in writable permission, the first one found would be returned.
> warn("More than one Abstract Controller with RW permission for "
> "addr: %#x on cacheline: %#x.", address, line_address);
> }
> // In Broadcast/Snoop protocols, this covers if you know the block
> // exists somewhere in the caching hierarchy, then you want to read
> any
> // valid RO or RW block. In directory protocols, same thing, you want
> // to read any valid readable copy of
[gem5-users] Ruby functional read fails and potential fix
Hello All,
I was running some experiments and I ran into a problem with ruby where a
functional read was failing. After some investigation I found that the
reason was that the functional read was trying to read a line that was in a
MaybeStale state (no ReadOnly or ReadWrite versions).
I implemented a fix which so far seems to be working fine but I am not sure
if there is a deeper problem that needs fixing or if my fix could present
future problems.
I am running Full System simulations with ARM ISA and MESI_Three_Level.
Here is my fix (I have marked new lines with //--NEW--//):
Basically what this fix does is perform the functional read from the
controller that has the line in the MaybeStale state if no ReadOnly or
ReadWrite versions in any controller.
bool
RubySystem::functionalRead(PacketPtr pkt)
{
Addr address(pkt->getAddr());
Addr line_address = makeLineAddress(address);
AccessPermission access_perm = AccessPermission_NotPresent;
int num_controllers = m_abs_cntrl_vec.size();
DPRINTF(RubySystem, "Functional Read request for %#x\n", address);
unsigned int num_ro = 0;
unsigned int num_rw = 0;
unsigned int num_busy = 0;
unsigned int num_backing_store = 0;
unsigned int num_invalid = 0;
unsigned int num_maybe_stale = 0;//--NEW--//
// In this loop we count the number of controllers that have the given
// address in read only, read write and busy states.
for (unsigned int i = 0; i < num_controllers; ++i) {
// Ignore ATD controllers for functional reads
if (m_abs_cntrl_vec[i]->getType() == MachineType_ATD) {
continue;
}
access_perm = m_abs_cntrl_vec[i]->
getAccessPermission(line_address);
if (access_perm == AccessPermission_Read_Only)
num_ro++;
else if (access_perm == AccessPermission_Read_Write)
num_rw++;
else if (access_perm == AccessPermission_Busy)
num_busy++;
else if (access_perm == AccessPermission_Backing_Store)
// See RubySlicc_Exports.sm for details, but Backing_Store is
meant
// to represent blocks in memory *for Broadcast/Snooping
protocols*,
// where memory has no idea whether it has an exclusive copy of
data
// or not.
num_backing_store++;
else if (access_perm == AccessPermission_Invalid ||
access_perm == AccessPermission_NotPresent)
num_invalid++;
else if (access_perm == AccessPermission_Maybe_Stale)
//--NEW--//
num_maybe_stale++;
//--NEW--//
}
// This if case is meant to capture what happens in a Broadcast/Snoop
// protocol where the block does not exist in the cache hierarchy. You
// only want to read from the Backing_Store memory if there is no copy
in
// the cache hierarchy, otherwise you want to try to read the RO or RW
// copies existing in the cache hierarchy (covered by the else
statement).
// The reason is because the Backing_Store memory could easily be
stale, if
// there are copies floating around the cache hierarchy, so you want to
read
// it only if it's not in the cache hierarchy at all.
if (num_invalid == (num_controllers - 1) && num_backing_store == 1) {
DPRINTF(RubySystem, "only copy in Backing_Store memory, read from
it\n");
for (unsigned int i = 0; i < num_controllers; ++i) {
access_perm =
m_abs_cntrl_vec[i]->getAccessPermission(line_address);
if (access_perm == AccessPermission_Backing_Store) {
m_abs_cntrl_vec[i]->functionalRead(line_address, pkt);
return true;
}
}
} else if (num_ro > 0 || num_rw >= 1 || num_maybe_stale > 0)
{ //--NEW--//
if (num_rw > 1) {
// We iterate over the vector of abstract controllers, and
return
// the first copy found. If we have more than one cache with
block
// in writable permission, the first one found would be
returned.
warn("More than one Abstract Controller with RW permission for "
"addr: %#x on cacheline: %#x.", address, line_address);
}
// In Broadcast/Snoop protocols, this covers if you know the block
// exists somewhere in the caching hierarchy, then you want to read
any
// valid RO or RW block. In directory protocols, same thing, you
want
// to read any valid readable copy of the block.
DPRINTF(RubySystem, "num_busy = %d, num_ro = %d, num_rw = %d\n",
num_busy, num_ro, num_rw);
// In this loop, we try to figure which controller has a read only
or
// a read write copy of the given address. Any valid copy would
suffice
// for a functional read.
// Sometimes the functional read is to a line that has recently
// transitioned to MaybeStale state and no other controller has it
in
// a
