All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arjan van de Ven <arjan@linux.intel.com>
To: Kevin Hilman <khilman@ti.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	linux-pm@lists.linux-foundation.org, Len Brown <lenb@kernel.org>,
	Nicole Chalhoub <n-chalhoub@ti.com>, Vincent Bour <v-bour@ti.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] nohz: delay going tickless under CPU load to favor deeper C states
Date: Thu, 07 Apr 2011 12:57:06 -0700	[thread overview]
Message-ID: <4D9E1712.5090600@linux.intel.com> (raw)
In-Reply-To: <1302200311-24263-1-git-send-email-khilman@ti.com>

On 4/7/2011 11:18 AM, Kevin Hilman wrote:
> From: Nicole Chalhoub<n-chalhoub@ti.com>
>
> While there is CPU load, continue the periodic tick in order to give
> CPUidle another opportunity to pick a deeper C-state instead of
> spending potentially long i


so I don't really like this patch. It's actually a pretty bad hack (I'm 
sure it'll work somewhat)
[and I mean that in the most positive sense of the word ;-) ]

what we really need instead, and this is inside cpuidle, is the option 
to set a timer when we enter the non-deepest C state,
so that if that timer fires we then reevaluate.
The duration of that timer will be dependent on the C state (so should 
come from the C state structure of the state we pick).

For the most shallow one this will be a relatively short time, but for 
the deepest-but-one this might be a lot longer time.


your patch abuses a completely different, unrelated timer for this, with 
a pretty much unspecified frequency, that also has other side effects 
that we probably don't want.


it shouldn't be hard to do the right thing instead and make it a 
separate timer with a per C state timeout.

(and I would say a default timeout of 10x the break even time that we 
already have in the structure)

WARNING: multiple messages have this Message-ID (diff)
From: arjan@linux.intel.com (Arjan van de Ven)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] nohz: delay going tickless under CPU load to favor deeper C states
Date: Thu, 07 Apr 2011 12:57:06 -0700	[thread overview]
Message-ID: <4D9E1712.5090600@linux.intel.com> (raw)
In-Reply-To: <1302200311-24263-1-git-send-email-khilman@ti.com>

On 4/7/2011 11:18 AM, Kevin Hilman wrote:
> From: Nicole Chalhoub<n-chalhoub@ti.com>
>
> While there is CPU load, continue the periodic tick in order to give
> CPUidle another opportunity to pick a deeper C-state instead of
> spending potentially long i


so I don't really like this patch. It's actually a pretty bad hack (I'm 
sure it'll work somewhat)
[and I mean that in the most positive sense of the word ;-) ]

what we really need instead, and this is inside cpuidle, is the option 
to set a timer when we enter the non-deepest C state,
so that if that timer fires we then reevaluate.
The duration of that timer will be dependent on the C state (so should 
come from the C state structure of the state we pick).

For the most shallow one this will be a relatively short time, but for 
the deepest-but-one this might be a lot longer time.


your patch abuses a completely different, unrelated timer for this, with 
a pretty much unspecified frequency, that also has other side effects 
that we probably don't want.


it shouldn't be hard to do the right thing instead and make it a 
separate timer with a per C state timeout.

(and I would say a default timeout of 10x the break even time that we 
already have in the structure)

  parent reply	other threads:[~2011-04-07 19:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-07 18:18 [PATCH] nohz: delay going tickless under CPU load to favor deeper C states Kevin Hilman
2011-04-07 18:18 ` Kevin Hilman
2011-04-07 19:57 ` Arjan van de Ven
2011-04-07 19:57 ` Arjan van de Ven [this message]
2011-04-07 19:57   ` Arjan van de Ven
2011-04-07 22:38   ` Kevin Hilman
2011-04-07 22:38     ` Kevin Hilman
2011-04-07 22:38   ` Kevin Hilman
  -- strict thread matches above, loose matches on Subject: below --
2011-04-07 18:18 Kevin Hilman

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=4D9E1712.5090600@linux.intel.com \
    --to=arjan@linux.intel.com \
    --cc=khilman@ti.com \
    --cc=lenb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=n-chalhoub@ti.com \
    --cc=tglx@linutronix.de \
    --cc=v-bour@ti.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.