On Thu, May 24, 2007 at 01:29:02AM +0400, Oleg Nesterov wrote: > Cliff Wickman wrote: > > > > In order to do this, move_task_off_dead_cpu() must make a call to > > cpuset_cpus_allowed(), which may block. > > > > move_task_off_dead_cpu() has been within a critical region when called > > from migrate_live_tasks(). So this patch also changes migrate_live_tasks() > > to enable interrupts before calling move_task_off_dead_cpu(). > > Since the tasklist_lock is dropped, the list scan must be restarted from > > the top. > > > > [... snip ...] > > > > - * NOTE: interrupts should be disabled by the caller > > + * NOTE: interrupts are not disabled by the caller > > */ > > static void move_task_off_dead_cpu(int dead_cpu, struct task_struct *p) > > { > > @@ -5008,6 +5008,17 @@ restart: > > if (dest_cpu == NR_CPUS) > > dest_cpu = any_online_cpu(p->cpus_allowed); > > > > + /* try to stay on the same cpuset */ > > + if (dest_cpu == NR_CPUS) { > > + /* > > + * Call to cpuset_cpus_allowed may sleep, so we depend > > + * on move_task_off_dead_cpu() being called in a non-critical > > + * region. > > + */ > > + p->cpus_allowed = cpuset_cpus_allowed(p); > > + dest_cpu = any_online_cpu(p->cpus_allowed); > > + } > > I know nothing about cpuset.c, a _very_ naive question.
Paul Jackson is the cpuset guru. > Do we really need task_lock() (used by cpuset_cpus_allowed) here ? According to Paul's comment in kernel/cpuset.c * It is ok to first take manage_sem, then nest callback_sem. We also * require taking task_lock() when dereferencing a tasks cpuset pointer. So I'm afraid it is not safe to call guarantee_online_cpus(tsk->cpuset, &mask); without it. Could the task not be exiting? > If not, probably we can make this simpler. CPU_DEAD takes cpuset_lock(), > move_task_off_dead_cpu() uses guarantee_online_cpus() which doesn't sleep, > so we don't need other changes. > > Possible? > > If not, this patch should also change migrate_dead(), it still calls > move_task_off_dead_cpu() with irqs disabled, no? Right, the lock is released but I indeed didn't reenable irqs. How would you suggest doing that? The irq state was saved in local variable "flags" back in migration_call(). > > Oleg. -- Cliff Wickman Silicon Graphics, Inc. [EMAIL PROTECTED] (651) 683-3824 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/