From: Mike wolf <mjw@linux.vnet.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
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 11:06:36 -0600 [thread overview]
Message-ID: <4EDCFA1C.3050105@linux.vnet.ibm.com> (raw)
In-Reply-To: <1323102051.32012.28.camel@twins>
On 12/05/2011 10:20 AM, Peter Zijlstra wrote:
> 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 ;-)
yes will do
> 2) You failed to CC the people who wrote the feature you're having a
> problem with.
ok, I will add Glauber and others when the patch is respun/resubmitted
>> 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.
I will make sure to explain why you would want the patch functionality
better
when I resubmit.
>
>
prev parent reply other threads:[~2011-12-05 17:10 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
2011-12-05 17:06 ` Mike wolf [this message]
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=4EDCFA1C.3050105@linux.vnet.ibm.com \
--to=mjw@linux.vnet.ibm.com \
--cc=dave@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--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.