Commit b9d10be7a8e88fdcb12540387c219cdde87b0795

Authored by Toshi Kani
Committed by Rafael J. Wysocki
1 parent 22e7551eb6

ACPI / processor: Acquire writer lock to update CPU maps

CPU system maps are protected with reader/writer locks.  The reader
lock, get_online_cpus(), assures that the maps are not updated while
holding the lock.  The writer lock, cpu_hotplug_begin(), is used to
udpate the cpu maps along with cpu_maps_update_begin().

However, the ACPI processor handler updates the cpu maps without
holding the the writer lock.

acpi_map_lsapic() is called from acpi_processor_hotadd_init() to
update cpu_possible_mask and cpu_present_mask.  acpi_unmap_lsapic()
is called from acpi_processor_remove() to update cpu_possible_mask.
Currently, they are either unprotected or protected with the reader
lock, which is not correct.

For example, the get_online_cpus() below is supposed to assure that
cpu_possible_mask is not changed while the code is iterating with
for_each_possible_cpu().

        get_online_cpus();
        for_each_possible_cpu(cpu) {
		:
        }
        put_online_cpus();

However, this lock has no protection with CPU hotplug since the ACPI
processor handler does not use the writer lock when it updates
cpu_possible_mask.  The reader lock does not serialize within the
readers.

This patch protects them with the writer lock with cpu_hotplug_begin()
along with cpu_maps_update_begin(), which must be held before calling
cpu_hotplug_begin().  It also protects arch_register_cpu() /
arch_unregister_cpu(), which creates / deletes a sysfs cpu device
interface.  For this purpose it changes cpu_hotplug_begin() and
cpu_hotplug_done() to global and exports them in cpu.h.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Showing 3 changed files with 23 additions and 11 deletions Side-by-side Diff

drivers/acpi/acpi_processor.c
... ... @@ -178,14 +178,17 @@
178 178 if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
179 179 return -ENODEV;
180 180  
  181 + cpu_maps_update_begin();
  182 + cpu_hotplug_begin();
  183 +
181 184 ret = acpi_map_lsapic(pr->handle, &pr->id);
182 185 if (ret)
183   - return ret;
  186 + goto out;
184 187  
185 188 ret = arch_register_cpu(pr->id);
186 189 if (ret) {
187 190 acpi_unmap_lsapic(pr->id);
188   - return ret;
  191 + goto out;
189 192 }
190 193  
191 194 /*
... ... @@ -195,7 +198,11 @@
195 198 */
196 199 pr_info("CPU%d has been hot-added\n", pr->id);
197 200 pr->flags.need_hotplug_init = 1;
198   - return 0;
  201 +
  202 +out:
  203 + cpu_hotplug_done();
  204 + cpu_maps_update_done();
  205 + return ret;
199 206 }
200 207 #else
201 208 static inline int acpi_processor_hotadd_init(struct acpi_processor *pr)
202 209  
203 210  
... ... @@ -452,11 +459,15 @@
452 459 per_cpu(processor_device_array, pr->id) = NULL;
453 460 per_cpu(processors, pr->id) = NULL;
454 461  
  462 + cpu_maps_update_begin();
  463 + cpu_hotplug_begin();
  464 +
455 465 /* Remove the CPU. */
456   - get_online_cpus();
457 466 arch_unregister_cpu(pr->id);
458 467 acpi_unmap_lsapic(pr->id);
459   - put_online_cpus();
  468 +
  469 + cpu_hotplug_done();
  470 + cpu_maps_update_done();
460 471  
461 472 try_offline_node(cpu_to_node(pr->id));
462 473  
... ... @@ -172,6 +172,8 @@
172 172 #ifdef CONFIG_HOTPLUG_CPU
173 173 /* Stop CPUs going up and down. */
174 174  
  175 +extern void cpu_hotplug_begin(void);
  176 +extern void cpu_hotplug_done(void);
175 177 extern void get_online_cpus(void);
176 178 extern void put_online_cpus(void);
177 179 extern void cpu_hotplug_disable(void);
... ... @@ -197,6 +199,8 @@
197 199  
198 200 #else /* CONFIG_HOTPLUG_CPU */
199 201  
  202 +static inline void cpu_hotplug_begin(void) {}
  203 +static inline void cpu_hotplug_done(void) {}
200 204 #define get_online_cpus() do { } while (0)
201 205 #define put_online_cpus() do { } while (0)
202 206 #define cpu_hotplug_disable() do { } while (0)
... ... @@ -113,7 +113,7 @@
113 113 * get_online_cpus() not an api which is called all that often.
114 114 *
115 115 */
116   -static void cpu_hotplug_begin(void)
  116 +void cpu_hotplug_begin(void)
117 117 {
118 118 cpu_hotplug.active_writer = current;
119 119  
... ... @@ -127,7 +127,7 @@
127 127 }
128 128 }
129 129  
130   -static void cpu_hotplug_done(void)
  130 +void cpu_hotplug_done(void)
131 131 {
132 132 cpu_hotplug.active_writer = NULL;
133 133 mutex_unlock(&cpu_hotplug.lock);
... ... @@ -154,10 +154,7 @@
154 154 cpu_maps_update_done();
155 155 }
156 156  
157   -#else /* #if CONFIG_HOTPLUG_CPU */
158   -static void cpu_hotplug_begin(void) {}
159   -static void cpu_hotplug_done(void) {}
160   -#endif /* #else #if CONFIG_HOTPLUG_CPU */
  157 +#endif /* CONFIG_HOTPLUG_CPU */
161 158  
162 159 /* Need to know about CPUs going up/down? */
163 160 int __ref register_cpu_notifier(struct notifier_block *nb)