[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 1/2] x86/cpu: maintain a parked CPU bitmap
On 21.11.2019 12:43, Chao Gao wrote: > On Thu, Nov 21, 2019 at 11:02:10AM +0100, Jan Beulich wrote: >> 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 > > Will do. I added this because I am not sure all compilers would omit > such access. > >> >> 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()), > > But when park_offline_cpus() is false, the map isn't allocated. I don't > think it is safe to access the map in this case. Oh, you're right of course. Unless the map was allocated unconditionally ... >> I'm not convinced the setting of the bit in cpu_down() is going to >> be correct in all cases. > > Do you mean in some cases, cpu_down() is to really offline a CPU even > park_offline_cpus is set? And in this case, setting the bit isn't > correct. The purposes of cpu_down() calls _may_ be different. Plus whether there's parking wanted/necessary for an architecture should remain - as much as possible - an architecture thing to deal with. I.e. despite park_offline_cpus being used in common code, I think we should strive to avoid adding more there when it can be avoided. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |