Re: 2.6.29-rc1 does not boot

Mike Travis
2009-01-11T19:14:59Z

Ingo Molnar wrote:
> * Dieter Ries <clip2@gmx.de> wrote:
>
>> Bisected it:
>>
>> ####################################################################
>> 7503bfbae89eba07b46441a5d1594647f6b8ab7d is first bad commit
>> commit 7503bfbae89eba07b46441a5d1594647f6b8ab7d
>> Author: Mike Travis <travis@sgi.com>
>> Date: Sun Jan 4 05:18:09 2009 -0800
>>
>> cpumask: use work_on_cpu in acpi-cpufreq.c for drv_read and drv_write
>>
>> Impact: use new cpumask API to reduce stack usage
>
> thanks, this is very helpful!
>
> Mike, most of the work_on_cpu() patches you did so far were rather
> problematic. Especially something like cpufreq can run rather early during
> bootup or during suspend/resume, so i'm not sure it's correct to rely on
> keventd for it.
>
> I dont see anything particularly wrong in the commit itself - but
> obviously it causes this boot hang - if the bug is not found we'll revert
> it .

All of these are low use functions, primarily used when bringing up
cpus. So reverting the patches does not have a big effect on the stack
size problem.
>
> Also, this bit in get_cur_val():
>
> + if (unlikely(!alloc_cpumask_var(&cmd.mask, GFP_KERNEL)))
> + return 0;
>
> how is that supposed to work? If we fail to allocate a cpumask we just
> ignore the call silently? That cannot be right. (but has no connection to
> this boot problem)
>
> Ingo

Well I did have a different approach but Rusty seemed to really be
attached to work_on_cpu. (The alternate was to add a 2nd cpumask to the
task struct to hold current->cpus_allowed while setting it to a special
cpus_allowed mask.)

In any case, except for the get_online_cpus() call, work_on_cpu is a
fairly straight forward approach. But I'm just not familiar enough
with the whole locking scheme to determine whether the cpu hotplug lock
has already been taken, which is causing this weird lockdep warning.
And I don't know of an adaptive way to do it (figure out in work_on_cpu()
if get_online_cpus() should be called or not.)

About the return 0, it was the default return for another error case.
Should the function panic because it can't read a cpu reg? That seems
wrong too.

Thanks,
Mike