Messages in this thread | | | Date | Mon, 12 Jan 2009 19:55:46 +0100 | From | Ingo Molnar <> | Subject | Re: 2.6.29-rc1 does not boot |
| |
* Mike Travis <travis@sgi.com> wrote:
> Ingo Molnar wrote: > > * Mike Travis <travis@sgi.com> wrote: > ... > >> Rusty - any ideas on how to avoid these clashes with the > >> get_online_cpus() call in work_on_cpu()? Or something else to indicate > >> to lockdep that the circular lock dependency is ok (as you mentioned > >> before)? > > > > I've queued up the revert below, please check the commit message whether > > you agree with the analysis. > > > > Mike, could you also check any other patches where you add work_on_cpu() > > usage to make sure we dont have similar mishaps? work_on_cpu() seems > > completely unsuited for any sort of set_cpus_allowed() replacement ... > > > > Ingo > > Yes, I'll do that now. With the resume feature also calling these functions, > I'm even less comfortable with it. > > Shall I resurrect the 2nd cpumask in the task struct from my original patches, > (and one that akpm also suggested more than a year ago)? > > Basically, it looks like this: > > --- linux-2.6-for-ingo.orig/include/linux/sched.h 2009-01-11 10:43:19.000000000 -0800 > +++ linux-2.6-for-ingo/include/linux/sched.h 2009-01-12 09:45:02.871247038 -0800 > @@ -1132,6 +1132,7 @@ struct task_struct { > > unsigned int policy; > cpumask_t cpus_allowed; > + cpumask_t save_cpus_allowed; > > --- linux-2.6-for-ingo.orig/arch/x86/kernel/microcode_core.c 2009-01-12 09:05:36.000000000 -0800 > +++ linux-2.6-for-ingo/arch/x86/kernel/microcode_core.c 2009-01-12 09:49:19.315276144 -0800 > @@ -110,11 +110,10 @@ EXPORT_SYMBOL_GPL(ucode_cpu_info); > #ifdef CONFIG_MICROCODE_OLD_INTERFACE > static int do_microcode_update(const void __user *buf, size_t size) > { > - cpumask_t old; > int error = 0; > int cpu; > > - old = current->cpus_allowed; > + cpumask_copy(¤t->save_cpus_allowed, ¤t->cpus_allowed); > > for_each_online_cpu(cpu) { > struct ucode_cpu_info *uci = ucode_cpu_info + cpu; > @@ -122,7 +121,7 @@ static int do_microcode_update(const voi > if (!uci->valid) > continue; > > - set_cpus_allowed_ptr(current, &cpumask_of_cpu(cpu)); > + set_cpus_allowed_ptr(current, cpumask_of(cpu)); > error = microcode_ops->request_microcode_user(cpu, buf, size); > if (error < 0) > goto out; > @@ -130,7 +129,7 @@ static int do_microcode_update(const voi > microcode_ops->apply_microcode(cpu); > } > out: > - set_cpus_allowed_ptr(current, &old); > + set_cpus_allowed_ptr(current, ¤t->save_cpus_allowed); > return error; > } > > The primary concern is that there is only one temp, so I had also put in > a warning if it was already in use. But the scope of where it's used is > very short-lived, so I don't know if a preempt_disable() is required, > but it seems the safe thing to do.
that's rather fragile. Fix work_on_cpu() instead? Why does it need to take the CPU-hotplug lock?
Ingo
| |