> On 2011-07-13 10:09:49, Steve Reinhardt wrote: > > I assume this means you got past the bugs you were seeing earlier... cool. > > > > I think you might still have a memory leak; if a new packet is allocated in > > getBusPacket() but the snoop succeeds then that new packet is not getting > > deleted. Since the only thing you're doing with pkt is an assert, the easy > > solution is to move the added code above getBusPacket() and get rid of the > > assert. > > > > (This paragraph is a totally optional side note.) The version I had with > > the snoop happening after getBusPacket() used pkt as the template for > > snoop_pkt instead of tgt_pkt; the main difference is that pkt will be a > > ReadReq rather than HardPFReq (if you look at getBusPacket() it never > > returns a HardPFReq). Though this will break your assert for HardPFResp in > > the other part of the code, it will make the request that the upper-level > > caches see on the snoop consistent with the one they would see if they were > > in a different part of the memory hierarchy. That probably shouldn't and > > probably doesn't matter, but I can see it as a potential point of confusion > > later. Another way to make things consistent would be to make > > getBusPacket() issue HardPFReq when appropriate. In practice though > > there's no difference in how caches would respond to a normal ReadReq vs. a > > HardPFReq though. > >
Yea... it appears to work. I think you're correct. I moved the code around to be closer to your code, but I added that. I'm happy to move the code back to where I originally had it. Seems like you don't have any feelings either way on the HardPF vs Read. I'm a bit indifferent, so I'm ok with leaving it for the moment if that is fine with you. I like the fact that if there isn't a forwardresponserecord it should be a prefetch and not something else. I think I would have to get rid of that check if i took option one above, although perhaps it would assert somewhere in handleResponse if we got a response we weren't expecting. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/787/#review1415 ----------------------------------------------------------- On 2011-07-13 09:01:53, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/787/ > ----------------------------------------------------------- > > (Updated 2011-07-13 09:01:53) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Mem: Fix issue with prefetches originating at non-L1 caches getting stale data > > Prefetch requests issued from the L2 or below wouldn't check if valid data is > present higher in the system. If a prefetch into the L2 occured at the same > time as writeback from a higher-level cache the dirty data could be replaced > in by unmodified data in memory. > > > Diffs > ----- > > src/mem/cache/cache_impl.hh 82ff928182c5 > > Diff: http://reviews.m5sim.org/r/787/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
