On 19.08.2021 12:20, Juergen Gross wrote: > On 05.07.21 17:13, Jan Beulich wrote: >> In send_memory_live() the precise value the dirty_count struct field >> gets initialized to doesn't matter much (apart from the triggering of >> the log message in send_dirty_pages(), see below), but it is important >> that it not be zero on the first iteration (or else send_dirty_pages() >> won't get called at all). Saturate the initializer value at the maximum >> value the field can hold. >> >> While there also initialize struct precopy_stats' respective field to a >> more sane value: We don't really know how many dirty pages there are at >> that point. >> >> In suspend_and_send_dirty() and verify_frames() the local variables >> don't need initializing at all, as they're only an output from the >> hypercall which gets invoked first thing. >> >> In send_checkpoint_dirty_pfn_list() the local variable can be dropped >> altogether: It's optional to xc_logdirty_control() and not used anywhere >> else. >> >> Note that in case the clipping actually takes effect, the "Bitmap >> contained more entries than expected..." log message will trigger. This >> being just an informational message, I don't think this is overly >> concerning. > > Is there any real reason why the width of the stats fields can't be > expanded to avoid clipping? This could avoid the need to set the > initial value to -1, which seems one of the more controversial changes.
While not impossible, it comes with a price tag, as we'd either need to decouple xc_shadow_op_stats_t from struct xen_domctl_shadow_op_stats or alter the underlying domctl. Neither of which looked either appealing or necessary to me; instead I'm still struggling with Andrew's comments, yet I didn't receive any clarification of further explanation. Plus I continue to think that statistics output like this shouldn't be assumed to be precise anyway, and for practical purposes I don't think it really matters how large the counts actually are once they've moved into the billions. Jan
