Memory hotplug / ACPI: Simplify memory removal

Now that the memory offlining should be taken care of by the companion device offlining code in acpi_scan_hot_remove(), the ACPI memory hotplug driver doesn't need to offline it in remove_memory() any more. Moreover, since the return value of remove_memory() is not used, it's better to make it be a void function and trigger a BUG() if the memory scheduled for removal is not offline. Change the code in accordance with the above observations. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Toshi Kani <toshi.kani@hp.com>

Memory hotplug / ACPI: Simplify memory removal
Now that the memory offlining should be taken care of by the companion device offlining code in acpi_scan_hot_remove(), the ACPI memory hotplug driver doesn't need to offline it in remove_memory() any more. Moreover, since the return value of remove_memory() is not used, it's better to make it be a void function and trigger a BUG() if the memory scheduled for removal is not offline. Change the code in accordance with the above observations. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Toshi Kani <toshi.kani@hp.com>
Rafael J. Wysocki
1 parent 303bfdb1a1
Showing 3 changed files with 12 additions and 74 deletions Side-by-side Diff
drivers/acpi/acpi_memhotplug.c
include/linux/memory_hotplug.h
mm/memory_hotplug.c
@@ -271,14 +271,12 @@
 	return 0;
 }
  
-static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
+static void acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
 {
 	acpi_handle handle = mem_device->device->handle;
-	int result = 0, nid;
 	struct acpi_memory_info *info, *n;
+	int nid = acpi_get_node(handle);
  
-	nid = acpi_get_node(handle);
-
 	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
 		if (!info->enabled)
 			continue;
  
@@ -287,15 +285,10 @@
 			nid = memory_add_physaddr_to_nid(info->start_addr);
  
 		acpi_unbind_memory_blocks(info, handle);
-		result = remove_memory(nid, info->start_addr, info->length);
-		if (result)
-			return result;
-
+		remove_memory(nid, info->start_addr, info->length);
 		list_del(&info->list);
 		kfree(info);
 	}
-
-	return result;
 }
  
 static void acpi_memory_device_free(struct acpi_memory_device *mem_device)
@@ -252,7 +252,7 @@
 extern int arch_add_memory(int nid, u64 start, u64 size);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
 extern bool is_memblock_offlined(struct memory_block *mem);
-extern int remove_memory(int nid, u64 start, u64 size);
+extern void remove_memory(int nid, u64 start, u64 size);
 extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn,
 								int nr_pages);
 extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms);
@@ -1670,24 +1670,6 @@
 }
  
 #ifdef CONFIG_MEMORY_HOTREMOVE
-/**
- * offline_memory_block_cb - callback function for offlining memory block
- * @mem: the memory block to be offlined
- * @arg: buffer to hold error msg
- *
- * Always return 0, and put the error msg in arg if any.
- */
-static int offline_memory_block_cb(struct memory_block *mem, void *arg)
-{
-	int *ret = arg;
-	int error = device_offline(&mem->dev);
-
-	if (error != 0 && *ret == 0)
-		*ret = error;
-
-	return 0;
-}
-
 static int is_memblock_offlined_cb(struct memory_block *mem, void *arg)
 {
 	int ret = !is_memblock_offlined(mem);
  
  
  
  
  
@@ -1813,54 +1795,22 @@
 }
 EXPORT_SYMBOL(try_offline_node);
  
-int __ref remove_memory(int nid, u64 start, u64 size)
+void __ref remove_memory(int nid, u64 start, u64 size)
 {
-	unsigned long start_pfn, end_pfn;
-	int ret = 0;
-	int retry = 1;
+	int ret;
  
-	start_pfn = PFN_DOWN(start);
-	end_pfn = PFN_UP(start + size - 1);
-
-	/*
-	 * When CONFIG_MEMCG is on, one memory block may be used by other
-	 * blocks to store page cgroup when onlining pages. But we don't know
-	 * in what order pages are onlined. So we iterate twice to offline
-	 * memory:
-	 * 1st iterate: offline every non primary memory block.
-	 * 2nd iterate: offline primary (i.e. first added) memory block.
-	 */
-repeat:
-	walk_memory_range(start_pfn, end_pfn, &ret,
-			  offline_memory_block_cb);
-	if (ret) {
-		if (!retry)
-			return ret;
-
-		retry = 0;
-		ret = 0;
-		goto repeat;
-	}
-
 	lock_memory_hotplug();
  
 	/*
-	 * we have offlined all memory blocks like this:
-	 *   1. lock memory hotplug
-	 *   2. offline a memory block
-	 *   3. unlock memory hotplug
-	 *
-	 * repeat step1-3 to offline the memory block. All memory blocks
-	 * must be offlined before removing memory. But we don't hold the
-	 * lock in the whole operation. So we should check whether all
-	 * memory blocks are offlined.
+	 * All memory blocks must be offlined before removing memory.  Check
+	 * whether all memory blocks in question are offline and trigger a BUG()
+	 * if this is not the case.
 	 */
-
-	ret = walk_memory_range(start_pfn, end_pfn, NULL,
+	ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL,
 				is_memblock_offlined_cb);
 	if (ret) {
 		unlock_memory_hotplug();
-		return ret;
+		BUG();
 	}
  
 	/* remove memmap entry */
  
@@ -1871,18 +1821,13 @@
 	try_offline_node(nid);
  
 	unlock_memory_hotplug();
-
-	return 0;
 }
 #else
 int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 {
 	return -EINVAL;
 }
-int remove_memory(int nid, u64 start, u64 size)
-{
-	return -EINVAL;
-}
+void remove_memory(int nid, u64 start, u64 size) {}
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 EXPORT_SYMBOL_GPL(remove_memory);
...	...	@@ -271,14 +271,12 @@
271	271	return 0;
272	272	}
273	273
274		-static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
	274	+static void acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
