> On Nov. 19, 2014, 12:22 a.m., Cagdas Dirik wrote: > > Please ignore my last review. I made a mistake with my patches. > > > > In FS, X86 mode I was able to boot with python variant, checkpoint, run a > > short program. Then I was able to restore from checkpoint and run the same > > program again. And in systemc variant then I was able to restore from > > checkpoint and run the same program. > > > > The exit was not clean though. It failed with > > fatal: Missed an event at time 123366500 gem5: 5699875688000, SystemC: > > 5699875688000 @ tick 5699875688000 > > [eventLoop:sc_module.cc, line 192] > > > > But that may be due to m5_exit in the test program. > > > > Andrew Bardsley wrote: > Hmm. The problem seems to be that sometimes asynchronous events schedule > actions based on gem5 time in the asynchronous event handler. I'm adding > code to catch up time in that handler and also a bit more 'in_simulate' > checking and general event management to fix the problem. > > A patch will follow... > > Andrew Bardsley wrote: > Oh, in the meantime, can you put the line: > > getEventQueue(0)->setCurTick(systemc_time); > > Near the top of serviceAsyncEvent and see if that helps. > > Cagdas Dirik wrote: > Unfortunately that did not resolve the issue. Debugging it a bit more, > here is what I believe is going wrong: > > There is no sync point between gem5 curTick and sc_time_stamp(). > Therefore, all events in the system are gem5 events based on curTick. > Somewhere between SimControl::SimControl, > SimControl::before_end_of_elaboration, and SimControl::run(), gem5 sub-system > starts ticking before systemc starts SimControl::run(). And then we go into > wait statement to advance systemc time, but gem5 keeps ticking. No gem5 > module knows about systemc scheduler and the fact that they have to wait if > we are restoring from a checkpoint. So gem5 events gets scheduled just like > normal execution, and shortly after things fall apart because curTick, > sc_time_stamp, checkpoint time and event times are totally out of whack. > > Cagdas Dirik wrote: > I believe I have a solution in mind. Let me run it by you and may be you > can add it to your patch. > > 1. I would remove config_manager->instantiate() from the end of > SimControl::SimControl, because we don't want any gem5 parts start ticking > yet. > 2. Instead at the end of SimControl::SimControl() unserializeGlobals, > save pointer to checkpoint, and only calculate what the restore time would be. > 3. Don't anything in before_end_of_elaboration(), because we may need to > wait if checkpoint_restore is true. > 4. Have another SimControl method to finish instantiation, > initState/loadState, and startup. > 5. At the beginning of run(), continue with if (checkpoint_restore) > statement, wait if needed. Afterwards reset curTick to sc_time_stamp() and > then call method from step 4 which would setup everything else. Then gem5 > will start ticking and curTick will be inline with sc_time_stamp(). > > Andrew Bardsley wrote: > I didn't think it would fix everything, but I thought it would help. > Specific responses (excuse me if I over explain): > > > There is no sync point between gem5 curTick and sc_time_stamp() > > There is for 'normal' events, that's what the setCurTick in the eventLoop > is for. (I'm going to admit again that a code reorganisation to not use > SC_METHOD for eventLoop that I made just before submitting the original patch > is the source of a lot of these problems.) > > I've added a synchronisation in a new patch and also a check to see if > we're in simulate before acting on normal events. > > > Therefore, all events in the system are gem5 events based on curTick > > Certainly true and if you source an event from outside the gem5 event > loop you must make sure that the time of that submitted event is based on the > latest SystemC time and you *must* use EventQueue::wakeup to possibly wake up > the gem5 event handling. If your external action could cause some other > piece of gem5 code to submit an event to the event queue you must also, > instead of taking that action, enqueue an with gem5 to handle that action so > that the eventLoop is visited and time is synchronised between gem5 and > SystemC. > > Unfortunately, this example code doesn't have an example of that kind of > event requirement. > > > Somewhere between SimControl::SimControl, > SimControl::before_end_of_elaboration ... > > It is possible to get an asynchronous event to accidentally restart > normal event processing (Ooops, wasn't the case in my old code). I *think* > I've fixed that in the not-yet-submitted patch. > > My understanding of the current constructor, before_end... calls is that > the only events they cause are the asynch. events associated with async. I/O. > I'll check ... Nope, I'm wrong. Yes, startup will need to move to > SimControl::run. > > > Andrew Bardsley wrote: > To your solution: > > 1) It's not really possible to drop instantiate from the constructor as > that model of 'make all sc_modules inside the constructor of the enclosing > module' is pretty much required by SystemC (please correct me). > > 2) I'm doing that in before_end_of_elaboration as that seems an > appropriate place to do it in case you want to use anything from gem5, or > inspect anything in end_of_elaboration or asynchronously to gem5 startup in > the first simulation step. I will move startup, however. > > 3) I'd rather just move startup, I think before_... is an appropriate > place to put the state setup. > > 4) If I drop startup from before_..., before_... *could* be rolled into > the constructor (I think). I'll make SimControl::run directly run startup. > > 5) As 4 > > So essentially the only problem is that I'm calling startup too soon (in > addition to my other problems). > > Oh, I'm going to apply the 'new' patch as a revision to this one. Should > hopefully be posted today.
3) 4) 5). Gah! events posted in initState. Right, I'm going to follow your idea and just dump the init/load/startup into run. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2504/#review5482 ----------------------------------------------------------- On Nov. 17, 2014, 6:19 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2504/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2014, 6:19 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10555:d1e51dc6cf86 > --------------------------- > config: Fix to SystemC example's event handling > > This patch fixes checkpoint restore in the SystemC hosting example by handling > early PollEvent events correctly before any EventQueue events are posted. > > The SystemC event queue handler (SCEventQueue) reports an error if the event > loop is entered with no Events posted. It is possible for this to happen > after instantiate due to PollEvent events. This patch separates out > `external' events into a different handler in sc_module.cc to prevent the > error from occurring. > > > Diffs > ----- > > util/systemc/Makefile 1a9e235cab09 > util/systemc/main.cc 1a9e235cab09 > util/systemc/sc_module.hh 1a9e235cab09 > util/systemc/sc_module.cc 1a9e235cab09 > > Diff: http://reviews.gem5.org/r/2504/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
