lkml.org 
[lkml]   [2012]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 02/18] sched: fix find_idlest_group mess logical
    On 12/11/2012 01:50 PM, Preeti U Murthy wrote:
    > Hi Alex,
    > On 12/11/2012 10:59 AM, Alex Shi wrote:
    >> On 12/11/2012 01:08 PM, Preeti U Murthy wrote:
    >>> Hi Alex,
    >>>
    >>> On 12/10/2012 01:52 PM, Alex Shi wrote:
    >>>> There is 4 situations in the function:
    >>>> 1, no task allowed group;
    >>>> so min_load = ULONG_MAX, this_load = 0, idlest = NULL
    >>>> 2, only local group task allowed;
    >>>> so min_load = ULONG_MAX, this_load assigned, idlest = NULL
    >>>> 3, only non-local task group allowed;
    >>>> so min_load assigned, this_load = 0, idlest != NULL
    >>>> 4, local group + another group are task allowed.
    >>>> so min_load assigned, this_load assigned, idlest != NULL
    >>>>
    >>>> Current logical will return NULL in first 3 kinds of scenarios.
    >>>> And still return NULL, if idlest group is heavier then the
    >>>> local group in the 4th situation.
    >>>>
    >>>> Actually, I thought groups in situation 2,3 are also eligible to host
    >>>> the task. And in 4th situation, agree to bias toward local group.
    >>>> So, has this patch.
    >>>>
    >>>> Signed-off-by: Alex Shi <alex.shi@intel.com>
    >>>> ---
    >>>> kernel/sched/fair.c | 12 +++++++++---
    >>>> 1 files changed, 9 insertions(+), 3 deletions(-)
    >>>>
    >>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
    >>>> index df99456..b40bc2b 100644
    >>>> --- a/kernel/sched/fair.c
    >>>> +++ b/kernel/sched/fair.c
    >>>> @@ -2953,6 +2953,7 @@ find_idlest_group(struct sched_domain *sd, struct task_struct *p,
    >>>> int this_cpu, int load_idx)
    >>>> {
    >>>> struct sched_group *idlest = NULL, *group = sd->groups;
    >>>> + struct sched_group *this_group = NULL;
    >>>> unsigned long min_load = ULONG_MAX, this_load = 0;
    >>>> int imbalance = 100 + (sd->imbalance_pct-100)/2;
    >>>>
    >>>> @@ -2987,14 +2988,19 @@ find_idlest_group(struct sched_domain *sd, struct task_struct *p,
    >>>>
    >>>> if (local_group) {
    >>>> this_load = avg_load;
    >>>> - } else if (avg_load < min_load) {
    >>>> + this_group = group;
    >>>> + }
    >>>> + if (avg_load < min_load) {
    >>>> min_load = avg_load;
    >>>> idlest = group;
    >>>> }
    >>>> } while (group = group->next, group != sd->groups);
    >>>>
    >>>> - if (!idlest || 100*this_load < imbalance*min_load)
    >>>> - return NULL;
    >>>> + if (this_group && idlest != this_group)
    >>>> + /* Bias toward our group again */
    >>>> + if (100*this_load < imbalance*min_load)
    >>>> + idlest = this_group;
    >>>
    >>> If the idlest group is heavier than this_group(or to put it better if
    >>> the difference in the loads of the local group and idlest group is less
    >>> than a threshold,it means there is no point moving the load from the
    >>> local group) you return NULL,that immediately means this_group is chosen
    >>> as the candidate group for the task to run,one does not have to
    >>> explicitly return that.
    >>
    >> In situation 4, this_group is not NULL.
    >
    > True.The return value of find_idlest_group() indicates that there is no
    > other idle group other than the local group(the group to which cpu
    > belongs to). it does not indicate that there is no host group for the
    > task.If this is the case,select_task_rq_fair() falls back to the
    > group(sd->child) to which the cpu chosen in the previous iteration
    > belongs to,This is nothing but this_group in the current iteration.

    Sorry, I didn't get you here.
    >
    > Regards
    > Preeti U Murthy
    >


    --
    Thanks
    Alex


    \
     
     \ /
      Last update: 2012-12-11 13:01    [W:2.537 / U:0.288 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site