> On Feb. 10, 2015, 9:37 a.m., 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.
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.
- Steve
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2636/#review5885
-----------------------------------------------------------
On Feb. 5, 2015, 4:38 p.m., Steve Reinhardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2636/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2015, 4:38 p.m.)
>
>
> Review request for Default.
>
>
> 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 <[email protected]> for
> help in tracking down and testing this fix.
>
>
> Diffs
> -----
>
> src/mem/cache/cache_impl.hh 3d17366c0423a59478ae63d40c8feeea34df218a
>
> Diff: http://reviews.gem5.org/r/2636/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Steve Reinhardt
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
