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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2019 12:42, Jürgen Groß wrote:
> On 30.09.19 12:39, 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.
> 
> It was there in the original code, too:
> 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/schedule.c;h=fd587622f4c3ee13d57334f90b1eab4b17031c0b;hb=refs/heads/staging-4.12#l1536

Oh, indeed - I must have been blind: It's also there in context above,
among the code being replaced.

Jan

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