Re: [PATCH] dom0/pvh: fix processing softirqs during memory map population

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> --- 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 <jbeulich@xxxxxxxx>

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)?

Furthermore, how about taking the opportunity and deleting the mis-
named and single-use-only MAP_MAX_ITER define?

Jan