On 10/12/2016 06:06 PM, Tim Chen wrote: > On Wed, 2016-10-12 at 16:54 -0400, Prarit Bhargava wrote: >> >> On 10/12/2016 03:51 PM, Tim Chen wrote: >>> >>> On Wed, 2016-10-12 at 11:06 -0400, Prarit Bhargava wrote: >>>> >>>> >>>> On 10/12/2016 10:36 AM, Prarit Bhargava wrote: >>>>> >>>>> >>>>> This was noticed during the investigation of >>>>> >>>>> http://git.kernel.org/tip/2a51fe083eba7f99cbda72f5ef90cdf2f4df882c >>>>> >>>>> A note for reviewers on the cleanup in smp_init_package_map(): The >>>>> per_cpu variable x86_bios_cpu_apicid is initialized to BAD_APICID, and the >>>>> bitmaps are set to false by default so the code isn't necessary. The >>>>> pr_warn() can also be dropped as generic_processor_info() will do a >>>>> pr_warning() in the same situation. >>>>> >>>>> >>>>> >>>>> P. >>>>> >>>>> -----8<----- >>>>> >>>>> After commit 1f12e32f4cd5 ("x86/topology: Create logical package id"), >>>>> topology_update_package_map() is called twice on a CPU. The first call >>>>> is in >>>>> generic_processor_info(), which is called on all present cpus (found in >>>>> either >>>>> mptable or ACPI MADT). The second call is in smp_init_package_map() which >>>>> is called later on in the init sequence. >>>>> >>>>> Only a single call is necessary for the kernel to initialize the data >>>>> structures properly. This patch drops the later call in >>>>> smp_init_package_map() >>>>> and moves smp_init_package_map() out of smp_store_boot_cpu_info(). >>>>> >>>> Self-NAK. >>>> >>>> This seems to not work on systems with an empty socket :/. Looking into >>>> this now. >>>> >>>> P. >>> My single node workstation also failed to boot with the patch. >> The patch is incorrect. I'm dropping it completely. >> > > The call from generic_processor_info to topology_update_package_map > during early boot was essentially no op, as physical_package_map has > not have been allocated by smp_init_package_map yet. > > We have to call topology_update_package_map in smp_init_package_map > after allocating physical_package_map to properly set up > physical_package_map.
Right -- the patch is incorrect (see above). I'm dropping it completely. P. > > Tim >
