RE: [PATCH] ACPI: Fix wrong atomicity check in preemption point

From: Moore, Robert
Date: Mon Aug 09 2010 - 11:56:35 EST


I'll be happy to include this in the aclinux.h file if the day ever comes when it is stable.


>-----Original Message-----
>From: Frederic Weisbecker [mailto:fweisbec@xxxxxxxxx]
>Sent: Friday, August 06, 2010 8:39 PM
>To: Brown, Len
>Cc: LKML; Frederic Weisbecker; Moore, Robert
>Subject: [PATCH] ACPI: Fix wrong atomicity check in preemption point
>
>The acpi preemption point checks the atomicity of the context
>using in_atomic_preempt_off(). This helper must be used only
>to check the atomicity before a prior call to preempt_disable(),
>which is not what we want here.
>
>What we want is to simply check if we are in an atomic section.
>This helper is actually only used by the scheduler for particular
>needs and shouldn't be used outside.
>
>The check made here is then always wrong. We will schedule only if
>preemption has been disabled once. It never has been a problem
>during the boot because premption is disabled and moreover the BKL
>is held, so we increase twice the preempt count. But now that
>we drop the bkl from the boot, the preempt count is only increased
>once, and then we schedule in the acpi preemption point while we
>shouldn't.
>
>In fact using such in_atomic*() like helpers is quite fragile to
>guess if we can schedule, but still, in_atomic() is less buggy than
>what was there before.
>
>This fixes:
>
>[ 0.008086] BUG: scheduling while atomic: swapper/0/0x10000002
>[ 0.008167] no locks held by swapper/0.
>[ 0.008243] Modules linked in:
>[ 0.008356] Pid: 0, comm: swapper Not tainted 2.6.35+ #793
>[ 0.008437] Call Trace:
>[ 0.008519] [<ffffffff8106eab3>] ? __debug_show_held_locks+0x13/0x30
>[ 0.008605] [<ffffffff81039a65>] __schedule_bug+0x85/0x90
>[ 0.008690] [<ffffffff815edf20>] schedule+0x670/0x840
>[ 0.008775] [<ffffffff8129ff88>] ? acpi_os_release_object+0x9/0xd
>[ 0.008860] [<ffffffff812beca0>] ? acpi_ps_free_op+0x22/0x24
>[ 0.008944] [<ffffffff8103ccd5>] __cond_resched+0x25/0x40
>[ 0.009008] [<ffffffff815ee1ed>] _cond_resched+0x2d/0x40
>[ 0.009091] [<ffffffff812bdf4a>] acpi_ps_complete_op+0x292/0x2a8
>[ 0.009174] [<ffffffff812be7b6>] acpi_ps_parse_loop+0x856/0x9ac
>[ 0.010008] [<ffffffff812bd81d>] acpi_ps_parse_aml+0x9a/0x2b9
>[ 0.010092] [<ffffffff812bc048>] acpi_ns_one_complete_parse+0xfc/0x117
>[ 0.010176] [<ffffffff812bc07f>] acpi_ns_parse_table+0x1c/0x35
>[ 0.010259] [<ffffffff812b9606>] acpi_ns_load_table+0x4a/0x8c
>[ 0.010343] [<ffffffff812c075f>] acpi_load_tables+0xa0/0x164
>[ 0.010429] [<ffffffff819751e1>] ? acpi_initialize_subsystem+0x69/0x91
>[ 0.010513] [<ffffffff819740df>] acpi_early_init+0x6c/0xf7
>[ 0.010598] [<ffffffff8194fd68>] start_kernel+0x3b3/0x3fb
>[ 0.010681] [<ffffffff8194f26d>] x86_64_start_reservations+0x7d/0x89
>[ 0.010765] [<ffffffff8194f359>] x86_64_start_kernel+0xe0/0xf2
>
>Signed-off-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>
>Cc: Bob Moore <robert.moore@xxxxxxxxx>
>---
> include/acpi/platform/aclinux.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>diff --git a/include/acpi/platform/aclinux.h
>b/include/acpi/platform/aclinux.h
>index e5039a2..8da1e8c 100644
>--- a/include/acpi/platform/aclinux.h
>+++ b/include/acpi/platform/aclinux.h
>@@ -152,7 +152,7 @@ static inline void *acpi_os_acquire_object(acpi_cache_t
>* cache)
> #include <linux/hardirq.h>
> #define ACPI_PREEMPTION_POINT() \
> do { \
>- if (!in_atomic_preempt_off() && !irqs_disabled()) \
>+ if (!in_atomic() && !irqs_disabled()) \
> cond_resched(); \
> } while (0)
>
>--
>1.6.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/