* Peter Zijlstra <a.p.zijlstra@xxxxxxxxx> wrote:
> Matt suggested, whereupon Linus sayeth:
>
> "This looks like a good patch. Please make it so"
>
> Compile and boot tested on x86_64 only.
I wanted to wait with his for .32, but Linus applied it yesterday so
lets hope it's all fine on all architectures ;-)
There's one new test failure on x86 caused by this commit: a
scheduling-while-atomic assert during early ACPI init - fixed by the
patch below. It triggers on two separate test-systems.
Btw., that ACPI_PREEMPTION_POINT() wrapper is both superfluous (we
should not wrap something as obvious as a cond_resched()) and shows
signs of problems:
#define ACPI_PREEMPTION_POINT() \
do { \
if (!irqs_disabled()) \
cond_resched(); \
} while (0)
that should have been 1) an inline function 2) should not check for
whether irqs are off. If we need to check for irqs-off like this
then this is a sign that either the code flow is too unbalanced
(mixing different things into the same function), or that the
preemption point has been applied at a too low level.
So a followup cleanup would likely be in order, especially that this
was the last (and only) call site of ACPI_PREEMPTION_POINT(). I'd
suggest to remove it.
( I'm not sure what prompted the addition of this rescheduling point
- if there was a strong reason for it then we should probably add
back the preemption point to the place that is causing the
latency. )
Ingo
-------------------->
>From 2b2b96115287177600c0158c95e87c5ab44f8379 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@xxxxxxx>
Date: Sat, 11 Jul 2009 13:15:04 +0200
Subject: [PATCH] ACPICA: Do not schedule during early init
-tip testing found a test failure on x86, a scheduling-while-atomic
bug during early ACPI init:
[ 0.048083] ACPI: Core revision 20090521
[ 0.051854] BUG: scheduling while atomic: swapper/0/0x10000002
[ 0.052076] no locks held by swapper/0.
[ 0.053000] Pid: 0, comm: swapper Not tainted 2.6.31-rc2-tip-01240-gd51b6ef-dirty #69901
[ 0.053997] Call Trace:
[ 0.055006] [<ffffffff8107afe2>] __schedule_bug+0x80/0x9c
[ 0.057001] [<ffffffff81f1160a>] schedule+0xe6/0x5de
[ 0.058000] [<ffffffff8151531f>] ? acpi_os_release_object+0x1c/0x34
[ 0.059002] [<ffffffff8154d16c>] ? acpi_ut_exit+0x40/0x5c
[ 0.060020] [<ffffffff8107e1dc>] __cond_resched+0x37/0x69
[ 0.060998] [<ffffffff81f11d55>] _cond_resched+0x37/0x56
[ 0.061999] [<ffffffff81546748>] acpi_ps_complete_op+0x412/0x457
[ 0.062998] [<ffffffff8154588d>] ? acpi_ps_next_parse_state+0x14a/0x16b
[ 0.064019] [<ffffffff81546e87>] acpi_ps_parse_loop+0x47e/0x513
[ 0.064998] [<ffffffff81545418>] acpi_ps_parse_aml+0x177/0x4a2
[ 0.065998] [<ffffffff8154a83b>] ? acpi_get_table_by_index+0x138/0x159
[ 0.066998] [<ffffffff81543c0d>] acpi_ns_one_complete_parse+0x1d1/0x220
[ 0.068019] [<ffffffff81543cd7>] acpi_ns_parse_table+0x7b/0x148
[ 0.068997] [<ffffffff8154b229>] ? acpi_tb_allocate_owner_id+0x95/0xb4
[ 0.069997] [<ffffffff8153e62d>] acpi_ns_load_table+0xc5/0x1b8
[ 0.070998] [<ffffffff8288a140>] ? early_idt_handler+0x0/0x71
[ 0.072018] [<ffffffff8154aa03>] acpi_tb_load_namespace+0x9d/0x1ba
[ 0.072996] [<ffffffff8288a140>] ? early_idt_handler+0x0/0x71
[ 0.073996] [<ffffffff8154ab5a>] acpi_load_tables+0x3a/0x99
[ 0.074997] [<ffffffff828bef2e>] acpi_early_init+0x71/0x11a
[ 0.076997] [<ffffffff8288a140>] ? early_idt_handler+0x0/0x71
[ 0.077996] [<ffffffff8288b0eb>] start_kernel+0x39f/0x3c1
[ 0.078995] [<ffffffff8288a140>] ? early_idt_handler+0x0/0x71
[ 0.080016] [<ffffffff8288a140>] ? early_idt_handler+0x0/0x71
[ 0.080995] [<ffffffff8288a2ce>] x86_64_start_reservations+0xb9/0xd4
[ 0.081995] [<ffffffff8288a000>] ? __init_begin+0x0/0x140
[ 0.082995] [<ffffffff8288a3ed>] x86_64_start_kernel+0x104/0x127
[ 0.088045] BUG: scheduling while atomic: swapper/0/0x10000002
[ 0.089999] no locks held by swapper/0.
[ 0.090993] Pid: 0, comm: swapper Not tainted 2.6.31-rc2-tip-01240-gd51b6ef-dirty #69901
The problem is that drivers/acpi/acpica/psloop.c has this line:
ACPI_PREEMPTION_POINT();
Which does not mix well with this early init stage - we have
preemption disabled and the init task has not started up yet, so we
really should not schedule.
Remove this explicit preemption point.
Cc: Len Brown <len.brown@xxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Ingo Molnar <mingo@xxxxxxx>
---
drivers/acpi/acpica/psloop.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/acpica/psloop.c b/drivers/acpi/acpica/psloop.c
index c5f6ce1..28cd67a 100644
--- a/drivers/acpi/acpica/psloop.c
+++ b/drivers/acpi/acpica/psloop.c
@@ -720,8 +720,6 @@ acpi_ps_complete_op(struct acpi_walk_state *walk_state,
*op = NULL;
}
- ACPI_PREEMPTION_POINT();
-
return_ACPI_STATUS(AE_OK);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
