RE: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)

RE: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)

[Protasevich, Natalie]

> > +#ifdef CONFIG_HOTPLUG_CPU
> > +	if (cpu_online(cpu)) {
> > +#else
> >  	if (cpu_online(cpu) || !cpu_present(cpu)) {
> > +#endif
> >  		ret = -EINVAL;
> >  		goto out;
> >  	}
> 
> Why this change?  I think the cpu_present check is needed for 
> ppc64 since it has non-present cpus in sysfs.
> 

The new processor was never brought up, its bit is only set in
cpu_possible_map, but not in present map. 

        ... 
        if (boot_error) {
                /* Try to put things back the way they were before ...
*/
                unmap_cpu_to_logical_apicid(cpu);
                cpu_clear(cpu, cpu_callout_map); /* was set here
(do_boot_cpu()) */
                cpu_clear(cpu, cpu_initialized); /* was set by
cpu_init() */
                cpucount--;
        } else {
                x86_cpu_to_apicid[cpu] = apicid;
                cpu_set(cpu, cpu_present_map);  <=====================
        }
        ...
So if someone tries to boot a CPU up first time during runtime it always
fails this check.
Thanks,
--Natalie

Re: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)

[Nathan Lynch]

Protasevich, Natalie wrote:
> > > +#ifdef CONFIG_HOTPLUG_CPU
> > > +	if (cpu_online(cpu)) {
> > > +#else
> > >  	if (cpu_online(cpu) || !cpu_present(cpu)) {
> > > +#endif
> > >  		ret = -EINVAL;
> > >  		goto out;
> > >  	}
> > 
> > Why this change?  I think the cpu_present check is needed for 
> > ppc64 since it has non-present cpus in sysfs.
> > 
> 
> The new processor was never brought up, its bit is only set in
> cpu_possible_map, but not in present map. 

If a cpu is physically present and is capable of being onlined it
should be marked in cpu_present_map, please.  This change would break
ppc64; can you rework it?

RE: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)

[Protasevich, Natalie]

> Protasevich, Natalie wrote:
> > > > +#ifdef CONFIG_HOTPLUG_CPU
> > > > +	if (cpu_online(cpu)) {
> > > > +#else
> > > >  	if (cpu_online(cpu) || !cpu_present(cpu)) {
> > > > +#endif
> > > >  		ret = -EINVAL;
> > > >  		goto out;
> > > >  	}
> > > 
> > > Why this change?  I think the cpu_present check is needed for
> > > ppc64 since it has non-present cpus in sysfs.
> > > 
> > 
> > The new processor was never brought up, its bit is only set in 
> > cpu_possible_map, but not in present map.
> 
> If a cpu is physically present and is capable of being 
> onlined it should be marked in cpu_present_map, please.  This 
> change would break ppc64; can you rework it?
>
OK, that has to be reworked then. That's what probably Shaohua meant
when mentioned that cpu_present_map has to be cleaned up... 
Thanks,
--Natalie

Re: [Hotplug_sig] [patch 1/1] Hot plug CPU to support physical add of new processors (i386)

[Nathan Lynch]

Natalie.Protasevich@unisys.com wrote:

> +#ifdef CONFIG_HOTPLUG_CPU
> +	if (cpu_online(cpu)) {
> +#else
>  	if (cpu_online(cpu) || !cpu_present(cpu)) {
> +#endif
>  		ret = -EINVAL;
>  		goto out;
>  	}

Why this change?  I think the cpu_present check is needed for ppc64
since it has non-present cpus in sysfs.