Re: 2.6.29-rc1 does not boot and fails to resume

Rafael J. Wysocki
2009-01-12T17:22:42Z

On Monday 12 January 2009, Ingo Molnar wrote:
>
> * Maciej Rutecki <maciej.rutecki@gmail.com> wrote:
>
> > 2009/1/11 Dieter Ries <clip2@gmx.de>:
> > > Hi,
> > >
> > > Ingo Molnar schrieb:
> > >>>> * Dieter Ries <clip2@gmx.de> wrote:
> > >>>>
> > >>>>> Hi,
> > >>>>>
> > >>>>> I just pulled 2.6.29-rc1, ran oldconfig with defaults and built it.
> > >>>>> When I try to boot it, that kind of works until init should start. Then
> > >>>>> nothing happens. I tried with init=/bin/bash, which sometimes works, and
> > >>>>> sometimes gets me a bash without the prompt flashing.
> > >>>>>
> >
> > Simmilar issue, described:
> > http://lkml.indiana.edu/hypermail/linux/kernel/0901.1/01701.html
> >
> > [...]
> > ####################################################################
> > > 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
> > >
> > > Replace the saving of current->cpus_allowed and set_cpus_allowed_ptr()
> > > with a work_on_cpu function for drv_read() and drv_write().
> > >
> > > Basically converts do_drv_{read,write} into "work_on_cpu" functions that
> > > are now called by drv_read and drv_write.
> > >
> > > Signed-off-by: Mike Travis <travis@sgi.com>
> > > Acked-by: Rusty Russell <rusty@rustcorp.com.au>
> > > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> >
> > Revert this commit also solves problem on my laptoop.
>
> yes - the revert patch can be found below.

Just for the record, apart from the boot problems the commit the patch below
reverts is also causing a suspend-resume regression for people.

Thanks,
Rafael

> ------------------->
> From e0b7a3bea054249b27ca3c843bf6eefcb509d1c2 Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <mingo@elte.hu>
> Date: Mon, 12 Jan 2009 10:49:53 +0100
> Subject: [PATCH] Revert "cpumask: use work_on_cpu in acpi-cpufreq.c for drv_read and drv_write"
>
> This reverts commit 7503bfbae89eba07b46441a5d1594647f6b8ab7d.
>
> Dieter Ries reported bootup soft-hangs and bisected it back to
> this commit, and reverting this commit gave him a working system.
>
> The commit introduces work_on_cpu() use into the cpufreq code,
> but that is subtly problematic from a lock hierarchy POV: the
> hotplug-cpu lock is an highlevel lock that is taken before
> lowlevel locks, and in this codepath we are called with the
> policy lock taken.
>
> Dieter did not have lockdep enabled so we dont have a nice stack
> trace proof for this, but using work_on_cpu() in such a lowlevel
> place certainly looks wrong, so we revert the patch.
>
> work_on_cpu() needs to be reworked to be more generally usable.
>
> Reported-by: Dieter Ries <clip2@gmx.de>
> Tested-by: Dieter Ries <clip2@gmx.de>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
> arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c | 25 ++++++++++++-------------
> 1 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 06fcd8f..6f11e02 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -150,9 +150,8 @@ struct drv_cmd {
> u32 val;
> };
>
> -static long do_drv_read(void *_cmd)
> +static void do_drv_read(struct drv_cmd *cmd)
> {
> - struct drv_cmd *cmd = _cmd;
> u32 h;
>
> switch (cmd->type) {
> @@ -167,12 +166,10 @@ static long do_drv_read(void *_cmd)
> default:
> break;
> }
> - return 0;
> }
>
> -static long do_drv_write(void *_cmd)
> +static void do_drv_write(struct drv_cmd *cmd)
> {
> - struct drv_cmd *cmd = _cmd;
> u32 lo, hi;
>
> switch (cmd->type) {
> @@ -189,23 +186,30 @@ static long do_drv_write(void *_cmd)
> default:
> break;
> }
> - return 0;
> }
>
> static void drv_read(struct drv_cmd *cmd)
> {
> + cpumask_t saved_mask = current->cpus_allowed;
> cmd->val = 0;
>
> - work_on_cpu(cpumask_any(cmd->mask), do_drv_read, cmd);
> + set_cpus_allowed_ptr(current, cmd->mask);
> + do_drv_read(cmd);
> + set_cpus_allowed_ptr(current, &saved_mask);
> }
>
> static void drv_write(struct drv_cmd *cmd)
> {
> + cpumask_t saved_mask = current->cpus_allowed;
> unsigned int i;
>
> for_each_cpu(i, cmd->mask) {
> - work_on_cpu(i, do_drv_write, cmd);
> + set_cpus_allowed_ptr(current, cpumask_of(i));
> + do_drv_write(cmd);
> }
> +
> + set_cpus_allowed_ptr(current, &saved_mask);
> + return;
> }
>
> static u32 get_cur_val(const struct cpumask *mask)
> @@ -231,15 +235,10 @@ static u32 get_cur_val(const struct cpumask *mask)
> return 0;
> }
>
> - if (unlikely(!alloc_cpumask_var(&cmd.mask, GFP_KERNEL)))
> - return 0;
> -
> cpumask_copy(cmd.mask, mask);
>
> drv_read(&cmd);
>
> - free_cpumask_var(cmd.mask);
> -
> dprintk("get_cur_val = %u\n", cmd.val);
>
> return cmd.val;
> --