On Wed, Sep 12, 2007 at 12:20:47PM -0600, Alex Williamson wrote:
> On Wed, 2007-09-12 at 17:08 +0900, Simon Horman wrote:
> >
> > + /* Ensure that we are not runnable on dying cpu */
> > + /* This is current->cpus_allowed on Linux,
> > + * which may well work completely differently */
> > + old_affinity = current->cpu_affinity;
> > + tmp = (cpumask_t)CPU_MASK_ALL;
> > + cpu_clear(cpu, tmp);
>
> Is it just me, or does this not do anything at all?
The old_affinity line certainly does nothing.
Its part of some unwind code from the linux version of
the code, but the xen version ended up not having
any unwind on error code.
And for different reasons the other lines also do nothing.
I'll #ifndef XEN them out for now.
But what I am not clear about is if we should be doing something here.
The Linux code is as follows, but set_cpus_allowed() doesn't exist
on Xen.
Linux 2.6.23-rc6:kernel/cpu.c:_cpu_down()
/* Ensure that we are not runnable on dying cpu */
old_allowed = current->cpus_allowed;
tmp = CPU_MASK_ALL;
cpu_clear(cpu, tmp);
set_cpus_allowed(current, tmp);
>
> > Index: xen-unstable.hg/xen/include/asm-ia64/linux-xen/linux/cpu.h
>
> README.origin in this directory also needs to be updated.
>
> > ===================================================================
> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> > +++ xen-unstable.hg/xen/include/asm-ia64/linux-xen/linux/cpu.h
> > 2007-08-08 17:59:07.000000000 +0900
> ...
> > --- xen-unstable.hg.orig/xen/include/asm-ia64/linux/asm/sal.h
> > 2007-08-08 17:51:34.000000000 +0900
> > +++ xen-unstable.hg/xen/include/asm-ia64/linux/asm/sal.h 2007-08-08
> > 17:59:07.000000000 +0900
> > @@ -856,7 +856,8 @@ extern int ia64_sal_oemcall_nolock(struc
> > u64, u64, u64, u64, u64);
> > extern int ia64_sal_oemcall_reentrant(struct ia64_sal_retval *, u64, u64,
> > u64,
> > u64, u64, u64, u64, u64);
> > -#ifdef CONFIG_HOTPLUG_CPU
> > +
> > +#if CONFIG_HOTPLUG_CPU
>
> Why? The next chunk of code users #ifdef again.
Sorry, I think that is just an artifact of my development process.
I'll remove that change.
>
> > +static void fix_b0_for_bsp(void)
> > +{
> > +#ifdef CONFIG_HOTPLUG_CPU
> > + int cpuid;
> > + static int fix_bsp_b0 = 1;
> > +
> > + cpuid = smp_processor_id();
> > +
> > + /*
> > + * Cache the b0 value on the first AP that comes up
> > + */
> > + if (!(fix_bsp_b0 && cpuid))
> > + return;
The following to lines are missing, which as you point out
results in a useless function:
sal_boot_rendez_state[0].br[0] = sal_boot_rendez_state[cpuid].br[0];
printk ("Fixed BSP b0 value from CPU %d\n", cpuid);
I think that this "actually does something" version is needed so
that kexec (and more importantly kdump) can work in the case where
it is an AP that intiates the Kexec (or crashes and initiates Kdump).
I'm yet to work out a good test case for this.
> > +
> > + fix_bsp_b0 = 0;
> > +#endif
> > +}
>
> I'm hoping there's another patch that I haven't read yet that makes
> this function useful.
--
Horms
H: http://www.vergenet.net/~horms/
W: http://www.valinux.co.jp/en/
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home