> On June 18, 2012, 5:32 p.m., Nilay Vaish wrote: > > src/mem/bus.cc, line 165 > > <http://reviews.gem5.org/r/1264/diff/1/?file=27317#file27317line165> > > > > I strongly suggest that either > > a. the name of function isOccupied() be changed, or > > b. the code be organised in a different fashion. > > > > As is the normal practice, a function named isOccupied() should not > > change anything at all.
The name is the same for "historic" reasons, but I'm happy to change it. Any suggestions? updateAvailable? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1264/#review2955 ----------------------------------------------------------- On June 8, 2012, 10:58 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1264/ > ----------------------------------------------------------- > > (Updated June 8, 2012, 10:58 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9071:e6145aa18228 > --------------------------- > Bus: Replace tickNextIdle and inRetry with a state variable > > This patch adds a state enum and member variable in the bus, tracking > the bus state, thus eliminating the need for tickNextIdle and inRetry, > and fixing an issue that allowed the bus to be occupied by multiple > packets at once (hopefully it also makes it easier to understand the > code). > > The bus, in its current form, uses tickNextIdle and inRetry to keep > track of the state of the bus. However, it only updates tickNextIdle > _after_ forwarding a packet using sendTiming, and the result is that > the bus is still seen as idle, and a module that receives the packet > and starts transmitting new packets in zero time will still see the > bus as idle (and this is done by a number of DMA devices). The issue > can also be seen in isOccupied where the bus calls reschedule on an > event instead of schedule. > > This patch addresses the problem by marking the bus as _not_ idle > already by the time we conclude that the bus is not occupied and we > will deal with the packet. > > As a result of not allowing multiple packets to occupy the bus, some > regressions have slight changes in their statistics. A separate patch > updates these accordingly. > > Further ahead, a follow-on patch will introduce a separate state > variable for request/responses/snoop responses, and thus implement a > split request/response bus with separate flow control for the > different message types (even further ahead it will introduce a > multi-layer bus). > > > Diffs > ----- > > src/mem/bus.hh 35ac3a6f8ee0 > src/mem/bus.cc 35ac3a6f8ee0 > src/mem/coherent_bus.cc 35ac3a6f8ee0 > src/mem/noncoherent_bus.cc 35ac3a6f8ee0 > > Diff: http://reviews.gem5.org/r/1264/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) with a few exception > that exhibit stat differences, e.g. > 10.linux-boot/alpha/linux/tsunami-simple-timing with a magnitude of <0.4%, > fs/10.linux-boot/arm/linux/realview-simple-timing with <0.01%, and > 10.linux-boot/x86/linux/pc-simple-timing with a max magnitude of +38%(!) > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
