On 21.11.2019 10:47, Julien Grall wrote: > On 20/11/2019 23:05, Chao Gao wrote: >> --- a/xen/arch/arm/smpboot.c >> +++ b/xen/arch/arm/smpboot.c >> @@ -39,6 +39,7 @@ >> cpumask_t cpu_online_map; >> cpumask_t cpu_present_map; >> cpumask_t cpu_possible_map; >> +cpumask_var_t cpu_parked_map; > > You define cpu_parked_map but AFAIK it will never get allocated. The > risk here is any access to that variable will result to a fault. > > Looking at the changes below, it looks like access in common code will > be protected by park_offline_cpus. This is always false on Arm, so the > compiler should remove any access to cpu_parked_map. > > With that in mind, I think it would be best to only provide a prototype > for cpu_parked_map and so the linker can warn if someone used it.
+1 In fact I wonder whether the maintenance of the map should live in common code in the first place. While clearing the respective bit in cpu_up() looks correct (and could be done without any if()), I'm not convinced the setting of the bit in cpu_down() is going to be correct in all cases. I'd rather leave this to the relevant callers of cpu_down(). To deal with cpu_add_remove_lock not being held perhaps it would be best to set the flag _before_ calling cpu_down(); consumers of the map then would need to double check that a CPU is not _also_ (still) online. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
