[PATCH v5 8/8] drm/i915/gvt: let timeslice usage control vGPU scheduling
Gao, Ping A
ping.a.gao at intel.com
Wed Mar 29 06:18:35 UTC 2017
On 2017/3/29 11:37, Tian, Kevin wrote:
>> From: Ping Gao
>> Sent: Tuesday, March 28, 2017 2:48 PM
>>
>> The timeslice usage will determine vGPU whether has chance to schedule or
>> not at every vGPU switch checkpoint.
> "The timeslice usage determines whether a vGPU still has chance to be
> scheduled. Move to take this new scheme in scheduling policy."
>
>> Signed-off-by: Ping Gao <ping.a.gao at intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/sched_policy.c | 48
>> ++++++++++++++++++++++++++-------
>> 1 file changed, 38 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c
>> b/drivers/gpu/drm/i915/gvt/sched_policy.c
>> index c7fd8b8..04f874b 100644
>> --- a/drivers/gpu/drm/i915/gvt/sched_policy.c
>> +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c
>> @@ -190,7 +190,10 @@ static void try_to_schedule_next_vgpu(struct
>> intel_gvt *gvt)
>> wake_up(&scheduler->waitq[i]);
>> }
>>
>> -static struct intel_vgpu *find_busy_vgpu(struct list_head *head,
>> +#define vgpu_has_ts_limit(vgpu_data) \
>> + ((vgpu_data)->allocated_ts <
>> ms_to_ktime(GVT_TS_BALANCE_PERIOD_MS))
>> +
> I don't quite understand the meaning of this macro. either add
> some comment or change to a better name.
>
> From my understanding, looks you want to catch a vGPU which
> is not scheduled in previous timeslice (in that cast the accumulated
> slice would exceed 100ms, and then you want some special
> handling?)
Actually this macro is introduce for cap and weight control mix
together, to avoid confuse I will move it at next version.
>> +static struct intel_vgpu *find_vgpu_timeslice_left(struct list_head
>> +*head,
>> struct gvt_sched_data *sched_data) {
>> struct vgpu_sched_data *vgpu_data;
>> @@ -206,8 +209,16 @@ static struct intel_vgpu *find_busy_vgpu(struct
>> list_head *head,
>> if (!vgpu_has_pending_workload(vgpu_data->vgpu))
>> continue;
>>
>> - vgpu = vgpu_data->vgpu;
>> - break;
>> + /* Return the vGPU if there is no time slice limitation */
>> + if (!vgpu_has_ts_limit(vgpu_data)) {
>> + vgpu = vgpu_data->vgpu;
>> + break;
>> +
>> + /* Return the vGPU only if it has time slice left */
>> + } else if (vgpu_data->left_ts > 0) {
>> + vgpu = vgpu_data->vgpu;
>> + break;
>> + }
> A bit confused here. Isn't it that you can always use left_ts to judge
> regardless of whether allocated_ts exceeds limitation? Suppose
> left_ts should be always >0 when allocated_ts>100ms...
allocated_ts is a fixed value at every stage base on the weight, left_ts is dynamic to track the actual usage, left_ts > allocated_ts or left_ts < 0 both valid, if left_ts > allocated_ts that means the vGPU's timeslice was not used because of vGPU is not very active, left_ts < 0 means timeslice run out.
>
>> }
>>
>> return vgpu;
>> @@ -243,6 +254,9 @@ static struct intel_vgpu
>> *find_longest_unsched_vgpu(struct list_head *lru_vgpu_h
>> return NULL;
>> }
>>
>> +#define is_not_idle_vgpu(vgpu) \
>> + ((vgpu) && ((vgpu) != (vgpu)->gvt->idle_vgpu))
> I would change to the other way:
>
> #define is_idle_vgpu(vgpu)
> ((vgpu) && ((vgpu) == (vgpu)->gvt->idle_vgpu))
>
> Then you can use !is_idle_vgpu(vgpu) for original purpose. :-)
there are three types for handle, vgpu==NULL, vgpu==idle_vgpu,
vgpu==active_vgpu; NULL and idle has the same method to handle for this
case.
>
>> +
>> static struct list_head *get_sched_head(struct gvt_sched_data *sched_data)
>> {
>> struct intel_gvt *gvt = sched_data->gvt; @@ -251,8 +265,12 @@
>> static struct list_head *get_sched_head(struct gvt_sched_data *sched_data)
>> struct list_head *head;
>>
>> if (scheduler->current_vgpu) {
>> - cur_vgpu_data = scheduler->current_vgpu->sched_data;
>> - head = &cur_vgpu_data->list;
>> + if (is_not_idle_vgpu(scheduler->current_vgpu)) {
>> + cur_vgpu_data = scheduler->current_vgpu-
>>> sched_data;
>> + head = &cur_vgpu_data->list;
>> + } else {
>> + head = &sched_data->runq_head;
>> + }
>> } else {
>> struct vgpu_sched_data *lru_vgpu_data;
>>
>> @@ -272,6 +290,7 @@ static struct list_head *get_sched_head(struct
>> gvt_sched_data *sched_data)
>>
>> static struct intel_vgpu *pickup_next_vgpu(struct gvt_sched_data
>> *sched_data) {
>> + struct intel_gvt *gvt = sched_data->gvt;
>> struct vgpu_sched_data *next_vgpu_data;
>> struct intel_vgpu *next_vgpu;
>> struct list_head *head;
>> @@ -297,8 +316,14 @@ static struct intel_vgpu *pickup_next_vgpu(struct
>> gvt_sched_data *sched_data)
>> list_del_init(&next_vgpu_data->list);
>> list_add(&next_vgpu_data->list, head);
>> } else {
>> - /* Choose the vGPU which has pending workload */
>> - next_vgpu = find_busy_vgpu(head, sched_data);
>> + /* Choose the vGPU which has timeslice left */
>> + next_vgpu = find_vgpu_timeslice_left(head, sched_data);
> find_eligible_vgpu?
>
>> +
>> + /* Switch to idle vGPU when all next candidates are
>> + * empty.
>> + */
>> + if (!next_vgpu)
>> + next_vgpu = gvt->idle_vgpu;
>> }
>>
>> return next_vgpu;
>> @@ -329,9 +354,12 @@ static void tbs_sched_func(struct gvt_sched_data
>> *sched_data)
>> gvt_dbg_sched("pick next vgpu %d\n", vgpu->id);
>>
>> /* Move the last used vGPU to the tail of lru_list */
>> - vgpu_data = vgpu->sched_data;
>> - list_del_init(&vgpu_data->lru_list);
>> - list_add_tail(&vgpu_data->lru_list, &sched_data-
>>> lru_vgpu_head);
>> + if (vgpu != vgpu->gvt->idle_vgpu) {
>> + vgpu_data = vgpu->sched_data;
>> + list_del_init(&vgpu_data->lru_list);
>> + list_add_tail(&vgpu_data->lru_list,
>> + &sched_data->lru_vgpu_head);
>> + }
>> }
>> out:
>> if (scheduler->next_vgpu) {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
More information about the intel-gvt-dev
mailing list