On Mon, Feb 07, 2022 at 09:21:11AM +0100, Jan Beulich wrote: > On 05.02.2022 11:18, Roger Pau Monne wrote: > > Make sure softirqs are processed at least once for every call to > > pvh_populate_memory_range. It's likely that none of the calls to > > pvh_populate_memory_range will perform 64 iterations, in which case > > softirqs won't be processed for the whole duration of the p2m > > population. > > > > In order to force softirqs to be processed at least once for every > > pvh_populate_memory_range call move the increasing of 'i' to be done > > after evaluation, so on the first loop iteration softirqs will > > unconditionally be processed. > > Nit: The change still guarantees one invocation only for every > iteration not encountering an error. That's fine from a functional > pov, but doesn't fully match what you claim.
OK, will fix on next iteration. > > --- a/xen/arch/x86/hvm/dom0_build.c > > +++ b/xen/arch/x86/hvm/dom0_build.c > > @@ -186,7 +186,7 @@ static int __init pvh_populate_memory_range(struct > > domain *d, > > start += 1UL << order; > > nr_pages -= 1UL << order; > > order_stats[order]++; > > - if ( (++i % MAP_MAX_ITER) == 0 ) > > + if ( (i++ % MAP_MAX_ITER) == 0 ) > > process_pending_softirqs(); > > } > > This way is perhaps easiest, so > > Acked-by: Jan Beulich <[email protected]> > > but I'd like you to consider to avoid doing this on the first > iteration. How about keeping the code here as is, but instead > insert an invocation in the sole caller (and there unconditionally > at the end of every successful loop iteration)? In fact I was thinking that we should call process_pending_softirqs on every iteration: the calls to guest_physmap_add_page could use a 1G page order, so if not using sync-pt (at least until your series for IOMMU super-page support is committed) mapping a whole 1G page using 4K chunks on the IOMMU page-tables could be quite time consuming, and hence we would likely need to process softirqs on every iteration. > > Furthermore, how about taking the opportunity and deleting the mis- > named and single-use-only MAP_MAX_ITER define? Right, let me know your opinion about the comment above. Thanks, Roger.
