> On May 13, 2015, 9:36 p.m., Andreas Hansson wrote:
> > I do not understand the livelock scenario based on the description. Is the 
> > patch trying to allow multiple loads/stores per cycle, or is it fixing an 
> > issue?
> 
> Yasuko Eckert wrote:
>     This patch fixes a performance issue. It does not allow a prefetch 
> request to seize an MSHR slot before a dependent memory instruction gets a 
> chance to issue in the next cycle.
> 
> Andreas Hansson wrote:
>     I am still a bit hesitant since I fear this has performance implications 
> in quite a few places, and would potentially throw off any corrrelation work 
> already done. When is this a problem? What components are involved?
>     
>     If the goal is to ensure the CPU gets a chance before any prefetch "at 
> the same time" then I'd say we should deal with that not by fiddling with the 
> time, but rather have an event for the next cycle that picks one. Makes sense?
> 
> Yasuko Eckert wrote:
>     This patch does have performance implications and that is intentional. We 
> found this performance bug while correlating O3 to AMD's x86 hardware. 
>     
>     The problem is when there are back-to-back stores and a hardware 
> prefetcher has prefetches to issue. An older store A issued a write and is 
> waiting for an ACK from Ruby. In cycle T, Ruby schedules sending an ACk to 
> store A in cycle T+1 (i.e., curTick()+1). In the same cycle T, the prefetcher 
> issues a prefetch request and successfully grabs an MSHR entry. The MSHR 
> becomes full at this point. In cycle T+1, store A receives the ACK. The 
> younger store B is ready to issue in the same cycle but cannot because the 
> MSHR is full. The last available entry was taken by the earlier prefetch that 
> was issued in cycle T. Hence, back-to-back execution of stores fails.
>     
>     To fix this problem, this patch sends an ACK to store A in cycle T, not 
> T+1. This ensures that store B is executed back-to-back after A, even in the 
> presence of prefetches.

To me this appears to be problem with the policy followed by the memory system
in deciding who should get preference, the cpu or the prefetcher.  What if, in
the new scenario, the prefetch is issued at T-1 instead?  store B would still 
not
be able to go ahead.  I think the solution is either have separate MSHRs for 
demand
and prefetch accesses, or if there is demand access waiting, block MSHR in 
advance.


- Nilay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2784/#review6250
-----------------------------------------------------------


On May 11, 2015, 10:19 p.m., Tony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2784/
> -----------------------------------------------------------
> 
> (Updated May 11, 2015, 10:19 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 10841:b015addd7b9d
> ---------------------------
> mem: MSHR livelock bug fix
> 
> This patch was created by Bihn Pham during his internship at AMD.
> 
> This bug fix prevents a case in which a prefetcher uses up all remaining MSHR
> entries before demand requests get a chance to, causing a livelock.
> This happens because events scheduled at curTick() + 1 are evaluated in the
> next cycle, not in the current cycle.
> 
> A specific case that caused this livelock situation is the following:
> There are back-to-back stores and the second store cannot be sent to the cache
> until the first store receives an ACK. When the ACK is scheduled at curTick() 
> +
> 1, meaning that the ACK is to be sent in the next cycle, there is an open MSHR
> entry in the current cycle. A prefetcher grabs the entry by issuing a prefetch
> request in the current cycle before the second store gets a chance to issue in
> the next cycle. The second store stalls because the MSHR is already full by
> that time.
> 
> 
> Diffs
> -----
> 
>   src/mem/packet_queue.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec 
> 
> Diff: http://reviews.gem5.org/r/2784/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to