-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mem-ruby: Reduce handshaking between CorePair and dir #1117
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the changes we discussed earlier, one more.
Otherwise this looks good -- nice job!
FYI, I may not get to this until next week |
@NSurawar has changes to make from my suggestions already, so no worries. |
@NSurawar can you please wrap the changes up this week? |
d8c01e0
to
32c32b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok from my end, @abmerop let us know
@NSurawar : you'll also need to update this branch to the head of develop before this can be committed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable but I'd recommending going through and removing all of the dead code introduced by this change (CPUData can go, and wd_data action).
//Deprecated - WriteBack data (wb_data) is now sent with VicDirty itself | ||
//Leaving this in for reference | ||
assert(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just remove this. The error below should tell you which message type is missing
I'm sorry for the delay. I was caught up in some other work. |
Currently when data is downgraded by MOESI_AMD_Base-CorePair (e.g. due to a replacement) this requires a 4-way handshake between the CorePair and the dir. Specifically, the CorePair send a message telling the dir it'd like to downgrade then, the dir sends an ACK back and then, the CorePair writes the data back, and finally, the dir ACKs the writeback. This is very inefficient and not representative of how modern protocols downgrade a request. Accordingly, this commits updates the downgrade support such that the CorePair writes back the data immediately and then the dir ACKs it. Thus, this approach requires only a 2-way handshake. Change-Id: I7ebc85bb03e8ce46a8847e3240fc170120e9fcd6
I've removed the stale code now. Please let me know if any additional changes are required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks all for getting this across the line while I was away |
Currently when data is downgraded by MOESI_AMD_Base-CorePair (e.g. due to a replacement) this requires a 4-way handshake between the CorePair and the dir. Specifically, the CorePair send a message telling the dir it'd like to downgrade then, the dir sends an ACK back and then, the CorePair writes the data back, and finally, the dir ACKs the writeback.
This is very inefficient and not representative of how modern protocols downgrade a request. Accordingly, this commits updates the downgrade support such that the CorePair writes back the data immediately and then the dir ACKs it.
Thus, this approach requires only a 2-way handshake.
Change-Id: I7ebc85bb03e8ce46a8847e3240fc170120e9fcd6