----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3316/#review7999 -----------------------------------------------------------
So if you look at http://grok.gem5.org/search?q=handleLockedSnoop&defs=&refs=&path=&hist=&type=&project=gem5 there are multiple places where TheISA::handleLockedSnoop() is called from the various CPU models. Are we consistent in that this is only ever called on writes or invalidates? Is there a problem with it being called unconditionally, as it apparently is in most places? Or to put it another way, your problem with AtomicSimpleCPU was the 'if (pkt->isInvalidate())' check; an alternative solution would be just to get rid of that check entirely rather than adding pkt->isWrite() to the condition. And regardless of which way the solution goes, we should be sure it's applied consistently at all of the TheISA::handleLockedSnoop() call sites. (And maybe even document that somewhere!) - Steve Reinhardt On Feb. 9, 2016, 4:31 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3316/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2016, 4:31 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > cpu: Fix LLSC atomic CPU wakeup > > Writes to locked memory addresses (LLSC) did not wake up the locking > CPU. This can lead to deadlocks on multi-core runs. In AtomicSimpleCPU, > recvAtomicSnoop was checking if the incoming packet was an invalidation > (isInvalidate) and only then handled a locked snoop. But, writes are > seen instead of invalidates when running without caches (fast-forward > configurations). As as simple fix, now handleLockedSnoop is also called > even if the incoming snoop packet are from writes. > > Tests, Ran regression on all ISAs, observed acceptable change in 1: > opt/quick/fs/10.linux-boot/arm/linux/realview-simple-timing-dual > > > Diffs > ----- > > src/cpu/minor/lsq.cc c2146e4e20cd30c35b7ff4f4cd15c953663cdc4d > src/cpu/simple/atomic.cc c2146e4e20cd30c35b7ff4f4cd15c953663cdc4d > src/cpu/simple/timing.cc c2146e4e20cd30c35b7ff4f4cd15c953663cdc4d > > Diff: http://reviews.gem5.org/r/3316/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
