Great. Let us know if there are still any remaining issues. We’ve got some additional cache fixes that should be on RB before the end of the week.
Andreas From: Steve Reinhardt <ste...@gmail.com<mailto:ste...@gmail.com>> Date: Monday, 2 March 2015 16:47 To: Andreas Hansson <andreas.hans...@arm.com<mailto:andreas.hans...@arm.com>> Cc: Stephan Diestelhorst <stephan.diestelho...@gmail.com<mailto:stephan.diestelho...@gmail.com>>, Default <gem5-dev@gem5.org<mailto:gem5-dev@gem5.org>> Subject: Re: Review Request 2636: mem: fix prefetcher bug regarding write buffer hits Done. Thanks for the reminder. Steve On Mon, Mar 2, 2015 at 2:59 AM, Andreas Hansson <andreas.hans...@arm.com<mailto:andreas.hans...@arm.com>> wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2636/ On February 10th, 2015, 5:37 p.m. UTC, Stephan Diestelhorst wrote: I have had a similar impulse, when inspecting this code. However, the prefetch hitting a write-back in an upper cache is actually already handled in Cache<TagStore>::getTimingPacket(): // Check if the prefetch hit a writeback in an upper cache // and if so we will eventually get a HardPFResp from // above if (snoop_pkt.memInhibitAsserted()) { // If we are getting a non-shared response it is dirty bool pending_dirty_resp = !snoop_pkt.sharedAsserted(); markInService(mshr, pending_dirty_resp); DPRINTF(Cache, "Upward snoop of prefetch for addr" " %#x (%s) hit\n", tgt_pkt->getAddr(), tgt_pkt->isSecure()? "s": "ns"); return NULL; } We are currently testing a patch that shuffles this thing upwards; we have more detail tomorrow. In either way, if we want to go with this patch, it should address this seemingly dead bit of logic, as well. My suggestion is to give this an additional day, and in the meantime test as Andreas has suggested. On February 10th, 2015, 7:44 p.m. UTC, Steve Reinhardt wrote: Good catch, I hadn't noticed that before. I believe what's happening in George's case is that there are multiple L1s above a shared L2, and one of them has/had the block in O state (dirty shared) and is in the process of writing it back, while others have the block in the shared state. So the one with the block in writeback goes to write back the data (in accordance with the code snippet you have there), but meanwhile other caches have squashed the prefetch, so your code never gets executed, because this code immediately above it gets executed instead: // Check to see if the prefetch was squashed by an upper // cache (to prevent us from grabbing the line) or if a // writeback arrived between the time the prefetch was // placed in the MSHRs and when it was selected to be sent. if (snoop_pkt.prefetchSquashed() || blk != NULL) { DPRINTF(Cache, "Prefetch squashed by cache. " "Deallocating mshr target %#x.\n", mshr->addr); [...] return NULL; } Interestingly the point where the prefetches get squashed is here, in handleSnoop(): // Invalidate any prefetch's from below that would strip write permissions // MemCmd::HardPFReq is only observed by upstream caches. After missing // above and in it's own cache, a new MemCmd::ReadReq is created that // downstream caches observe. if (pkt->cmd == MemCmd::HardPFReq) { DPRINTF(Cache, "Squashing prefetch from lower cache %#x\n", pkt->getAddr()); pkt->setPrefetchSquashed(); return; } where despite the language about "strip write permissions", the prefetch appears to get squashed as long as the block is valid, regardless of the state. On February 10th, 2015, 7:48 p.m. UTC, Steve Reinhardt wrote: So if nothing else, my commit message describes the bug incorrectly... it's not just a matter of hitting in the write buffer, it's handling the case where it *both* hits in the write buffer of one upper-level cache, and also gets squashed because of a hit in another upper-level cache. The actual symptom we were seeing was that the response from the cache with the write-buffer copy was causing an assertion, since the receiving cache wasn't expecting a response because it had squashed the prefetch. On February 13th, 2015, 8:37 a.m. UTC, Andreas Hansson wrote: Here is the fix: http://reviews.gem5.org/r/2654/ The fix has been pushed. I believe this patch can be discarded. - Andreas On February 6th, 2015, 12:38 a.m. UTC, Steve Reinhardt wrote: Review request for Default. By Steve Reinhardt. Updated Feb. 6, 2015, 12:38 a.m. Repository: gem5 Description Changeset 10683:3147f3a868f7 --------------------------- mem: fix prefetcher bug regarding write buffer hits Prefetches are supposed to be squashed if the block is already present in a higher-level cache. We squash appropriately if the block is in a higher-level cache or MSHR, but did not properly handle the case where the block is in the write buffer. Thanks to George Michelogiannakis <mihe...@lbl.gov<mailto:mihe...@lbl.gov>> for help in tracking down and testing this fix. Diffs * src/mem/cache/cache_impl.hh (3d17366c0423a59478ae63d40c8feeea34df218a) View Diff<http://reviews.gem5.org/r/2636/diff/> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
