From: Byungchul Park <byungchul.park@lge.com>
To: Yuyang Du <yuyang.du@intel.com>
Cc: mingo@kernel.org, peterz@infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched: sync with the cfs_rq when changing sched class
Date: Thu, 13 Aug 2015 16:19:04 +0900 [thread overview]
Message-ID: <20150813071904.GN3956@byungchulpark-X58A-UD3R> (raw)
In-Reply-To: <20150812224145.GA2143@intel.com>
On Thu, Aug 13, 2015 at 06:41:45AM +0800, Yuyang Du wrote:
> On Thu, Aug 13, 2015 at 02:55:55PM +0900, byungchul.park@lge.com wrote:
> >
> > currently, a task load is synced with its cfs_rq, only when it
> > leaves from fair class. we also need to sync it with cfs_rq when
> > it returns back to fair class, too.
>
> Syncing it at the time it is switched to fair is not necessary, because
> since last_update_time if it has ever been updated, the load has become
> random, IOW, useless. So we simply leave it unattended, and let itself
> merge in the system.
hello,
i agree with you almost. it would have a meaningless value over time,
while it has a meaningful value as soon as it leaved that cfs_rq.
however, IMHO, nobody know when a task is switched between sched classes.
i think it would be better that we consider that load rather than leave
it unattended, even though, of course, in both of cases __update_load_avg()
will dacay and fix it over time.
shouldn't we consider it?
1. case returning back to fair class very soon:
original code cannot reflect the task load to cfs_rq, while
patched code can reflect the task load to cfs_rq.
2. case returning back to fair class after long:
original code adds 0 to cfs_rq and let __update_load_avg() fix it, while
patched code adds a meaningless value to cfs_rq and let
__update_load_avg() fix it, afterall these become same.
>
> >
> > #ifdef CONFIG_SMP
> > /* synchronize task with its prev cfs_rq */
> > - if (!queued)
> > - __update_load_avg(cfs_rq->avg.last_update_time, cpu_of(rq_of(cfs_rq)),
> > - &se->avg, se->on_rq * scale_load_down(se->load.weight),
> > - cfs_rq->curr == se, NULL);
> > -
> > - /* remove our load when we leave */
> > - cfs_rq->avg.load_avg = max_t(long, cfs_rq->avg.load_avg - se->avg.load_avg, 0);
> > - cfs_rq->avg.load_sum = max_t(s64, cfs_rq->avg.load_sum - se->avg.load_sum, 0);
> > - cfs_rq->avg.util_avg = max_t(long, cfs_rq->avg.util_avg - se->avg.util_avg, 0);
> > - cfs_rq->avg.util_sum = max_t(s32, cfs_rq->avg.util_sum - se->avg.util_sum, 0);
> > + detach_entity_load_avg(cfs_rq, se);
> > #endif
>
> You changed the logic.
yes, i changed it. but i think that calling __update_load_avg() is not
a problem even in case of "queued == 1". so i didn't think that change
seriously.
wrong? :(
thanks,
byungchul
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
next prev parent reply other threads:[~2015-08-13 7:19 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-13 5:55 [PATCH] sched: sync with the cfs_rq when changing sched class byungchul.park
2015-08-12 22:41 ` Yuyang Du
2015-08-13 7:19 ` Byungchul Park [this message]
2015-08-13 2:30 ` Yuyang Du
2015-08-13 7:46 ` Peter Zijlstra
2015-08-13 8:21 ` Byungchul Park
2015-08-13 2:15 ` Yuyang Du
2015-08-13 10:56 ` Byungchul Park
2015-08-13 15:22 ` Peter Zijlstra
2015-08-13 23:20 ` Yuyang Du
2015-08-14 8:46 ` Peter Zijlstra
2015-08-15 6:52 ` Byungchul Park
2015-08-15 7:16 ` Byungchul Park
2015-08-13 8:42 ` Byungchul Park
2015-08-14 12:59 ` T. Zhou
2015-08-15 4:24 ` Byungchul Park
2015-08-15 12:34 ` T. Zhou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150813071904.GN3956@byungchulpark-X58A-UD3R \
--to=byungchul.park@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=yuyang.du@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).