Re: 2.6.21-rc3-mm1

[Jiri Kosina]

On Sat, 10 Mar 2007, Greg KH wrote:

> > hid_parse_report() is doing kmalloc(128k kbytes).  We canot sanely 
> > support that and the code shold be rewritten to not do that.  A simple 
> > though somewhat lame fix would be to switch to vmalloc().
> > It's been this way for some time, so it's odd that the failures have 
> > just popped up now.
> Jiri is the person to ask about this now.  Jiri, any thoughts about
> this?

Hi,

I have just queued the patch below to HID tree for the next upstream 
merge. Mariusz, I guess it solves your issue, right?

I have already been talking with Vojtech some time ago that rewritting the 
hid parser so that it would use less memory (but probably slightly a bit 
more CPU) would be a good thing to do, and it's been sitting in my TODO 
list for quite some time already. It's really not a straightforward 
rewrite, so I would incline to use the vmalloc() solution until the parser 
code has been rewritten. The hid_parser structure in question is living 
for very short time anyway, so it shouldn't be that big issue.

Thanks.


From: Jiri Kosina <jkosina@xxxxxxx>
Subject: [PATCH] HID: allocate hid_parser through vmalloc()

hid_parser is non-trivially large structure, so it should be allocated
using vmalloc() to avoid unsuccessful allocations when memory fragmentation
is too high.
This structue has a very short life, it's destroyed as soon as the report
descriptor has been completely parsed.

This should be considered a temporary solution, until the hid_parser is
rewritten to consume less memory during report descriptor parsing.

Signed-off-by: Jiri Kosina <jkosina@xxxxxxx>
---
 drivers/hid/hid-core.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index f4ee1af..e5894a7 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -26,6 +26,7 @@
 #include <asm/byteorder.h>
 #include <linux/input.h>
 #include <linux/wait.h>
+#include <linux/vmalloc.h>
 
 #include <linux/hid.h>
 #include <linux/hiddev.h>
@@ -654,12 +655,13 @@ struct hid_device *hid_parse_report(__u8 *start, unsigned size)
 	memcpy(device->rdesc, start, size);
 	device->rsize = size;
 
-	if (!(parser = kzalloc(sizeof(struct hid_parser), GFP_KERNEL))) {
+	if (!(parser = vmalloc(sizeof(struct hid_parser)))) {
 		kfree(device->rdesc);
 		kfree(device->collection);
 		kfree(device);
 		return NULL;
 	}
+	memset(parser, 0, sizeof(struct hid_parser));
 	parser->device = device;
 
 	end = start + size;
@@ -668,7 +670,7 @@ struct hid_device *hid_parse_report(__u8 *start, unsigned size)
 		if (item.format != HID_ITEM_FORMAT_SHORT) {
 			dbg("unexpected long global item");
 			hid_free_device(device);
-			kfree(parser);
+			vfree(parser);
 			return NULL;
 		}
 
@@ -676,7 +678,7 @@ struct hid_device *hid_parse_report(__u8 *start, unsigned size)
 			dbg("item %u %u %u %u parsing failed\n",
 				item.format, (unsigned)item.size, (unsigned)item.type, (unsigned)item.tag);
 			hid_free_device(device);
-			kfree(parser);
+			vfree(parser);
 			return NULL;
 		}
 
@@ -684,23 +686,23 @@ struct hid_device *hid_parse_report(__u8 *start, unsigned size)
 			if (parser->collection_stack_ptr) {
 				dbg("unbalanced collection at end of report description");
 				hid_free_device(device);
-				kfree(parser);
+				vfree(parser);
 				return NULL;
 			}
 			if (parser->local.delimiter_depth) {
 				dbg("unbalanced delimiter at end of report description");
 				hid_free_device(device);
-				kfree(parser);
+				vfree(parser);
 				return NULL;
 			}
-			kfree(parser);
+			vfree(parser);
 			return device;
 		}
 	}
 
 	dbg("item fetching failed at offset %d\n", (int)(end - start));
 	hid_free_device(device);
-	kfree(parser);
+	vfree(parser);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(hid_parse_report);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/