RE: 2.5.66-bk12: acpi_power_off: sleeping function called from il legal context

RE: 2.5.66-bk12: acpi_power_off: sleeping function called from il legal context

[Grover, Andrew]

> From: Andrew Morton [mailto:akpm@digeo.com] 
> Zwane Mwaikambo <zwane@linuxpower.ca> wrote:
> >
> > -	if (in_atomic())
> > +	if (in_atomic() || irqs_disabled())
> >  		timeout = 0;
> 
> Andy, why does the ACPI code have this test?
> 
> Is it to determine whether a caller of this functon is 
> currently holding a spinlock?  If so then it will only work 
> on a preemptible kernel.

No, see below.

> A non-preempt kernel will not increment preempt_count() when 
> it takes a spinlock and ACPI could mistakenly schedule away 
> and cause a system deadlock.

acpi_enter_sleep_state should not be acquiring any semaphores. All calls
to acpi_set_register in that function should be called with
ACPI_MTX_DO_NOT_LOCK. Problem solved. :)

Andrew, as to why we are doing this:

The main function of the ACPI interpreter is to execute control methods.
We never execute a control method from an interrupt, we always do it
from thread context. We have semaphores to protect various resources,
and use this function (acpi_os_wait_semaphore) to acquire them. We
usually call it with timeout value ACPI_WAIT_FOREVER, which results in a
down().

However, we also have to execute control methods early in the boot
sequence. down() would never block but it thinks it might, so we want to
call down_trylock instead. in_atomic() seemed to be a good (?) way to
tell whether we need to avoid down() or not.

Thoughts on better ways to do this, perhaps? I guess I should at least
add a comment above that line.

Regards -- Andy

Re: 2.5.66-bk12: acpi_power_off: sleeping function called from il legal context

[Andrew Morton]

"Grover, Andrew" <andrew.grover@intel.com> wrote:
>
> However, we also have to execute control methods early in the boot
> sequence. down() would never block but it thinks it might, so we want to
> call down_trylock instead. in_atomic() seemed to be a good (?) way to
> tell whether we need to avoid down() or not.
> 
> Thoughts on better ways to do this, perhaps? I guess I should at least
> add a comment above that line.
> 

So really it's just the debug code which is being misleading?  hm.

Couldn't you set some magical global ACPI flag:

acpi_super_early_init()
{
	acpi_in_super_early_init = 1;
	do_stuff();
	acpi_in_super_early_init = 0;
}

And test that flag in acpi_os_wait_semaphore()?

It's a bit grubby, but so is the problem.

We do have this `system_running' flags in init/main.c which perhaps should be
fleshed out into a more fine-grained way of communicating the kernel's
start/run/stop state.