Skip to content

Commit

Permalink
Revert "cpumask: use work_on_cpu in acpi-cpufreq.c for drv_read and d…
Browse files Browse the repository at this point in the history
…rv_write"

This reverts commit 7503bfb.

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>
  • Loading branch information
Ingo Molnar committed Jan 12, 2009
1 parent 2bc1379 commit 50c668d
Showing 1 changed file with 12 additions and 13 deletions.
25 changes: 12 additions & 13 deletions arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)
Expand All @@ -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;
Expand Down

0 comments on commit 50c668d

Please sign in to comment.