When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
verbose WARN_ONCE message would print to the kernel log. It turned out
that the GPE interrupt handler was being called with local interrupts
enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
preemption strips local interrupt disabling from spinlock_t operations,
but not for raw_spinlock_t operations.
This is the warning that was triggered:
------------[ cut here ]------------
WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149 __handle_irq_event_percpu+0x6f/0xcf
irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
Modules linked in: gpio_irq_demo(O)
CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O 4.8.6-rt5-00012-geaa3b7c #6
Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643, BIOS 1-1.1.12.3_Alpha 04/29/2016
0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
Call Trace:
[<ffffffff81219a93>] dump_stack+0x4d/0x63
[<ffffffff8104b84a>] __warn+0xc0/0xdb
[<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
[<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
[<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
[<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
[<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
[<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
[<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
[<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
[<ffffffff81281090>] acpi_ev_gpe_dispatch+0xb1/0x125
[<ffffffff81281259>] acpi_ev_gpe_detect+0x155/0x1a2
[<ffffffff8107e423>] ? irq_thread_fn+0x2f/0x2f
[<ffffffff81283125>] acpi_ev_sci_xrupt_handler+0x1d/0x35
[<ffffffff8126f02e>] acpi_irq+0x11/0x2c
[<ffffffff8107e441>] irq_forced_thread_fn+0x1e/0x4a
[<ffffffff8107e668>] irq_thread+0xe0/0x19b
[<ffffffff815fc594>] ? __schedule+0x262/0x310
[<ffffffff8107e4f2>] ? wake_threads_waitq+0x28/0x28
[<ffffffff8107e588>] ? irq_thread_dtor+0x96/0x96
[<ffffffff81061aa4>] kthread+0xcd/0xd5
[<ffffffff815fef7f>] ret_from_fork+0x1f/0x40
[<ffffffff810619d7>] ? kthread_worker_fn+0xe8/0xe8
---[ end trace 0000000000000002 ]---
Signed-off-by: Aaron Sierra <[email protected]>
---
drivers/acpi/acpica/acglobal.h | 2 +-
drivers/acpi/acpica/evgpe.c | 15 +++++++--------
drivers/acpi/acpica/evgpeblk.c | 8 ++++----
drivers/acpi/acpica/evgpeutil.c | 12 ++++++------
drivers/acpi/acpica/evsci.c | 8 ++++----
drivers/acpi/acpica/evxface.c | 22 +++++++++++-----------
drivers/acpi/acpica/evxfgpe.c | 36 ++++++++++++++++++------------------
drivers/acpi/acpica/utmutex.c | 4 ++--
8 files changed, 53 insertions(+), 54 deletions(-)
diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index bda5232..c52d093 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -115,7 +115,7 @@ ACPI_GLOBAL(u8, acpi_gbl_global_lock_pending);
* Spinlocks are used for interfaces that can be possibly called at
* interrupt level
*/
-ACPI_GLOBAL(acpi_spinlock, acpi_gbl_gpe_lock); /* For GPE data structs and registers */
+ACPI_GLOBAL(acpi_raw_spinlock, acpi_gbl_gpe_lock); /* For GPE data structs and registers */
ACPI_GLOBAL(acpi_raw_spinlock, acpi_gbl_hardware_lock); /* For ACPI H/W except GPE registers */
ACPI_GLOBAL(acpi_spinlock, acpi_gbl_reference_count_lock);
diff --git a/drivers/acpi/acpica/evgpe.c b/drivers/acpi/acpica/evgpe.c
index 4b4949c..ee97308 100644
--- a/drivers/acpi/acpica/evgpe.c
+++ b/drivers/acpi/acpica/evgpe.c
@@ -355,7 +355,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list)
* Note: Not necessary to obtain the hardware lock, since the GPE
* registers are owned by the gpe_lock.
*/
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Examine all GPE blocks attached to this interrupt level */
@@ -479,7 +479,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list)
* acpi_os_wait_events_complete() before the
* destruction.
*/
- acpi_os_release_lock
+ raw_spin_unlock_irqrestore
(acpi_gbl_gpe_lock, flags);
int_status |=
gpe_handler_info->
@@ -487,9 +487,8 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list)
gpe_number,
gpe_handler_info->
context);
- flags =
- acpi_os_acquire_lock
- (acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(
+ acpi_gbl_gpe_lock, flags);
} else {
/*
* Dispatch the event to a standard handler or
@@ -509,7 +508,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list)
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return (int_status);
}
@@ -631,9 +630,9 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_enable_gpe(void *context)
struct acpi_gpe_event_info *gpe_event_info = context;
acpi_cpu_flags flags;
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
(void)acpi_ev_finish_gpe(gpe_event_info);
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return;
}
diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
index d54014c..131050c 100644
--- a/drivers/acpi/acpica/evgpeblk.c
+++ b/drivers/acpi/acpica/evgpeblk.c
@@ -95,7 +95,7 @@ acpi_ev_install_gpe_block(struct acpi_gpe_block_info *gpe_block,
/* Install the new block at the end of the list with lock */
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
if (gpe_xrupt_block->gpe_block_list_head) {
next_gpe_block = gpe_xrupt_block->gpe_block_list_head;
while (next_gpe_block->next) {
@@ -109,7 +109,7 @@ acpi_ev_install_gpe_block(struct acpi_gpe_block_info *gpe_block,
}
gpe_block->xrupt_block = gpe_xrupt_block;
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
unlock_and_exit:
(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
@@ -156,7 +156,7 @@ acpi_status acpi_ev_delete_gpe_block(struct acpi_gpe_block_info *gpe_block)
} else {
/* Remove the block on this interrupt with lock */
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
if (gpe_block->previous) {
gpe_block->previous->next = gpe_block->next;
} else {
@@ -168,7 +168,7 @@ acpi_status acpi_ev_delete_gpe_block(struct acpi_gpe_block_info *gpe_block)
gpe_block->next->previous = gpe_block->previous;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
}
acpi_current_gpe_count -= gpe_block->gpe_count;
diff --git a/drivers/acpi/acpica/evgpeutil.c b/drivers/acpi/acpica/evgpeutil.c
index 3f150d5..5852fce 100644
--- a/drivers/acpi/acpica/evgpeutil.c
+++ b/drivers/acpi/acpica/evgpeutil.c
@@ -71,7 +71,7 @@ acpi_ev_walk_gpe_list(acpi_gpe_callback gpe_walk_callback, void *context)
ACPI_FUNCTION_TRACE(ev_walk_gpe_list);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Walk the interrupt level descriptor list */
@@ -102,7 +102,7 @@ acpi_ev_walk_gpe_list(acpi_gpe_callback gpe_walk_callback, void *context)
}
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -195,7 +195,7 @@ acpi_ev_get_gpe_xrupt_block(u32 interrupt_number,
/* Install new interrupt descriptor with spin lock */
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
if (acpi_gbl_gpe_xrupt_list_head) {
next_gpe_xrupt = acpi_gbl_gpe_xrupt_list_head;
while (next_gpe_xrupt->next) {
@@ -208,7 +208,7 @@ acpi_ev_get_gpe_xrupt_block(u32 interrupt_number,
acpi_gbl_gpe_xrupt_list_head = gpe_xrupt;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
/* Install new interrupt handler if not SCI_INT */
@@ -266,7 +266,7 @@ acpi_status acpi_ev_delete_gpe_xrupt(struct acpi_gpe_xrupt_info *gpe_xrupt)
/* Unlink the interrupt block with lock */
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
if (gpe_xrupt->previous) {
gpe_xrupt->previous->next = gpe_xrupt->next;
} else {
@@ -278,7 +278,7 @@ acpi_status acpi_ev_delete_gpe_xrupt(struct acpi_gpe_xrupt_info *gpe_xrupt)
if (gpe_xrupt->next) {
gpe_xrupt->next->previous = gpe_xrupt->previous;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
/* Free the block */
diff --git a/drivers/acpi/acpica/evsci.c b/drivers/acpi/acpica/evsci.c
index 3b7757c..e60c0b8 100644
--- a/drivers/acpi/acpica/evsci.c
+++ b/drivers/acpi/acpica/evsci.c
@@ -78,7 +78,7 @@ u32 acpi_ev_sci_dispatch(void)
return (int_status);
}
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Invoke all host-installed SCI handlers */
@@ -92,7 +92,7 @@ u32 acpi_ev_sci_dispatch(void)
sci_handler = sci_handler->next;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return (int_status);
}
@@ -233,7 +233,7 @@ acpi_status acpi_ev_remove_all_sci_handlers(void)
return (status);
}
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Free all host-installed SCI handlers */
@@ -243,7 +243,7 @@ acpi_status acpi_ev_remove_all_sci_handlers(void)
ACPI_FREE(sci_handler);
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
diff --git a/drivers/acpi/acpica/evxface.c b/drivers/acpi/acpica/evxface.c
index e4e9260..36c5661 100644
--- a/drivers/acpi/acpica/evxface.c
+++ b/drivers/acpi/acpica/evxface.c
@@ -450,7 +450,7 @@ acpi_status acpi_install_sci_handler(acpi_sci_handler address, void *context)
/* Lock list during installation */
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
sci_handler = acpi_gbl_sci_handler_list;
/* Ensure handler does not already exist */
@@ -471,7 +471,7 @@ acpi_status acpi_install_sci_handler(acpi_sci_handler address, void *context)
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
exit:
@@ -514,7 +514,7 @@ acpi_status acpi_remove_sci_handler(acpi_sci_handler address)
/* Remove the SCI handler with lock */
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
prev_sci_handler = NULL;
next_sci_handler = acpi_gbl_sci_handler_list;
@@ -530,7 +530,7 @@ acpi_status acpi_remove_sci_handler(acpi_sci_handler address)
next_sci_handler->next;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
ACPI_FREE(next_sci_handler);
goto unlock_and_exit;
}
@@ -539,7 +539,7 @@ acpi_status acpi_remove_sci_handler(acpi_sci_handler address)
next_sci_handler = next_sci_handler->next;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
status = AE_NOT_EXIST;
unlock_and_exit:
@@ -779,7 +779,7 @@ acpi_ev_install_gpe_handler(acpi_handle gpe_device,
goto unlock_and_exit;
}
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -840,14 +840,14 @@ acpi_ev_install_gpe_handler(acpi_handle gpe_device,
(is_raw_handler ? ACPI_GPE_DISPATCH_RAW_HANDLER :
ACPI_GPE_DISPATCH_HANDLER));
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
unlock_and_exit:
(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
return_ACPI_STATUS(status);
free_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
ACPI_FREE(handler);
goto unlock_and_exit;
}
@@ -957,7 +957,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
return_ACPI_STATUS(status);
}
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -1008,7 +1008,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
(void)acpi_ev_add_gpe_reference(gpe_event_info);
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
/* Make sure all deferred GPE tasks are completed */
@@ -1021,7 +1021,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
return_ACPI_STATUS(status);
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
return_ACPI_STATUS(status);
diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c
index 17cfef7..5854bd9 100644
--- a/drivers/acpi/acpica/evxfgpe.c
+++ b/drivers/acpi/acpica/evxfgpe.c
@@ -123,7 +123,7 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
ACPI_FUNCTION_TRACE(acpi_enable_gpe);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/*
* Ensure that we have a valid GPE number and that there is some way
@@ -140,7 +140,7 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
}
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
ACPI_EXPORT_SYMBOL(acpi_enable_gpe)
@@ -168,7 +168,7 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number)
ACPI_FUNCTION_TRACE(acpi_disable_gpe);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -177,7 +177,7 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number)
status = acpi_ev_remove_gpe_reference(gpe_event_info) ;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -219,7 +219,7 @@ acpi_status acpi_set_gpe(acpi_handle gpe_device, u32 gpe_number, u8 action)
ACPI_FUNCTION_TRACE(acpi_set_gpe);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -249,7 +249,7 @@ acpi_status acpi_set_gpe(acpi_handle gpe_device, u32 gpe_number, u8 action)
}
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -283,7 +283,7 @@ acpi_status acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number)
ACPI_FUNCTION_TRACE(acpi_mark_gpe_for_wake);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -296,7 +296,7 @@ acpi_status acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number)
status = AE_OK;
}
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -368,7 +368,7 @@ acpi_setup_gpe_for_wake(acpi_handle wake_device,
return_ACPI_STATUS(AE_NO_MEMORY);
}
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -426,7 +426,7 @@ acpi_setup_gpe_for_wake(acpi_handle wake_device,
status = AE_OK;
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
/* Delete the notify object if it was not used above */
@@ -463,7 +463,7 @@ acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 action)
ACPI_FUNCTION_TRACE(acpi_set_gpe_wake_mask);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/*
* Ensure that we have a valid GPE number and that this GPE is in
@@ -511,7 +511,7 @@ acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 action)
}
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -537,7 +537,7 @@ acpi_status acpi_clear_gpe(acpi_handle gpe_device, u32 gpe_number)
ACPI_FUNCTION_TRACE(acpi_clear_gpe);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -550,7 +550,7 @@ acpi_status acpi_clear_gpe(acpi_handle gpe_device, u32 gpe_number)
status = acpi_hw_clear_gpe(gpe_event_info);
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -580,7 +580,7 @@ acpi_get_gpe_status(acpi_handle gpe_device,
ACPI_FUNCTION_TRACE(acpi_get_gpe_status);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -595,7 +595,7 @@ acpi_get_gpe_status(acpi_handle gpe_device,
status = acpi_hw_get_gpe_status(gpe_event_info, event_status);
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
@@ -625,7 +625,7 @@ acpi_status acpi_finish_gpe(acpi_handle gpe_device, u32 gpe_number)
ACPI_FUNCTION_TRACE(acpi_finish_gpe);
- flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+ raw_spin_lock_irqsave(acpi_gbl_gpe_lock, flags);
/* Ensure that we have a valid GPE number */
@@ -638,7 +638,7 @@ acpi_status acpi_finish_gpe(acpi_handle gpe_device, u32 gpe_number)
status = acpi_ev_finish_gpe(gpe_event_info);
unlock_and_exit:
- acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+ raw_spin_unlock_irqrestore(acpi_gbl_gpe_lock, flags);
return_ACPI_STATUS(status);
}
diff --git a/drivers/acpi/acpica/utmutex.c b/drivers/acpi/acpica/utmutex.c
index 357e7ca..d7257a1 100644
--- a/drivers/acpi/acpica/utmutex.c
+++ b/drivers/acpi/acpica/utmutex.c
@@ -83,7 +83,7 @@ acpi_status acpi_ut_mutex_initialize(void)
/* Create the spinlocks for use at interrupt level or for speed */
- status = acpi_os_create_lock (&acpi_gbl_gpe_lock);
+ status = acpi_os_create_raw_lock (&acpi_gbl_gpe_lock);
if (ACPI_FAILURE (status)) {
return_ACPI_STATUS (status);
}
@@ -144,7 +144,7 @@ void acpi_ut_mutex_terminate(void)
/* Delete the spinlocks */
- acpi_os_delete_lock(acpi_gbl_gpe_lock);
+ acpi_os_delete_raw_lock(acpi_gbl_gpe_lock);
acpi_os_delete_raw_lock(acpi_gbl_hardware_lock);
acpi_os_delete_lock(acpi_gbl_reference_count_lock);
--
1.9.1
On Wed, 30 Nov 2016, Aaron Sierra wrote:
> When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
> verbose WARN_ONCE message would print to the kernel log. It turned out
> that the GPE interrupt handler was being called with local interrupts
> enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
> preemption strips local interrupt disabling from spinlock_t operations,
> but not for raw_spinlock_t operations.
>
> This is the warning that was triggered:
>
> ------------[ cut here ]------------
> WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149 __handle_irq_event_percpu+0x6f/0xcf
> irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
> Modules linked in: gpio_irq_demo(O)
> CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O 4.8.6-rt5-00012-geaa3b7c #6
> Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643, BIOS 1-1.1.12.3_Alpha 04/29/2016
> 0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
> 0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
> ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
> Call Trace:
> [<ffffffff81219a93>] dump_stack+0x4d/0x63
> [<ffffffff8104b84a>] __warn+0xc0/0xdb
> [<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
> [<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
> [<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
> [<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
> [<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
> [<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
> [<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
> [<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
gpe_irq_handler is not in tree, so I really cannot tell what this is
about. It looks like it does interrupt demultiplexing, which triggers the
warning.
We tried to make that lock raw years ago and this results in a different
splat: https://marc.info/?l=linux-kernel&m=132468512523207&w=2
So no, we are not making that lock raw again blindly just because of random
out of tree code.
Thanks,
tglx
----- Original Message -----
> From: "Thomas Gleixner" <[email protected]>
> Sent: Thursday, December 1, 2016 8:52:48 AM
> On Wed, 30 Nov 2016, Aaron Sierra wrote:
>> When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
>> verbose WARN_ONCE message would print to the kernel log. It turned out
>> that the GPE interrupt handler was being called with local interrupts
>> enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
>> preemption strips local interrupt disabling from spinlock_t operations,
>> but not for raw_spinlock_t operations.
>>
>> This is the warning that was triggered:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149
>> __handle_irq_event_percpu+0x6f/0xcf
>> irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
>> Modules linked in: gpio_irq_demo(O)
>> CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O
>> 4.8.6-rt5-00012-geaa3b7c #6
>> Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643,
>> BIOS 1-1.1.12.3_Alpha 04/29/2016
>> 0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
>> 0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
>> ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
>> Call Trace:
>> [<ffffffff81219a93>] dump_stack+0x4d/0x63
>> [<ffffffff8104b84a>] __warn+0xc0/0xdb
>> [<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
>> [<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
>> [<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
>> [<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
>> [<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
>> [<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
>> [<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
>> [<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
>
> gpe_irq_handler is not in tree, so I really cannot tell what this is
> about. It looks like it does interrupt demultiplexing, which triggers the
> warning.
Thomas,
Your guess is correct, this involves a currently out-of-tree extension to
the gpio-ich driver which allows GPIO pins to be mapped to IRQs by demuxing
their corresponding GPE events.
The gpio_irq_demo module requests a GPIO pin, converts it to an IRQ using
gpio_to_irq() and registers a trivial handler against the IRQ.
> We tried to make that lock raw years ago and this results in a different
> splat: https://marc.info/?l=linux-kernel&m=132468512523207&w=2
Thanks for the background.
> So no, we are not making that lock raw again blindly just because of random
> out of tree code.
I thought that might be that case.
-Aaron S.
On Thu, 1 Dec 2016, Aaron Sierra wrote:
> ----- Original Message -----
> > From: "Thomas Gleixner" <[email protected]>
> > Sent: Thursday, December 1, 2016 8:52:48 AM
>
> > On Wed, 30 Nov 2016, Aaron Sierra wrote:
> >> When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
> >> verbose WARN_ONCE message would print to the kernel log. It turned out
> >> that the GPE interrupt handler was being called with local interrupts
> >> enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
> >> preemption strips local interrupt disabling from spinlock_t operations,
> >> but not for raw_spinlock_t operations.
> >>
> >> This is the warning that was triggered:
> >>
> >> ------------[ cut here ]------------
> >> WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149
> >> __handle_irq_event_percpu+0x6f/0xcf
> >> irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
> >> Modules linked in: gpio_irq_demo(O)
> >> CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O
> >> 4.8.6-rt5-00012-geaa3b7c #6
> >> Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643,
> >> BIOS 1-1.1.12.3_Alpha 04/29/2016
> >> 0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
> >> 0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
> >> ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
> >> Call Trace:
> >> [<ffffffff81219a93>] dump_stack+0x4d/0x63
> >> [<ffffffff8104b84a>] __warn+0xc0/0xdb
> >> [<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
> >> [<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
> >> [<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
> >> [<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
> >> [<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
> >> [<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
> >> [<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
> >> [<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
> >
> > gpe_irq_handler is not in tree, so I really cannot tell what this is
> > about. It looks like it does interrupt demultiplexing, which triggers the
> > warning.
>
> Thomas,
> Your guess is correct, this involves a currently out-of-tree extension to
> the gpio-ich driver which allows GPIO pins to be mapped to IRQs by demuxing
> their corresponding GPE events.
> The gpio_irq_demo module requests a GPIO pin, converts it to an IRQ using
> gpio_to_irq() and registers a trivial handler against the IRQ.
So the simple solution is in the demux handler:
gpe_irq_handler()
{
+ local_irq_save(flags);
for_each_pending_gpio_irq(irq)
generic_handle_irq(irq);
+ local_irq_restore(flags);
}
Thanks,
tglx
On Thursday, December 01, 2016 05:16:00 PM Aaron Sierra wrote:
> ----- Original Message -----
> > From: "Thomas Gleixner" <[email protected]>
> > Sent: Thursday, December 1, 2016 8:52:48 AM
>
> > On Wed, 30 Nov 2016, Aaron Sierra wrote:
> >> When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
> >> verbose WARN_ONCE message would print to the kernel log. It turned out
> >> that the GPE interrupt handler was being called with local interrupts
> >> enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
> >> preemption strips local interrupt disabling from spinlock_t operations,
> >> but not for raw_spinlock_t operations.
> >>
> >> This is the warning that was triggered:
> >>
> >> ------------[ cut here ]------------
> >> WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149
> >> __handle_irq_event_percpu+0x6f/0xcf
> >> irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
> >> Modules linked in: gpio_irq_demo(O)
> >> CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O
> >> 4.8.6-rt5-00012-geaa3b7c #6
> >> Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643,
> >> BIOS 1-1.1.12.3_Alpha 04/29/2016
> >> 0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
> >> 0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
> >> ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
> >> Call Trace:
> >> [<ffffffff81219a93>] dump_stack+0x4d/0x63
> >> [<ffffffff8104b84a>] __warn+0xc0/0xdb
> >> [<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
> >> [<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
> >> [<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
> >> [<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
> >> [<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
> >> [<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
> >> [<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
> >> [<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
> >
> > gpe_irq_handler is not in tree, so I really cannot tell what this is
> > about. It looks like it does interrupt demultiplexing, which triggers the
> > warning.
>
> Thomas,
> Your guess is correct, this involves a currently out-of-tree extension to
> the gpio-ich driver which allows GPIO pins to be mapped to IRQs by demuxing
> their corresponding GPE events.
>
> The gpio_irq_demo module requests a GPIO pin, converts it to an IRQ using
> gpio_to_irq() and registers a trivial handler against the IRQ.
And what does it have to do with GPEs, if I may ask?
In the future, please CC all ACPI-related changes to [email protected].
Thanks,
Rafael
----- Original Message -----
> From: "Thomas Gleixner" <[email protected]>
> Sent: Thursday, December 1, 2016 5:21:20 PM
> On Thu, 1 Dec 2016, Aaron Sierra wrote:
>> ----- Original Message -----
>> > From: "Thomas Gleixner" <[email protected]>
>> > Sent: Thursday, December 1, 2016 8:52:48 AM
>>
>> > On Wed, 30 Nov 2016, Aaron Sierra wrote:
>> >> When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
>> >> verbose WARN_ONCE message would print to the kernel log. It turned out
>> >> that the GPE interrupt handler was being called with local interrupts
>> >> enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
>> >> preemption strips local interrupt disabling from spinlock_t operations,
>> >> but not for raw_spinlock_t operations.
>> >>
>> >> This is the warning that was triggered:
>> >>
>> >> ------------[ cut here ]------------
>> >> WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149
>> >> __handle_irq_event_percpu+0x6f/0xcf
>> >> irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
>> >> Modules linked in: gpio_irq_demo(O)
>> >> CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O
>> >> 4.8.6-rt5-00012-geaa3b7c #6
>> >> Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643,
>> >> BIOS 1-1.1.12.3_Alpha 04/29/2016
>> >> 0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
>> >> 0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
>> >> ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
>> >> Call Trace:
>> >> [<ffffffff81219a93>] dump_stack+0x4d/0x63
>> >> [<ffffffff8104b84a>] __warn+0xc0/0xdb
>> >> [<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
>> >> [<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
>> >> [<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
>> >> [<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
>> >> [<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
>> >> [<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
>> >> [<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
>> >> [<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
>> >
>> > gpe_irq_handler is not in tree, so I really cannot tell what this is
>> > about. It looks like it does interrupt demultiplexing, which triggers the
>> > warning.
>>
>> Thomas,
>> Your guess is correct, this involves a currently out-of-tree extension to
>> the gpio-ich driver which allows GPIO pins to be mapped to IRQs by demuxing
>> their corresponding GPE events.
>> The gpio_irq_demo module requests a GPIO pin, converts it to an IRQ using
>> gpio_to_irq() and registers a trivial handler against the IRQ.
>
> So the simple solution is in the demux handler:
>
> gpe_irq_handler()
> {
> + local_irq_save(flags);
>
> for_each_pending_gpio_irq(irq)
> generic_handle_irq(irq);
>
> + local_irq_restore(flags);
> }
Thomas,
Yes, of course. That's how I initially worked around my problem before
digging into the ACPI subsystem. I just didn't believe that was anything
more than a workaround, so I went looking for a more "correct" solution.
I don't have a problem maintaining this for myself if it isn't generally
useful.
-Aaron S.
----- Original Message -----
> From: "Rafael J. Wysocki" <[email protected]>
> Sent: Thursday, December 1, 2016 5:27:03 PM
> On Thursday, December 01, 2016 05:16:00 PM Aaron Sierra wrote:
>> ----- Original Message -----
>> > From: "Thomas Gleixner" <[email protected]>
>> > Sent: Thursday, December 1, 2016 8:52:48 AM
>>
>> > On Wed, 30 Nov 2016, Aaron Sierra wrote:
>> >> When testing GPE interrupts with CONFIG_PREEMPT_RT_FULL enabled, a
>> >> verbose WARN_ONCE message would print to the kernel log. It turned out
>> >> that the GPE interrupt handler was being called with local interrupts
>> >> enabled because acpi_gbl_gpe_lock was implemented as a spinlock_t. Full
>> >> preemption strips local interrupt disabling from spinlock_t operations,
>> >> but not for raw_spinlock_t operations.
>> >>
>> >> This is the warning that was triggered:
>> >>
>> >> ------------[ cut here ]------------
>> >> WARNING: CPU: 8 PID: 483 at kernel/irq/handle.c:149
>> >> __handle_irq_event_percpu+0x6f/0xcf
>> >> irq 33 handler irq_default_primary_handler+0x0/0xb enabled interrupts
>> >> Modules linked in: gpio_irq_demo(O)
>> >> CPU: 8 PID: 483 Comm: irq/9-acpi Tainted: G O
>> >> 4.8.6-rt5-00012-geaa3b7c #6
>> >> Hardware name: Extreme Engineering Solutions, Inc. XCalibur4643/XCalibur4643,
>> >> BIOS 1-1.1.12.3_Alpha 04/29/2016
>> >> 0000000000000000 ffff880858f3bc10 ffffffff81219a93 ffff880858f3bc60
>> >> 0000000000000000 ffff880858f3bc50 ffffffff8104b84a 0000009500000000
>> >> ffff880855b76880 0000000000000021 0000000000000002 ffff880856356800
>> >> Call Trace:
>> >> [<ffffffff81219a93>] dump_stack+0x4d/0x63
>> >> [<ffffffff8104b84a>] __warn+0xc0/0xdb
>> >> [<ffffffff8104b8af>] warn_slowpath_fmt+0x4a/0x4c
>> >> [<ffffffff810f7192>] ? path_openat+0xbf8/0xc62
>> >> [<ffffffff8107d7c3>] ? handle_irq_event+0x75/0x75
>> >> [<ffffffff8107d68d>] __handle_irq_event_percpu+0x6f/0xcf
>> >> [<ffffffff8107d727>] handle_irq_event_percpu+0x3a/0x61
>> >> [<ffffffff8107d7a3>] handle_irq_event+0x55/0x75
>> >> [<ffffffff8107ff76>] handle_simple_irq+0x5c/0x92
>> >> [<ffffffff81243a53>] gpe_irq_handler+0x2a/0x31
>> >
>> > gpe_irq_handler is not in tree, so I really cannot tell what this is
>> > about. It looks like it does interrupt demultiplexing, which triggers the
>> > warning.
>>
>> Thomas,
>> Your guess is correct, this involves a currently out-of-tree extension to
>> the gpio-ich driver which allows GPIO pins to be mapped to IRQs by demuxing
>> their corresponding GPE events.
>>
>> The gpio_irq_demo module requests a GPIO pin, converts it to an IRQ using
>> gpio_to_irq() and registers a trivial handler against the IRQ.
>
> And what does it have to do with GPEs, if I may ask?
Rafael,
The gpio-ich driver controls Intel ICH/PCH GPIOs. The first 16 of those GPIOs
on many platforms can be configured to generate GPEs. Demuxing those GPEs to
IRQs allows this driver to fit within the gpiolib framework to make GPIO-to-
IRQ translation possible.
> In the future, please CC all ACPI-related changes to [email protected].
Sorry, I only looked up where RT patches should go.
-Aaron S.