* [PATCH 5/5] processor: jiffies-based bm_check, bugfixes
@ 2004-12-23 14:08 Dominik Brodowski
[not found] ` <20041223140849.GE7973-X3ehHDuj6sIIGcDfoQAp7BvVK+yQ3ZXh@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Dominik Brodowski @ 2004-12-23 14:08 UTC (permalink / raw)
To: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
len.brown-ral2JQCrhuEAvxtiuMwx3w
Make the bm_activity depend on "jiffies", instead of numbers
of the check being called. This means bus mastering activity
is assumed if bm_check isn't called; and multiple calls during
one jiffy will be |='ed.
Also, two fixups where promotion and demotion were mixed up.
Signed-off-by: Dominik Brodowski <linux-JhLEnvuH02M@public.gmane.org>
---
drivers/acpi/processor_idle.c | 19 ++++++++++++++++---
include/acpi/processor.h | 1 +
2 files changed, 17 insertions(+), 3 deletions(-)
Index: linux-2.6.10-rc3+bk-acpi/drivers/acpi/processor_idle.c
===================================================================
--- linux-2.6.10-rc3+bk-acpi.orig/drivers/acpi/processor_idle.c 2004-12-23 14:25:32.385908579 +0100
+++ linux-2.6.10-rc3+bk-acpi/drivers/acpi/processor_idle.c 2004-12-23 14:39:58.360081349 +0100
@@ -193,8 +193,18 @@
*/
if (pr->flags.bm_check) {
u32 bm_status = 0;
+ unsigned long diff = jiffies - pr->power.bm_check_timestamp;
- pr->power.bm_activity <<= 1;
+ if (diff > 32)
+ diff = 32;
+
+ while (diff) {
+ /* if we didn't get called, assume there was busmaster activity */
+ diff--;
+ if (diff)
+ pr->power.bm_activity |= 0x1;
+ pr->power.bm_activity <<= 1;
+ }
acpi_get_register(ACPI_BITREG_BUS_MASTER_STATUS,
&bm_status, ACPI_MTX_DO_NOT_LOCK);
@@ -213,6 +223,9 @@
|| (inb_p(errata.piix4.bmisx + 0x0A) & 0x01))
pr->power.bm_activity++;
}
+
+ pr->power.bm_check_timestamp = jiffies;
+
/*
* Apply bus mastering demotion policy. Automatically demote
* to avoid a faulty transition. Note that the processor
@@ -439,13 +452,13 @@
if (higher) {
cx->promotion.state = higher;
- cx->demotion.threshold.ticks = cx->latency_ticks;
+ cx->promotion.threshold.ticks = cx->latency_ticks;
if (cx->type >= ACPI_STATE_C2)
cx->promotion.threshold.count = 4;
else
cx->promotion.threshold.count = 10;
if (higher->type == ACPI_STATE_C3)
- cx->demotion.threshold.bm = 0x0F;
+ cx->promotion.threshold.bm = 0x0F;
}
higher = cx;
Index: linux-2.6.10-rc3+bk-acpi/include/acpi/processor.h
===================================================================
--- linux-2.6.10-rc3+bk-acpi.orig/include/acpi/processor.h 2004-12-23 13:21:12.000000000 +0100
+++ linux-2.6.10-rc3+bk-acpi/include/acpi/processor.h 2004-12-23 14:41:47.558845116 +0100
@@ -54,6 +54,7 @@
struct acpi_processor_power {
struct acpi_processor_cx *state;
+ unsigned long bm_check_timestamp;
u32 default_state;
u32 bm_activity;
int count;
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/5] processor: jiffies-based bm_check, bugfixes
[not found] ` <20041223140849.GE7973-X3ehHDuj6sIIGcDfoQAp7BvVK+yQ3ZXh@public.gmane.org>
@ 2004-12-23 20:11 ` Len Brown
2005-01-21 4:08 ` [PATCH 5/5] processor: jiffies-based bm_check Len Brown
0 siblings, 1 reply; 5+ messages in thread
From: Len Brown @ 2004-12-23 20:11 UTC (permalink / raw)
To: Dominik Brodowski; +Cc: ACPI Developers
Applied the promotion/demotion bugfix part.
Didn't apply the jiffies part yet.
thanks,
-Len
On Thu, 2004-12-23 at 09:08, Dominik Brodowski wrote:
> Make the bm_activity depend on "jiffies", instead of numbers
> of the check being called. This means bus mastering activity
> is assumed if bm_check isn't called; and multiple calls during
> one jiffy will be |='ed.
>
> Also, two fixups where promotion and demotion were mixed up.
>
> Signed-off-by: Dominik Brodowski <linux-JhLEnvuH02M@public.gmane.org>
> ---
>
> drivers/acpi/processor_idle.c | 19 ++++++++++++++++---
> include/acpi/processor.h | 1 +
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.10-rc3+bk-acpi/drivers/acpi/processor_idle.c
> ===================================================================
> --- linux-2.6.10-rc3+bk-acpi.orig/drivers/acpi/processor_idle.c
> 2004-12-23 14:25:32.385908579 +0100
> +++ linux-2.6.10-rc3+bk-acpi/drivers/acpi/processor_idle.c
> 2004-12-23 14:39:58.360081349 +0100
> @@ -193,8 +193,18 @@
> */
> if (pr->flags.bm_check) {
> u32 bm_status = 0;
> + unsigned long diff = jiffies -
> pr->power.bm_check_timestamp;
>
> - pr->power.bm_activity <<= 1;
> + if (diff > 32)
> + diff = 32;
> +
> + while (diff) {
> + /* if we didn't get called, assume there was
> busmaster activity */
> + diff--;
> + if (diff)
> + pr->power.bm_activity |= 0x1;
> + pr->power.bm_activity <<= 1;
> + }
>
> acpi_get_register(ACPI_BITREG_BUS_MASTER_STATUS,
> &bm_status, ACPI_MTX_DO_NOT_LOCK);
> @@ -213,6 +223,9 @@
> || (inb_p(errata.piix4.bmisx + 0x0A) &
> 0x01))
> pr->power.bm_activity++;
> }
> +
> + pr->power.bm_check_timestamp = jiffies;
> +
> /*
> * Apply bus mastering demotion policy. Automatically
> demote
> * to avoid a faulty transition. Note that the
> processor
> @@ -439,13 +452,13 @@
>
> if (higher) {
> cx->promotion.state = higher;
> - cx->demotion.threshold.ticks =
> cx->latency_ticks;
> + cx->promotion.threshold.ticks =
> cx->latency_ticks;
> if (cx->type >= ACPI_STATE_C2)
> cx->promotion.threshold.count = 4;
> else
> cx->promotion.threshold.count = 10;
> if (higher->type == ACPI_STATE_C3)
> - cx->demotion.threshold.bm = 0x0F;
> + cx->promotion.threshold.bm = 0x0F;
> }
>
> higher = cx;
> Index: linux-2.6.10-rc3+bk-acpi/include/acpi/processor.h
> ===================================================================
> --- linux-2.6.10-rc3+bk-acpi.orig/include/acpi/processor.h
> 2004-12-23 13:21:12.000000000 +0100
> +++ linux-2.6.10-rc3+bk-acpi/include/acpi/processor.h 2004-12-23
> 14:41:47.558845116 +0100
> @@ -54,6 +54,7 @@
>
> struct acpi_processor_power {
> struct acpi_processor_cx *state;
> + unsigned long bm_check_timestamp;
> u32 default_state;
> u32 bm_activity;
> int count;
>
>
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/5] processor: jiffies-based bm_check
2004-12-23 20:11 ` Len Brown
@ 2005-01-21 4:08 ` Len Brown
2005-01-21 16:23 ` Dominik Brodowski
0 siblings, 1 reply; 5+ messages in thread
From: Len Brown @ 2005-01-21 4:08 UTC (permalink / raw)
To: Dominik Brodowski; +Cc: ACPI Developers, Venkatesh Pallipadi, Robert Moore
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
On Thu, 2004-12-23 at 15:11, Len Brown wrote:
> Didn't apply the jiffies part yet.
I've applied it now. (attached as jiffy.patch)
However, "threshold.bm = 0xF" was written when
HZ = 100 yielding 40ms, but this
becomes 4ms for HZ = 1000.
While I don't know what the optimal value is for
the bus master history threshold, it seemed like
a good idea for it to default to about 40ms per
the original code until we know better.
So I've also applied the attached bm_history.patch
to default this to 0xFFFFFFFF for HZ = 1000 (32ms)
and it is tunable via /sys/module/processor/parameters/bm_history
what do you think?
thanks,
-Len
[-- Attachment #2: bm_history.patch --]
[-- Type: text/plain, Size: 1905 bytes --]
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/01/20 22:43:52-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
# [ACPI] Add a module parameter to allow tuning how much bus-master activity
# we remember when entering C3 -- /sys/module/processor/parameters/bm_history
# Default varies with HZ -- 40ms for 25 - 800 HZ, 32ms for 1000 HZ.
#
# drivers/acpi/processor_idle.c
# 2005/01/20 22:43:44-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org +11 -2
# /sys/module/processor/parameters/bm_history
#
diff -Nru a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
--- a/drivers/acpi/processor_idle.c 2005-01-20 22:44:28 -05:00
+++ b/drivers/acpi/processor_idle.c 2005-01-20 22:44:28 -05:00
@@ -60,6 +60,15 @@
static unsigned int nocst = 0;
module_param(nocst, uint, 0000);
+/*
+ * bm_history -- bit-mask with a bit per jiffy of bus-master activity
+ * 1000 HZ: 0xFFFFFFFF: 32 jiffies = 32ms
+ * 800 HZ: 0xFFFFFFFF: 32 jiffies = 40ms
+ * 100 HZ: 0x0000000F: 4 jiffies = 40ms
+ * reduce history for more aggressive entry into C3
+ */
+static unsigned int bm_history = (HZ >= 800 ? 0xFFFFFFFF : ((1U << (HZ / 25)) - 1));
+module_param(bm_history, uint, 0644);
/* --------------------------------------------------------------------------
Power Management
-------------------------------------------------------------------------- */
@@ -438,7 +447,7 @@
cx->demotion.threshold.ticks = cx->latency_ticks;
cx->demotion.threshold.count = 1;
if (cx->type == ACPI_STATE_C3)
- cx->demotion.threshold.bm = 0x0F;
+ cx->demotion.threshold.bm = bm_history;
}
lower = cx;
@@ -458,7 +467,7 @@
else
cx->promotion.threshold.count = 10;
if (higher->type == ACPI_STATE_C3)
- cx->promotion.threshold.bm = 0x0F;
+ cx->promotion.threshold.bm = bm_history;
}
higher = cx;
[-- Attachment #3: jiffy.patch --]
[-- Type: text/plain, Size: 2215 bytes --]
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/01/20 21:52:27-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
# [ACPI] Make the bm_activity depend on "jiffies", instead of numbers
# of the check being called. This means bus mastering activity
# is assumed if bm_check isn't called; and multiple calls during
# one jiffy will be |='ed.
#
# Signed-off-by: Dominik Brodowski <linux-JhLEnvuH02M@public.gmane.org>
# Signed-off-by: Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
#
# include/acpi/processor.h
# 2005/01/20 21:52:11-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org +1 -0
# jiffies-based bus-master activity history
#
# drivers/acpi/processor_idle.c
# 2005/01/20 21:52:11-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org +14 -1
# jiffies-based bus-master activity history
#
diff -Nru a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
--- a/drivers/acpi/processor_idle.c 2005-01-20 22:45:19 -05:00
+++ b/drivers/acpi/processor_idle.c 2005-01-20 22:45:19 -05:00
@@ -193,8 +193,18 @@
*/
if (pr->flags.bm_check) {
u32 bm_status = 0;
+ unsigned long diff = jiffies - pr->power.bm_check_timestamp;
- pr->power.bm_activity <<= 1;
+ if (diff > 32)
+ diff = 32;
+
+ while (diff) {
+ /* if we didn't get called, assume there was busmaster activity */
+ diff--;
+ if (diff)
+ pr->power.bm_activity |= 0x1;
+ pr->power.bm_activity <<= 1;
+ }
acpi_get_register(ACPI_BITREG_BUS_MASTER_STATUS,
&bm_status, ACPI_MTX_DO_NOT_LOCK);
@@ -213,6 +223,9 @@
|| (inb_p(errata.piix4.bmisx + 0x0A) & 0x01))
pr->power.bm_activity++;
}
+
+ pr->power.bm_check_timestamp = jiffies;
+
/*
* Apply bus mastering demotion policy. Automatically demote
* to avoid a faulty transition. Note that the processor
diff -Nru a/include/acpi/processor.h b/include/acpi/processor.h
--- a/include/acpi/processor.h 2005-01-20 22:45:19 -05:00
+++ b/include/acpi/processor.h 2005-01-20 22:45:19 -05:00
@@ -54,6 +54,7 @@
struct acpi_processor_power {
struct acpi_processor_cx *state;
+ unsigned long bm_check_timestamp;
u32 default_state;
u32 bm_activity;
int count;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/5] processor: jiffies-based bm_check
2005-01-21 4:08 ` [PATCH 5/5] processor: jiffies-based bm_check Len Brown
@ 2005-01-21 16:23 ` Dominik Brodowski
0 siblings, 0 replies; 5+ messages in thread
From: Dominik Brodowski @ 2005-01-21 16:23 UTC (permalink / raw)
To: Len Brown; +Cc: ACPI Developers, Venkatesh Pallipadi, Robert Moore
On Thu, Jan 20, 2005 at 11:08:06PM -0500, Len Brown wrote:
> On Thu, 2004-12-23 at 15:11, Len Brown wrote:
>
> > Didn't apply the jiffies part yet.
>
> I've applied it now. (attached as jiffy.patch)
>
> However, "threshold.bm = 0xF" was written when
> HZ = 100 yielding 40ms, but this
> becomes 4ms for HZ = 1000.
>
> While I don't know what the optimal value is for
> the bus master history threshold, it seemed like
> a good idea for it to default to about 40ms per
> the original code until we know better.
>
> So I've also applied the attached bm_history.patch
> to default this to 0xFFFFFFFF for HZ = 1000 (32ms)
> and it is tunable via /sys/module/processor/parameters/bm_history
>
> what do you think?
This sounds sensible to do. Note that the "diff" measured in idle() will
become problematic once "tickless" systems are implemented -- we'll sleep
for multiple jiffies then, and there was _no_ bm activiy in between. We need
to sort that out somehow...
Dominik
-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 5/5] processor: jiffies-based bm_check
@ 2005-01-21 21:03 Pallipadi, Venkatesh
0 siblings, 0 replies; 5+ messages in thread
From: Pallipadi, Venkatesh @ 2005-01-21 21:03 UTC (permalink / raw)
To: Dominik Brodowski, Brown, Len; +Cc: ACPI Developers, Moore, Robert
>-----Original Message-----
>From: Dominik Brodowski [mailto:linux-X3ehHDuj6sIIGcDfoQAp7BvVK+yQ3ZXh@public.gmane.org]
>Sent: Friday, January 21, 2005 8:24 AM
>To: Brown, Len
>Cc: ACPI Developers; Pallipadi, Venkatesh; Moore, Robert
>Subject: Re: [PATCH 5/5] processor: jiffies-based bm_check
>
>On Thu, Jan 20, 2005 at 11:08:06PM -0500, Len Brown wrote:
>> On Thu, 2004-12-23 at 15:11, Len Brown wrote:
>>
>> > Didn't apply the jiffies part yet.
>>
>> I've applied it now. (attached as jiffy.patch)
>>
>> However, "threshold.bm = 0xF" was written when
>> HZ = 100 yielding 40ms, but this
>> becomes 4ms for HZ = 1000.
>>
>> While I don't know what the optimal value is for
>> the bus master history threshold, it seemed like
>> a good idea for it to default to about 40ms per
>> the original code until we know better.
>>
>> So I've also applied the attached bm_history.patch
>> to default this to 0xFFFFFFFF for HZ = 1000 (32ms)
>> and it is tunable via /sys/module/processor/parameters/bm_history
>>
>> what do you think?
>
>This sounds sensible to do. Note that the "diff" measured in
>idle() will
>become problematic once "tickless" systems are implemented --
>we'll sleep
>for multiple jiffies then, and there was _no_ bm activiy in
>between. We need
>to sort that out somehow...
>
> Dominik
How about using gettimeofday here and using that in place of jiffies?
-Venki
-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2005-01-21 21:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-12-23 14:08 [PATCH 5/5] processor: jiffies-based bm_check, bugfixes Dominik Brodowski
[not found] ` <20041223140849.GE7973-X3ehHDuj6sIIGcDfoQAp7BvVK+yQ3ZXh@public.gmane.org>
2004-12-23 20:11 ` Len Brown
2005-01-21 4:08 ` [PATCH 5/5] processor: jiffies-based bm_check Len Brown
2005-01-21 16:23 ` Dominik Brodowski
-- strict thread matches above, loose matches on Subject: below --
2005-01-21 21:03 Pallipadi, Venkatesh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox