lkml.org 
[lkml]   [2009]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 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(&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.

    that's rather fragile. Fix work_on_cpu() instead? Why does it need to take
    the CPU-hotplug lock?

    Ingo


    \
     
     \ /
      Last update: 2009-01-12 19:59    [W:2.701 / U:0.260 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site