Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Nilay Vaish]

> 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.
> 
> Arkaprava Basu wrote:
> 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

Arka's right, I was talking about those very 'UNBLOCK' messages. I
agree this discussion is unrelated to the changes necessary for fixing
the protocol. When I fixed the protocol, I was trying to reason why
those states might have been added. I think the state MT_IIB makes 
sure that the L2 cache receives the modified data from the L1 cache. Once the 
data has been received, the L2 cache state should directly move to SS.


- Nilay


---
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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Nilay Vaish]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/420/#review734
---


I am going to commit this patch soon. Along with the patch, I am also
thinking of updating the reference outputs for the four regression tests
done on MESI_CMP_directory. Currently, 60.rubytest fails on running the tests.

- Nilay


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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Arkaprava Basu]

> 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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Brad Beckmann]

> 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.

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.


- Brad


---
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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Nilay Vaish]

> On 2011-01-12 17:04:00, 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.


- Nilay


---
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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Nilay Vaish]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/420/#review723
---


- Nilay


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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Brad Beckmann]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/420/#review721
---


After you delete the commented out lines, please check it in.  I was leery of 
silently dropping those L1_PUTXes and I'm glad to see the L2 no longer does 
that.


src/mem/protocol/MESI_CMP_directory-L2cache.sm


Please delete these commented out lines


- Brad


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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: Ruby: Fixes MESI CMP directory protocol

[Nilay Vaish]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/420/
---

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
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev