block/partitions: optimize memory allocation in check_partition()

Currently, sizeof(struct parsed_partitions) may be 64KB in 32bit arch, so it is easy to trigger page allocation failure by check_partition, especially in hotplug block device situation(such as, USB mass storage, MMC card, ...), and Felipe Balbi has observed the failure. This patch does below optimizations on the allocation of struct parsed_partitions to try to address the issue: - make parsed_partitions.parts as pointer so that the pointed memory can fit in 32KB buffer, then approximate 32KB memory can be saved - vmalloc the buffer pointed by parsed_partitions.parts because 32KB is still a bit big for kmalloc - given that many devices have the partition count limit, so only allocate disk_max_parts() partitions instead of 256 partitions always Signed-off-by: Ming Lei <ming.lei@canonical.com> Reported-by: Felipe Balbi <balbi@ti.com> Cc: Jens Axboe <axboe@kernel.dk> Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

block/partitions: optimize memory allocation in check_partition()
Currently, sizeof(struct parsed_partitions) may be 64KB in 32bit arch, so it is easy to trigger page allocation failure by check_partition, especially in hotplug block device situation(such as, USB mass storage, MMC card, ...), and Felipe Balbi has observed the failure. This patch does below optimizations on the allocation of struct parsed_partitions to try to address the issue: - make parsed_partitions.parts as pointer so that the pointed memory can fit in 32KB buffer, then approximate 32KB memory can be saved - vmalloc the buffer pointed by parsed_partitions.parts because 32KB is still a bit big for kmalloc - given that many devices have the partition count limit, so only allocate disk_max_parts() partitions instead of 256 partitions always Signed-off-by: Ming Lei <ming.lei@canonical.com> Reported-by: Felipe Balbi <balbi@ti.com> Cc: Jens Axboe <axboe@kernel.dk> Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Ming Lei · Linus Torvalds
1 parent 06004e6eeb
Showing 3 changed files with 37 additions and 8 deletions Side-by-side Diff
block/partition-generic.c
block/partitions/check.c
block/partitions/check.h
@@ -418,7 +418,7 @@
 	int p, highest, res;
 rescan:
 	if (state && !IS_ERR(state)) {
-		kfree(state);
+		free_partitions(state);
 		state = NULL;
 	}
  
@@ -525,7 +525,7 @@
 			md_autodetect_dev(part_to_dev(part)->devt);
 #endif
 	}
-	kfree(state);
+	free_partitions(state);
 	return 0;
 }
  
@@ -14,6 +14,7 @@
  */
  
 #include <linux/slab.h>
+#include <linux/vmalloc.h>
 #include <linux/ctype.h>
 #include <linux/genhd.h>
  
  
  
@@ -106,18 +107,45 @@
 	NULL
 };
  
