> On Oct. 18, 2015, 6:49 p.m., Steve Reinhardt wrote: > > Can you elaborate on the ordering issue? Couldn't it be solved just by > > making sure the appropriate latencies are used? > > > > Also, I know it's outside the scope of this patch, but 'force_order' is a > > really ambiguous name, and I originally thought it had the opposite effect > > that it does. Basically packets in the queue can be timestamp ordered or > > insertion-sequence ordered, and 'order' doesn't disambiguate between those. > > Andreas Hansson wrote: > I don't mind people using latencies to "ensure" this re-ordering never > happens, but I find that incredibly brittle. As stated in the patch > description, the issue arises when a snoop response is forwarded through a > cache, and scheduled to be sent, but before the response is sent, a new > request comes in, and the request-response latency is lower than the time > when the snoop response is going out. > > The stricted checks in the snoop filter catch the violation. > > I am happy to change the name, but I'd really like the functionality to > be all there first.
Thanks. I was thinking that if there were simple checks that could be added at config time, e.g., to make sure that the response latency is >= the snoop forward latency, then maybe this wouldn't be necessary. Even if so, I'm sure it would be good to have this code to check that messages aren't getting reordered. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3152/#review7394 ----------------------------------------------------------- On Oct. 19, 2015, 7:20 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3152/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2015, 7:20 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11175:7efa6f630cd8 > --------------------------- > mem: Enforce insertion order on the cache response path > > This patch enforces insertion order transmission of packets on the > response path in the cache. Note that the logic to enforce order is > already present in the packet queue, this patch simply turns it on for > queues in the response path. > > Without this patch, there are corner cases where a request-response is > faster than a response-response forwarded through the cache. This > violation of queuing order causes problems in the snoop filter leaving > it with inaccurate information. This causes assert failures in the > snoop filter later on. > > A follow on patch relaxes the order enforcement in the packet queue to > limit the performance impact. > > > Diffs > ----- > > src/mem/cache/cache.cc 3a4d1b5cd05c > src/mem/qport.hh 3a4d1b5cd05c > > Diff: http://reviews.gem5.org/r/3152/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
