> On Sept. 22, 2015, 2:03 p.m., Andreas Hansson wrote: > > src/mem/cache/cache.cc, line 1771 > > <http://reviews.gem5.org/r/3047/diff/1/?file=49087#file49087line1771> > > > > The trick here is in the difference between snoopDelay and headerDelay. > > > > Snoop delay is where we do the max on each level. Header delay is just > > used to eventually pay for it and pass it "downwards". > > Nilay Vaish wrote: > Here is what I think you are doing. Suppose we have three levels of > caching. We have three private > controllers (A1,B1,C1) at the first level, another three private > controllers at the second level > (A2,B2,C2) and one single shared controller (S) at the third level. When > S receives a packet from > any of the second level controllers, it forwards those packets to the > other second level > controllers so that they can invalidate or writeback blocks if they need > to. > > Now suppose A2 sent a packet to S which S forwarded to B2 and C2. Then, > B2 and C2 would update the snoop delay > of packet with their lookup latency and the header delay with the total > delay of snoops sent to B1 and C1. > Since all this was happening in parallel (which is why I think you take > max), should not the header delay > be also maximum of the delays of snoops sent to B1 and C1. > > I might be completely incorrect here. > > Andreas Hansson wrote: > Who needs Sudokus... :-). I think you're right in that I've messed this > up. > > The packet from A2 will be forwarded to B2 and C2 by the crossbar in > front of S. First we forward it to B2, and B2 updates the snoopDelay with a > max expression, and then takes a copy (snoopPkt), then forwards that to B1. > On returning from the upwards snoop, the headerDelay from the copied snoopPkt > is added to the original packets headerDelay (and not snoopDelay). I think > this is indeed wrong, we should add it to the snoopDelay rather, and > preferably do so before the max expression.
I broke out the upward snoop delay calculation, and handleSnoop now returns the delay. That way we can sum up the upwards delay (which in turn may be the result of a max), and the current cache, before taking the max and passing it downstream. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3047/#review7243 ----------------------------------------------------------- On Sept. 23, 2015, 5:38 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3047/ > ----------------------------------------------------------- > > (Updated Sept. 23, 2015, 5:38 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11126:1c89381f6775 > --------------------------- > mem: Make the coherent crossbar account for timing snoops > > This patch introduces the concept of a snoop latency. Given the > requirement to snoop and forward packets in zero time (due to the > coherency mechanism), the latency is accounted for later. > > On a snoop, we establish the latency, and later add it to the header > delay of the packet. To allow multiple caches to contribute to the > snoop latency, we use a separate variable in the packet, and then take > the maximum before adding it to the header delay. > > > Diffs > ----- > > src/mem/coherent_xbar.cc a8980f67b3fc > src/mem/packet.hh a8980f67b3fc > src/mem/cache/cache.hh a8980f67b3fc > src/mem/cache/cache.cc a8980f67b3fc > > Diff: http://reviews.gem5.org/r/3047/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