+static struct parsed_partitions *allocate_partitions(struct gendisk *hd)
+{
+	struct parsed_partitions *state;
+	int nr;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	nr = disk_max_parts(hd);
+	state->parts = vzalloc(nr * sizeof(state->parts[0]));
+	if (!state->parts) {
+		kfree(state);
+		return NULL;
+	}
+
+	state->limit = nr;
+
+	return state;
+}
+
+void free_partitions(struct parsed_partitions *state)
+{
+	vfree(state->parts);
+	kfree(state);
+}
+
 struct parsed_partitions *
 check_partition(struct gendisk *hd, struct block_device *bdev)
 {
 	struct parsed_partitions *state;
 	int i, res, err;
  
-	state = kzalloc(sizeof(struct parsed_partitions), GFP_KERNEL);
+	state = allocate_partitions(hd);
 	if (!state)
 		return NULL;
 	state->pp_buf = (char *)__get_free_page(GFP_KERNEL);
 	if (!state->pp_buf) {
-		kfree(state);
+		free_partitions(state);
 		return NULL;
 	}
 	state->pp_buf[0] = '\0';
  
@@ -128,10 +156,9 @@
 	if (isdigit(state->name[strlen(state->name)-1]))
 		sprintf(state->name, "p");
  
-	state->limit = disk_max_parts(hd);
 	i = res = err = 0;
 	while (!res && check_part[i]) {
-		memset(&state->parts, 0, sizeof(state->parts));
+		memset(state->parts, 0, state->limit * sizeof(state->parts[0]));
 		res = check_part[i++](state);
 		if (res < 0) {
 			/* We have hit an I/O error which we don't report now.
@@ -161,7 +188,7 @@
 	printk(KERN_INFO "%s", state->pp_buf);
  
 	free_page((unsigned long)state->pp_buf);
-	kfree(state);
+	free_partitions(state);
 	return ERR_PTR(res);
 }
@@ -15,12 +15,14 @@
 		int flags;
 		bool has_info;
 		struct partition_meta_info info;
-	} parts[DISK_MAX_PARTS];
+	} *parts;
 	int next;
 	int limit;
 	bool access_beyond_eod;
 	char *pp_buf;
 };
+
+void free_partitions(struct parsed_partitions *state);
  
 struct parsed_partitions *
 check_partition(struct gendisk *, struct block_device *);
...	...	@@ -418,7 +418,7 @@
418	418	int p, highest, res;
419	419	rescan:
420	420	if (state && !IS_ERR(state)) {
421		- kfree(state);
	421	+ free_partitions(state);
422	422	state = NULL;
423	423	}
424	424
...	...	@@ -525,7 +525,7 @@
525	525	md_autodetect_dev(part_to_dev(part)->devt);
526	526	#endif
527	527	}
528		- kfree(state);
	528	+ free_partitions(state);
529	529	return 0;
530	530	}
531	531
...	...	@@ -14,6 +14,7 @@
14	14	*/
15	15
16	16	#include <linux/slab.h>
	17	+#include <linux/vmalloc.h>
17	18	#include <linux/ctype.h>
18	19	#include <linux/genhd.h>
19	20
20	21
21	22
...	...	@@ -106,18 +107,45 @@
106	107	NULL
107	108	};
108	109
	110	+static struct parsed_partitions allocate_partitions(struct gendisk hd)
	111	+{
	112	+ struct parsed_partitions *state;
	113	+ int nr;
	114	+
	115	+ state = kzalloc(sizeof(*state), GFP_KERNEL);
	116	+ if (!state)
	117	+ return NULL;
	118	+
	119	+ nr = disk_max_parts(hd);
	120	+ state->parts = vzalloc(nr * sizeof(state->parts[0]));
	121	+ if (!state->parts) {
	122	+ kfree(state);
	123	+ return NULL;
	124	+ }
	125	+
	126	+ state->limit = nr;
	127	+
	128	+ return state;
	129	+}
	130	+
	131	+void free_partitions(struct parsed_partitions *state)
	132	+{
	133	+ vfree(state->parts);
	134	+ kfree(state);
	135	+}
	136	+
109	137	struct parsed_partitions *
110	138	check_partition(struct gendisk hd, struct block_device bdev)
111	139	{
112	140	struct parsed_partitions *state;
113	141	int i, res, err;
114	142
115		- state = kzalloc(sizeof(struct parsed_partitions), GFP_KERNEL);
	143	+ state = allocate_partitions(hd);
116	144	if (!state)
117	145	return NULL;
118	146	state->pp_buf = (char *)__get_free_page(GFP_KERNEL);
119	147	if (!state->pp_buf) {
120		- kfree(state);
	148	+ free_partitions(state);
121	149	return NULL;
122	150	}
123	151	state->pp_buf[0] = '\0';
124	152
...	...	@@ -128,10 +156,9 @@
128	156	if (isdigit(state->name[strlen(state->name)-1]))
129	157	sprintf(state->name, "p");
130	158
131		- state->limit = disk_max_parts(hd);
132	159	i = res = err = 0;
133	160	while (!res && check_part[i]) {
134		- memset(&state->parts, 0, sizeof(state->parts));
	161	+ memset(state->parts, 0, state->limit * sizeof(state->parts[0]));
135	162	res = check_part[i++](state);
136	163	if (res < 0) {
137	164	/* We have hit an I/O error which we don't report now.
...	...	@@ -161,7 +188,7 @@
161	188	printk(KERN_INFO "%s", state->pp_buf);
162	189
163	190	free_page((unsigned long)state->pp_buf);
164		- kfree(state);
	191	+ free_partitions(state);
165	192	return ERR_PTR(res);
166	193	}
...	...	@@ -15,12 +15,14 @@
15	15	int flags;
16	16	bool has_info;
17	17	struct partition_meta_info info;
18		- } parts[DISK_MAX_PARTS];
	18	+ } *parts;
19	19	int next;
20	20	int limit;
21	21	bool access_beyond_eod;
22	22	char *pp_buf;
23	23	};
	24	+
	25	+void free_partitions(struct parsed_partitions *state);
24	26
25	27	struct parsed_partitions *
26	28	check_partition(struct gendisk , struct block_device );