On Sat, Dec 21, 2024 at 01:02:01PM +1000, Nicholas Piggin wrote: > On Sat Dec 21, 2024 at 2:31 AM AEST, Peter Xu wrote: > > On Fri, Dec 20, 2024 at 08:42:03PM +1000, Nicholas Piggin wrote: > > > Migration reads CLOCK_HOST when not holding the replay_mutex, which > > > asserts when recording a trace. These are not guest visible so should > > > be CLOCK_REALTIME like other statistics in MigrationState, which do > > > not require the replay_mutex. > > > > Irrelevant of the change, should we document such lock implications in > > timer.h? > > I guess the intention was to try to avoid caller caring too much > about replay internals, so I'm not sure if that will help or > hinder understanding :(
CLOCK_HOST should be the wall clock in QEMU, IIUC. If any QEMU caller tries to read host wall clock requires some mutex to be held.. then I don't see how we can avoid mentioning it. It's indeed weird if we need to take a feature specific mutex just to read the wallclock.. But maybe I misread the context somewhere.. > > I think the big rule is something like "if it affects guest state, > then you must use HOST or VIRTUAL*, if it does not affect guest state HOST clock logically shouldn't be relevant to guest-state? > then you must use REALTIME". record-replay code then takes care of > replay mutex locking. > > Does get a little fuzzy around edges in code that is somewhat > aware of record-replay though, like migration/snapshots. Said that, I agree with the change itself - any measurement may not want to involve NTP at all... which HOST / gtod will, but REALTIME won't. However this patch doesn't seem to be for that purpose.. So I'd like to double check. Thanks, -- Peter Xu
