From: Peter Zijlstra <peterz@infradead.org>
To: Mike wolf <mjw@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org,
paulmck <paulmck@linux.vnet.ibm.com>,
Dave Hansen <dave@linux.vnet.ibm.com>,
Ingo Molnar <mingo@elte.hu>, Paul Turner <pjt@google.com>
Subject: Re: [PATCH] Do not include throttled time as steal time
Date: Mon, 05 Dec 2011 17:20:51 +0100 [thread overview]
Message-ID: <1323102051.32012.28.camel@twins> (raw)
In-Reply-To: <4EDCDE54.3030300@linux.vnet.ibm.com>
On Mon, 2011-12-05 at 09:08 -0600, Mike wolf wrote:
Hi Mike, couple of problems with this:
1) You failed to CC the appropriate maintainers for the piece of code
you're trying to have changed. When in doubt see the MAINTAINERS
file ;-)
2) You failed to CC the people who wrote the feature you're having a
problem with.
> When the linux kernel is running as the guest OS and is configured
> for bandwidth control and steal time reporting, it can be confusing
> to users to see the throttled time show up in the steal time stats.
> The user will think they are not getting the cpu resources they have
> been configured.
Supposedly this is a BAD (tm) thing :-)
> Signed-off-by: Mike Wolf <mjw@linux.vnet.ibm.com>
> ---
> kernel/sched_fair.c | 4 ++--
> kernel/sched_stats.h | 7 ++++++-
> 2 files changed, 8 insertions(+), 3 deletions(-)
3) You blink you loose, those files don't exist anymore. Patches are
best provided against the development tree of the particular subsystem
you're working against.
In this particular case tip/master is your target.
> static inline void sched_info_depart(struct task_struct *t)
> {
> + struct task_group *tg = task_group(t);
> + struct cfs_rq *cfs_rq;
> unsigned long long delta = task_rq(t)->clock -
> t->sched_info.last_arrival;
>
> + cfs_rq = tg->cfs_rq[smp_processor_id()];
> rq_sched_info_depart(task_rq(t), delta);
>
> - if (t->state == TASK_RUNNING)
> +
> + if (t->state == TASK_RUNNING && !throttled_hierarchy(cfs_rq))
> sched_info_queued(t);
> }
4) so there's a lot more steal time crap all over the scheduler, you
failed to explain why only this particular bit is important enough to
change.
next prev parent reply other threads:[~2011-12-05 16:21 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-05 15:08 [PATCH] Do not include throttled time as steal time Mike wolf
2011-12-05 16:20 ` Peter Zijlstra [this message]
2011-12-05 17:06 ` Mike wolf
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=1323102051.32012.28.camel@twins \
--to=peterz@infradead.org \
--cc=dave@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=mjw@linux.vnet.ibm.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=pjt@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.