Re: [PATCH] reorg RTAS delay code
From: Nathan Lynch <hidden>
Date: 2006-07-13 18:20:37
Hi folks- John Rose wrote:
This patch attempts to handle RTAS "busy" return codes in a more simple and consistent manner. Typical callers of RTAS shouldn't have to manage wait times and delay calls. This patch also changes the kernel to use msleep() rather than udelay() when a runtime delay is necessary. This will avoid CPU soft lockups for extended delay conditions.
...
+/* For an RTAS busy status code, perform the hinted delay. */
+unsigned int rtas_busy_delay(int status)
+{
+ unsigned int ms;
- /* Use microseconds for reasonable accuracy */
- for (ms = 1; order > 0; order--)
- ms *= 10;
+ might_sleep();
+ ms = rtas_busy_delay_time(status);
+ if (ms)
+ msleep(ms);
- return ms;
+ return ms;
}
So I managed to test this with cpu hotplug recently and the
might_sleep warning triggers in the cpu offline path (I had to
reconstruct this from xmon due to the kernel crashing later on for a
different reason, so it might be a little off):
BUG: sleeping function called from invalid context at arch/powerpc/kernel/rtas.c:463.
in_atomic():1, irqs_disabled():1.
Call Trace:
[C0000000AAD379A0] [C000000000010ADC] .show_stack+0x68/0x1b4 (unreliable)
[C0000000AAD37A50] [C000000000050C98] .__might_sleep+0xd0/0xec
[C0000000AAD37AE0] [C00000000001DF5C] .rtas_busy_delay+0x20/0x54
[C0000000AAD37B70] [C00000000001E2D0] .rtas_set_indicator+0x70/0xd4
[C0000000AAD37C10] [C0000000000491C8] .xics_migrate_irqs_away+0x78/0x228
[C0000000AAD37CD0] [C000000000047C14] .pSeries_cpu_disable+0x98/0xb4
[C0000000AAD37D50] [C000000000029A5C] .__cpu_disable+0x4c/0x60
[C0000000AAD37DC0] [C000000000080834] .take_cpu_down+0x10/0x38
[C0000000AAD37E40] [C00000000008D1C0] .do_stop+0x198/0x248
[C0000000AAD37EE0] [C000000000078124] .kthread+0x124/0x174
[C0000000AAD37F90] [C000000000026568] .kernel_thread+0x4c/0x68
As it turns out, set-indicator is not allowed to return a busy delay
in this context, so we're actually safe here. Maybe we need an
rtas_set_indicator_fast which could take that into account, e.g.
int rtas_set_indicator_fast(int indicator, int index, int new_value)
{
int token = rtas_token("set-indicator");
int rc;
rc = rtas_call(token, 3, 1, NULL, indicator, index, new_value);
WARN_ON(rc == -2 || rc >= 9000 || rc <= 9005);
if (rc < 0)
return rtas_error_rc(rc);
return rc;
}