Re: [Xen-devel] [PATCH v5 01/19] xen/sched: add code to sync scheduling of all vcpus of a sched unit

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 9/30/19 11:39 AM, Jan Beulich wrote:
> On 30.09.2019 12:38, Andrew Cooper wrote:
>> On 30/09/2019 11:36, Jan Beulich wrote:
>>> On 30.09.2019 07:21, Juergen Gross wrote:
>>>> When switching sched units synchronize all vcpus of the new unit to be
>>>> scheduled at the same time.
>>>>
>>>> A variable sched_granularity is added which holds the number of vcpus
>>>> per schedule unit.
>>>>
>>>> As tasklets require to schedule the idle unit it is required to set the
>>>> tasklet_work_scheduled parameter of do_schedule() to true if any cpu
>>>> covered by the current schedule() call has any pending tasklet work.
>>>>
>>>> For joining other vcpus of the schedule unit we need to add a new
>>>> softirq SCHED_SLAVE_SOFTIRQ in order to have a way to initiate a
>>>> context switch without calling the generic schedule() function
>>>> selecting the vcpu to switch to, as we already know which vcpu we
>>>> want to run. This has the other advantage not to loose any other
>>>> concurrent SCHEDULE_SOFTIRQ events.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>>>> Reviewed-by: Dario Faggioli <dfaggioli@xxxxxxxx>
>>> x86 and applicable common code parts
>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> However, ...
>>>
>>>> +static void sched_context_switch(struct vcpu *vprev, struct vcpu *vnext,
>>>> +                                 s_time_t now)
>>>> +{
>>>> +    if ( unlikely(vprev == vnext) )
>>>>      {
>>>> -        pcpu_schedule_unlock_irq(lock, cpu);
>>>>          TRACE_4D(TRC_SCHED_SWITCH_INFCONT,
>>>> -                 next->domain->domain_id, next->unit_id,
>>>> -                 now - prev->state_entry_time,
>>>> -                 prev->next_time);
>>>> -        trace_continue_running(next->vcpu_list);
>>>> -        return continue_running(prev->vcpu_list);
>>>> +                 vnext->domain->domain_id, vnext->sched_unit->unit_id,
>>>> +                 now - vprev->runstate.state_entry_time,
>>>> +                 vprev->sched_unit->next_time);
>>>> +        sched_context_switched(vprev, vnext);
>>>> +        trace_continue_running(vnext);
>>>> +        return continue_running(vprev);
>>>>      }
>>> ... I don't recall if there weren't compiler (clang?) versions not
>>> allowing (or at least warning about) use of this extension.
>>
>> Which extension?
> 
> "return" with an expression of "void" type.

I think that must be a mistake.  In this instance there isn't really
even a "syntactic sugar"* reason to use it.

 -George

* Syntactic sugar being to do something like:

if ( blah )
   return foo();

rather than

if ( blah ) {
    foo();
    return;
}

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel