Re: [Xen-devel] [PATCH v3 4/9] mm: Scrub memory from idle loop

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index 90e2b1f..a5f62b5 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -118,7 +118,8 @@ static void idle_loop(void)
>>      {
>>          if ( cpu_is_offline(smp_processor_id()) )
>>              play_dead();
>> -        (*pm_idle)();
>> +        if ( !scrub_free_pages() )
>> +            (*pm_idle)();
>>          do_tasklet();
>>
> This means that, if we got here to run a tasklet (as in, if the idle
> vCPU has been forced into execution, because there were a vCPU context
> tasklet wanting to run), we will (potentially) do some scrubbing first.
>
> Is this on purpose, and, in any case, ideal? vCPU context tasklets are
> not terribly common, but I still don't think it is (ideal).
>
> Not sure how to address this, though. What (the variants of) pm_idle()
> uses for deciding whether or not to actually go to sleep is
> cpu_is_haltable(), which checks per_cpu(tasklet_work_to_do, cpu):
>
> /*
>  * Used by idle loop to decide whether there is work to do:
>  *  (1) Run softirqs; or (2) Play dead; or (3) Run tasklets.
>  */
> #define cpu_is_haltable(cpu)                    \
>     (!softirq_pending(cpu) &&                   \
>      cpu_online(cpu) &&                         \
>      !per_cpu(tasklet_work_to_do, cpu))
>
> Pulling it out/adding a call to it (cpu_is_haltable()) is ugly, and
> probably not what we want (e.g., it's always called with IRQs disabled,
> while they're on here).
>
> Maybe we can test tasklet_work_to_do, before calling scrub_free_pages()
> (also ugly, IMO).
> Or, if scrub_free_pages() is, and always will be, called only from
> here, within the idle loop, test tasklet_work_to_do inside, similarly
> to what it does already for pending softirqs...

We can move do_tasklet() above scrub_free_pages(). And new tasklet after
that would result in a softirq being set so we'd do an early exit from
scrub_free_pages().

OTOH since, as you say, we only get to idle loop() if no tasklet is
pending (cpu_is_haltable() test) then would even that be needed?


-boris

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel