Re: 2.6.29-rc1 does not boot

[Mike Travis]

Ingo Molnar wrote:
> * Mike Travis <travis@xxxxxxx> 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(&current->save_cpus_allowed, &current->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, &current->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.

THanks,
MIke

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/