Re: powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers
On Fri, 2015-17-07 at 10:46:58 UTC, Thomas Huth wrote: The EPOW interrupt handler uses rtas_get_sensor(), which in turn uses rtas_busy_delay() to wait for RTAS becoming ready in case it is necessary. But rtas_busy_delay() is annotated with might_sleep() and thus may not be used by interrupts handlers like the EPOW handler! This leads to the following BUG when CONFIG_DEBUG_ATOMIC_SLEEP is enabled: BUG: sleeping function called from invalid context at arch/powerpc/kernel/rtas.c:496 in_atomic(): 1, irqs_disabled(): 1, pid: 0, name: swapper/1 CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0-rc2-thuth #6 Call Trace: [c0007ffe7b90] [c0807670] dump_stack+0xa0/0xdc (unreliable) [c0007ffe7bc0] [c00e1f14] ___might_sleep+0x134/0x180 [c0007ffe7c20] [c002aec0] rtas_busy_delay+0x30/0xd0 [c0007ffe7c50] [c002bde4] rtas_get_sensor+0x74/0xe0 [c0007ffe7ce0] [c0083264] ras_epow_interrupt+0x44/0x450 [c0007ffe7d90] [c0120260] handle_irq_event_percpu+0xa0/0x300 [c0007ffe7e70] [c0120524] handle_irq_event+0x64/0xc0 [c0007ffe7eb0] [c0124dbc] handle_fasteoi_irq+0xec/0x260 [c0007ffe7ef0] [c011f4f0] generic_handle_irq+0x50/0x80 [c0007ffe7f20] [c0010f3c] __do_irq+0x8c/0x200 [c0007ffe7f90] [c00236cc] call_do_irq+0x14/0x24 [c0007e6f39e0] [c0011144] do_IRQ+0x94/0x110 [c0007e6f3a30] [c0002594] hardware_interrupt_common+0x114/0x180 Fix this issue by introducing a new rtas_get_sensor_fast() function that does not use rtas_busy_delay() - and thus can only be used for sensors that do not cause a BUSY condition (which should be the case for the sensor that is queried by the EPOW IRQ handler). Signed-off-by: Thomas Huth th...@redhat.com Reviewed-by: Nathan Fontenot nf...@linux.vnet.ibm.com Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/1c2cb594441d02815d30 cheers -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers
- Original Message -
The EPOW interrupt handler uses rtas_get_sensor(), which in turn
uses rtas_busy_delay() to wait for RTAS becoming ready in case it
is necessary. But rtas_busy_delay() is annotated with might_sleep()
and thus may not be used by interrupts handlers like the EPOW handler!
This leads to the following BUG when CONFIG_DEBUG_ATOMIC_SLEEP is
enabled:
BUG: sleeping function called from invalid context at
arch/powerpc/kernel/rtas.c:496
in_atomic(): 1, irqs_disabled(): 1, pid: 0, name: swapper/1
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0-rc2-thuth #6
Call Trace:
[c0007ffe7b90] [c0807670] dump_stack+0xa0/0xdc (unreliable)
[c0007ffe7bc0] [c00e1f14] ___might_sleep+0x134/0x180
[c0007ffe7c20] [c002aec0] rtas_busy_delay+0x30/0xd0
[c0007ffe7c50] [c002bde4] rtas_get_sensor+0x74/0xe0
[c0007ffe7ce0] [c0083264] ras_epow_interrupt+0x44/0x450
[c0007ffe7d90] [c0120260] handle_irq_event_percpu+0xa0/0x300
[c0007ffe7e70] [c0120524] handle_irq_event+0x64/0xc0
[c0007ffe7eb0] [c0124dbc] handle_fasteoi_irq+0xec/0x260
[c0007ffe7ef0] [c011f4f0] generic_handle_irq+0x50/0x80
[c0007ffe7f20] [c0010f3c] __do_irq+0x8c/0x200
[c0007ffe7f90] [c00236cc] call_do_irq+0x14/0x24
[c0007e6f39e0] [c0011144] do_IRQ+0x94/0x110
[c0007e6f3a30] [c0002594] hardware_interrupt_common+0x114/0x180
Fix this issue by introducing a new rtas_get_sensor_fast() function
that does not use rtas_busy_delay() - and thus can only be used for
sensors that do not cause a BUSY condition (which should be the case
for the sensor that is queried by the EPOW IRQ handler).
Signed-off-by: Thomas Huth th...@redhat.com
---
arch/powerpc/include/asm/rtas.h | 1 +
arch/powerpc/kernel/rtas.c | 17 +
arch/powerpc/platforms/pseries/ras.c | 3 ++-
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/rtas.h
b/arch/powerpc/include/asm/rtas.h
index 7a4ede1..b77ef36 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -343,6 +343,7 @@ extern void rtas_power_off(void);
extern void rtas_halt(void);
extern void rtas_os_term(char *str);
extern int rtas_get_sensor(int sensor, int index, int *state);
+extern int rtas_get_sensor_fast(int sensor, int index, int *state);
extern int rtas_get_power_level(int powerdomain, int *level);
extern int rtas_set_power_level(int powerdomain, int level, int *setlevel);
extern bool rtas_indicator_present(int token, int *maxindex);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 7a488c1..caffb10 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -584,6 +584,23 @@ int rtas_get_sensor(int sensor, int index, int *state)
}
EXPORT_SYMBOL(rtas_get_sensor);
+int rtas_get_sensor_fast(int sensor, int index, int *state)
+{
+ int token = rtas_token(get-sensor-state);
+ int rc;
+
+ if (token == RTAS_UNKNOWN_SERVICE)
+ return -ENOENT;
+
+ rc = rtas_call(token, 2, 2, state, sensor, index);
+ WARN_ON(rc == RTAS_BUSY || (rc = RTAS_EXTENDED_DELAY_MIN
+ rc = RTAS_EXTENDED_DELAY_MAX));
+
+ if (rc 0)
+ return rtas_error_rc(rc);
+ return rc;
+}
+
bool rtas_indicator_present(int token, int *maxindex)
{
int proplen, count, i;
diff --git a/arch/powerpc/platforms/pseries/ras.c
b/arch/powerpc/platforms/pseries/ras.c
index 02e4a17..3b6647e 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -189,7 +189,8 @@ static irqreturn_t ras_epow_interrupt(int irq, void
*dev_id)
int state;
int critical;
- status = rtas_get_sensor(EPOW_SENSOR_TOKEN, EPOW_SENSOR_INDEX, state);
+ status = rtas_get_sensor_fast(EPOW_SENSOR_TOKEN, EPOW_SENSOR_INDEX,
+ state);
if (state 3)
critical = 1; /* Time Critical */
--
1.8.3.1
*ping*
Michael, do you think this patch is OK for fixing this problem?
Or shall I rather send a patch to simply revert 587f83e8dd50d instead?
Thomas
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers
On Fri, 2015-17-07 at 10:46:58 UTC, Thomas Huth wrote: The EPOW interrupt handler uses rtas_get_sensor(), which in turn uses rtas_busy_delay() to wait for RTAS becoming ready in case it is necessary. But rtas_busy_delay() is annotated with might_sleep() and thus may not be used by interrupts handlers like the EPOW handler! This leads to the following BUG when CONFIG_DEBUG_ATOMIC_SLEEP is enabled: When did we break this? cheers -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers
On 22/07/15 13:25, Michael Ellerman wrote: On Fri, 2015-17-07 at 10:46:58 UTC, Thomas Huth wrote: The EPOW interrupt handler uses rtas_get_sensor(), which in turn uses rtas_busy_delay() to wait for RTAS becoming ready in case it is necessary. But rtas_busy_delay() is annotated with might_sleep() and thus may not be used by interrupts handlers like the EPOW handler! This leads to the following BUG when CONFIG_DEBUG_ATOMIC_SLEEP is enabled: When did we break this? Hi Michael, the bug has been introduced by commit 587f83e8dd50d22bc0c62e32ec49fd31 (powerpc/pseries: Use rtas_get_sensor in RAS code) which switched the EPOW handler to use rtas_get_sensor() instead of using rtas_call directly. Also have a look at this thread here: http://www.spinics.net/lists/kvm-ppc/msg10768.html Thomas -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers
On 07/17/2015 05:46 AM, Thomas Huth wrote: The EPOW interrupt handler uses rtas_get_sensor(), which in turn uses rtas_busy_delay() to wait for RTAS becoming ready in case it is necessary. But rtas_busy_delay() is annotated with might_sleep() and thus may not be used by interrupts handlers like the EPOW handler! This leads to the following BUG when CONFIG_DEBUG_ATOMIC_SLEEP is enabled: BUG: sleeping function called from invalid context at arch/powerpc/kernel/rtas.c:496 in_atomic(): 1, irqs_disabled(): 1, pid: 0, name: swapper/1 CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0-rc2-thuth #6 Call Trace: [c0007ffe7b90] [c0807670] dump_stack+0xa0/0xdc (unreliable) [c0007ffe7bc0] [c00e1f14] ___might_sleep+0x134/0x180 [c0007ffe7c20] [c002aec0] rtas_busy_delay+0x30/0xd0 [c0007ffe7c50] [c002bde4] rtas_get_sensor+0x74/0xe0 [c0007ffe7ce0] [c0083264] ras_epow_interrupt+0x44/0x450 [c0007ffe7d90] [c0120260] handle_irq_event_percpu+0xa0/0x300 [c0007ffe7e70] [c0120524] handle_irq_event+0x64/0xc0 [c0007ffe7eb0] [c0124dbc] handle_fasteoi_irq+0xec/0x260 [c0007ffe7ef0] [c011f4f0] generic_handle_irq+0x50/0x80 [c0007ffe7f20] [c0010f3c] __do_irq+0x8c/0x200 [c0007ffe7f90] [c00236cc] call_do_irq+0x14/0x24 [c0007e6f39e0] [c0011144] do_IRQ+0x94/0x110 [c0007e6f3a30] [c0002594] hardware_interrupt_common+0x114/0x180 Fix this issue by introducing a new rtas_get_sensor_fast() function that does not use rtas_busy_delay() - and thus can only be used for sensors that do not cause a BUSY condition (which should be the case for the sensor that is queried by the EPOW IRQ handler). Signed-off-by: Thomas Huth th...@redhat.com Reviewed-by: Nathan Fontenot nf...@linux.vnet.ibm.com --- arch/powerpc/include/asm/rtas.h | 1 + arch/powerpc/kernel/rtas.c | 17 + arch/powerpc/platforms/pseries/ras.c | 3 ++- 3 files changed, 20 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers
The EPOW interrupt handler uses rtas_get_sensor(), which in turn
uses rtas_busy_delay() to wait for RTAS becoming ready in case it
is necessary. But rtas_busy_delay() is annotated with might_sleep()
and thus may not be used by interrupts handlers like the EPOW handler!
This leads to the following BUG when CONFIG_DEBUG_ATOMIC_SLEEP is
enabled:
BUG: sleeping function called from invalid context at
arch/powerpc/kernel/rtas.c:496
in_atomic(): 1, irqs_disabled(): 1, pid: 0, name: swapper/1
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.2.0-rc2-thuth #6
Call Trace:
[c0007ffe7b90] [c0807670] dump_stack+0xa0/0xdc (unreliable)
[c0007ffe7bc0] [c00e1f14] ___might_sleep+0x134/0x180
[c0007ffe7c20] [c002aec0] rtas_busy_delay+0x30/0xd0
[c0007ffe7c50] [c002bde4] rtas_get_sensor+0x74/0xe0
[c0007ffe7ce0] [c0083264] ras_epow_interrupt+0x44/0x450
[c0007ffe7d90] [c0120260] handle_irq_event_percpu+0xa0/0x300
[c0007ffe7e70] [c0120524] handle_irq_event+0x64/0xc0
[c0007ffe7eb0] [c0124dbc] handle_fasteoi_irq+0xec/0x260
[c0007ffe7ef0] [c011f4f0] generic_handle_irq+0x50/0x80
[c0007ffe7f20] [c0010f3c] __do_irq+0x8c/0x200
[c0007ffe7f90] [c00236cc] call_do_irq+0x14/0x24
[c0007e6f39e0] [c0011144] do_IRQ+0x94/0x110
[c0007e6f3a30] [c0002594] hardware_interrupt_common+0x114/0x180
Fix this issue by introducing a new rtas_get_sensor_fast() function
that does not use rtas_busy_delay() - and thus can only be used for
sensors that do not cause a BUSY condition (which should be the case
for the sensor that is queried by the EPOW IRQ handler).
Signed-off-by: Thomas Huth th...@redhat.com
---
arch/powerpc/include/asm/rtas.h | 1 +
arch/powerpc/kernel/rtas.c | 17 +
arch/powerpc/platforms/pseries/ras.c | 3 ++-
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 7a4ede1..b77ef36 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -343,6 +343,7 @@ extern void rtas_power_off(void);
extern void rtas_halt(void);
extern void rtas_os_term(char *str);
extern int rtas_get_sensor(int sensor, int index, int *state);
+extern int rtas_get_sensor_fast(int sensor, int index, int *state);
extern int rtas_get_power_level(int powerdomain, int *level);
extern int rtas_set_power_level(int powerdomain, int level, int *setlevel);
extern bool rtas_indicator_present(int token, int *maxindex);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 7a488c1..caffb10 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -584,6 +584,23 @@ int rtas_get_sensor(int sensor, int index, int *state)
}
EXPORT_SYMBOL(rtas_get_sensor);
+int rtas_get_sensor_fast(int sensor, int index, int *state)
+{
+ int token = rtas_token(get-sensor-state);
+ int rc;
+
+ if (token == RTAS_UNKNOWN_SERVICE)
+ return -ENOENT;
+
+ rc = rtas_call(token, 2, 2, state, sensor, index);
+ WARN_ON(rc == RTAS_BUSY || (rc = RTAS_EXTENDED_DELAY_MIN
+ rc = RTAS_EXTENDED_DELAY_MAX));
+
+ if (rc 0)
+ return rtas_error_rc(rc);
+ return rc;
+}
+
bool rtas_indicator_present(int token, int *maxindex)
{
int proplen, count, i;
diff --git a/arch/powerpc/platforms/pseries/ras.c
b/arch/powerpc/platforms/pseries/ras.c
index 02e4a17..3b6647e 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -189,7 +189,8 @@ static irqreturn_t ras_epow_interrupt(int irq, void *dev_id)
int state;
int critical;
- status = rtas_get_sensor(EPOW_SENSOR_TOKEN, EPOW_SENSOR_INDEX, state);
+ status = rtas_get_sensor_fast(EPOW_SENSOR_TOKEN, EPOW_SENSOR_INDEX,
+ state);
if (state 3)
critical = 1; /* Time Critical */
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
