linux-kernel - Re: [External] Re: [PATCH v3 2/2] sched/fair: optimize and simplify rq leaf_cfs_rq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <270c2315-7ba5-4bbb-ee5c-38cc8a57e628@bytedance.com>
Date:   Tue, 31 May 2022 21:55:23 +0800
From:   Chengming Zhou <zhouchengming@...edance.com>
To:     Vincent Guittot <vincent.guittot@...aro.org>
Cc:     mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, bristot@...hat.com, vschneid@...hat.com,
        odin@...d.al, linux-kernel@...r.kernel.org,
        duanxiongchun@...edance.com, songmuchun@...edance.com
Subject: Re: [External] Re: [PATCH v3 2/2] sched/fair: optimize and simplify
 rq leaf_cfs_rq_list

Hi,

On 2022/5/30 15:52, Vincent Guittot wrote:
> On Thu, 26 May 2022 at 12:40, Chengming Zhou
> <zhouchengming@...edance.com> wrote:
>>
[...]
>> @@ -11257,9 +11218,10 @@ static inline bool vruntime_normalized(struct task_struct *p)
>>   */
>>  static void propagate_entity_cfs_rq(struct sched_entity *se)
>>  {
>> -       struct cfs_rq *cfs_rq;
>> +       struct cfs_rq *cfs_rq = cfs_rq_of(se);
>>
>> -       list_add_leaf_cfs_rq(cfs_rq_of(se));
>> +       if (!throttled_hierarchy(cfs_rq))
>> +               list_add_leaf_cfs_rq(cfs_rq);
>>
>>         /* Start to propagate at parent */
>>         se = se->parent;
>> @@ -11268,7 +11230,8 @@ static void propagate_entity_cfs_rq(struct sched_entity *se)
>>                 cfs_rq = cfs_rq_of(se);
>>
> 
>  you can break  if the cfs is throttled because it's sched_entity has
> been dequeued. In this case we check if the cfs is throttled not if
> the hierarchy is throttled
> 
> +               if (cfs_rq_throttled(cfs_rq))
> +                       break;
> 

This propagate part still make me confused. :-)

I wonder if you think we should change like this:

static void propagate_entity_cfs_rq(struct sched_entity *se)
{
        struct cfs_rq *cfs_rq = cfs_rq_of(se);

        if (cfs_rq_throttled(cfs_rq))			--> break if cfs is throttled
                return;

        if (!throttled_hierarchy(cfs_rq))
                list_add_leaf_cfs_rq(cfs_rq);

        /* Start to propagate at parent */
        se = se->parent;

        for_each_sched_entity(se) {
                cfs_rq = cfs_rq_of(se);

                if (cfs_rq_throttled(cfs_rq))		--> break if cfs is throttled
                        break;

                update_load_avg(cfs_rq, se, UPDATE_TG); --> throttled cfs_rq->prop not updated
                if (!throttled_hierarchy(cfs_rq))
                        list_add_leaf_cfs_rq(cfs_rq);
        }
}



If I understand right, we should update_load_avg() until cfs_rq_throttled(),
including that throttled cfs_rq? So we can go on propagating when unthrottle.

Maybe like this?

static void propagate_entity_cfs_rq(struct sched_entity *se)
{
        struct cfs_rq *cfs_rq = cfs_rq_of(se);

        if (cfs_rq_throttled(cfs_rq))			--> break if cfs is throttled
                return;

        if (!throttled_hierarchy(cfs_rq))
                list_add_leaf_cfs_rq(cfs_rq);

        /* Start to propagate at parent */
        se = se->parent;

        for_each_sched_entity(se) {
                cfs_rq = cfs_rq_of(se);

                update_load_avg(cfs_rq, se, UPDATE_TG);	--> update throttled cfs_rq->prop

                if (cfs_rq_throttled(cfs_rq))		--> break if cfs is throttled
                        break;

                if (!throttled_hierarchy(cfs_rq))
                        list_add_leaf_cfs_rq(cfs_rq);
        }
}


Thanks!


>>                 update_load_avg(cfs_rq, se, UPDATE_TG);
>> -               list_add_leaf_cfs_rq(cfs_rq);
>> +               if (!throttled_hierarchy(cfs_rq))
>> +                       list_add_leaf_cfs_rq(cfs_rq);
>>         }
>>  }
>>  #else
>> --
>> 2.36.1
>>