> On 2011-01-12 17:04:00, Nilay Vaish wrote: > > > > Nilay Vaish wrote: > I don't know why the comment went missing. I'll post the question again. > I think the states MT_SB and MT_IB are not required. In fact, I am not > sure why unblock messages have to be sent out. > > Brad Beckmann wrote: > I'm confused. I just asked for the commented out transition to be > deleted. I'm not sure what comment you're referring to. > > When you say the unblock messages, I assume you are referring to the > WB_acks, right? I believe the acks need to be sent out because you want to > block the L1 until it receives the acks. Otherwise, sinking random writeback > acks can be confusing and lead to several bugs, which currently this protocol > definitely has. There may be further optimizations we can make, such as > removing the MT_SM and MT_IB states, as well as possibly combining L1_PUTX > and L1_PUTX_old events. However, I suggest making those optimizations in a > separate patch. In my opinion, right now the number one priority is to fix > this protocol as soon as possible. Otherwise when checked in, my series of > patches will expose several bugs in the protocol and thus break the > regression tester.
I am confused too. I think Nilay is talking about "UNBLOCK" messages that are sent when there is exclusive owner change for a cache block or a cache block is doing a M->S transition. In general, directory cache coherence protocols uses blocking states (in this protocol the states whose name ends with "B") to constrain amount of races as blocking helps in serializing requests. In theory you can get rid of UNBLOCKS through more transient states and/or through NACKs. For this particular case that Nilay mentioned here, the MT_SB and MT_IB states are used to make sure modified data is not lost when there is coherence permission down-gradation at the exclusive owner's cache due to request from other cores. I also totally agree with Brad that the purpose of this patch is to fix a broken protocol not to do optimization. And unlike MOESI_CMP_directory, this protocol is targeted to work as simple baseline protocol than highly optimized one. Thanks Arka - Arkaprava ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/420/#review723 ----------------------------------------------------------- On 2011-01-10 11:48:16, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/420/ > ----------------------------------------------------------- > > (Updated 2011-01-10 11:48:16) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Ruby: Fixes MESI CMP directory protocol > The current implementation of MESI CMP directory protocol is broken. > This patch, from Arkaprava Basu, fixes the protocol. > > > Diffs > ----- > > src/mem/protocol/MESI_CMP_directory-L1cache.sm c06505ff551e > src/mem/protocol/MESI_CMP_directory-L2cache.sm c06505ff551e > > Diff: http://reviews.m5sim.org/r/420/diff > > > Testing > ------- > > > Thanks, > > Nilay > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