275	275	{
276	276	acpi_handle handle = mem_device->device->handle;
277		- int result = 0, nid;
278	277	struct acpi_memory_info info, n;
	278	+ int nid = acpi_get_node(handle);
279	279
280		- nid = acpi_get_node(handle);
281		-
282	280	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
283	281	if (!info->enabled)
284	282	continue;
285	283
...	...	@@ -287,15 +285,10 @@
287	285	nid = memory_add_physaddr_to_nid(info->start_addr);
288	286
289	287	acpi_unbind_memory_blocks(info, handle);
290		- result = remove_memory(nid, info->start_addr, info->length);
291		- if (result)
292		- return result;
293		-
	288	+ remove_memory(nid, info->start_addr, info->length);
294	289	list_del(&info->list);
295	290	kfree(info);
296	291	}
297		-
298		- return result;
299	292	}
300	293
301	294	static void acpi_memory_device_free(struct acpi_memory_device *mem_device)
...	...	@@ -252,7 +252,7 @@
252	252	extern int arch_add_memory(int nid, u64 start, u64 size);
253	253	extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
254	254	extern bool is_memblock_offlined(struct memory_block *mem);
255		-extern int remove_memory(int nid, u64 start, u64 size);
	255	+extern void remove_memory(int nid, u64 start, u64 size);
256	256	extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn,
257	257	int nr_pages);
258	258	extern void sparse_remove_one_section(struct zone zone, struct mem_section ms);
...	...	@@ -1670,24 +1670,6 @@
1670	1670	}
1671	1671
1672	1672	#ifdef CONFIG_MEMORY_HOTREMOVE
1673		-/**
1674		- * offline_memory_block_cb - callback function for offlining memory block
1675		- * @mem: the memory block to be offlined
1676		- * @arg: buffer to hold error msg
1677		- *
1678		- * Always return 0, and put the error msg in arg if any.
1679		- */
1680		-static int offline_memory_block_cb(struct memory_block mem, void arg)
1681		-{
1682		- int *ret = arg;
1683		- int error = device_offline(&mem->dev);
1684		-
1685		- if (error != 0 && *ret == 0)
1686		- *ret = error;
1687		-
1688		- return 0;
1689		-}
1690		-
1691	1673	static int is_memblock_offlined_cb(struct memory_block mem, void arg)
1692	1674	{
1693	1675	int ret = !is_memblock_offlined(mem);
1694	1676
1695	1677
1696	1678
1697	1679
1698	1680
...	...	@@ -1813,54 +1795,22 @@
1813	1795	}
1814	1796	EXPORT_SYMBOL(try_offline_node);
1815	1797
1816		-int __ref remove_memory(int nid, u64 start, u64 size)
	1798	+void __ref remove_memory(int nid, u64 start, u64 size)
1817	1799	{
1818		- unsigned long start_pfn, end_pfn;
1819		- int ret = 0;
1820		- int retry = 1;
	1800	+ int ret;
1821	1801
1822		- start_pfn = PFN_DOWN(start);
1823		- end_pfn = PFN_UP(start + size - 1);
1824		-
1825		- /*
1826		- * When CONFIG_MEMCG is on, one memory block may be used by other
1827		- * blocks to store page cgroup when onlining pages. But we don't know
1828		- * in what order pages are onlined. So we iterate twice to offline
1829		- * memory:
1830		- * 1st iterate: offline every non primary memory block.
1831		- * 2nd iterate: offline primary (i.e. first added) memory block.
1832		- */
1833		-repeat:
1834		- walk_memory_range(start_pfn, end_pfn, &ret,
1835		- offline_memory_block_cb);
1836		- if (ret) {
1837		- if (!retry)
1838		- return ret;
1839		-
1840		- retry = 0;
1841		- ret = 0;
1842		- goto repeat;
1843		- }
1844		-
1845	1802	lock_memory_hotplug();
1846	1803
1847	1804	/*
1848		- * we have offlined all memory blocks like this:
1849		- * 1. lock memory hotplug
1850		- * 2. offline a memory block
1851		- * 3. unlock memory hotplug
1852		- *
1853		- * repeat step1-3 to offline the memory block. All memory blocks
1854		- * must be offlined before removing memory. But we don't hold the
1855		- * lock in the whole operation. So we should check whether all
1856		- * memory blocks are offlined.
	1805	+ * All memory blocks must be offlined before removing memory. Check
	1806	+ * whether all memory blocks in question are offline and trigger a BUG()
	1807	+ * if this is not the case.
1857	1808	*/
1858		-
1859		- ret = walk_memory_range(start_pfn, end_pfn, NULL,
	1809	+ ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL,
1860	1810	is_memblock_offlined_cb);
1861	1811	if (ret) {
1862	1812	unlock_memory_hotplug();
1863		- return ret;
	1813	+ BUG();
1864	1814	}
1865	1815
1866	1816	/* remove memmap entry */
1867	1817
...	...	@@ -1871,18 +1821,13 @@
1871	1821	try_offline_node(nid);
1872	1822
1873	1823	unlock_memory_hotplug();
1874		-
1875		- return 0;
1876	1824	}
1877	1825	#else
1878	1826	int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
1879	1827	{
1880	1828	return -EINVAL;
1881	1829	}
1882		-int remove_memory(int nid, u64 start, u64 size)
1883		-{
1884		- return -EINVAL;
1885		-}
	1830	+void remove_memory(int nid, u64 start, u64 size) {}
1886	1831	#endif /* CONFIG_MEMORY_HOTREMOVE */
1887	1832	EXPORT_SYMBOL_GPL(remove_memory);