diff mbox series

HID: Use kvzalloc instead of kzalloc in hid_register_field()

Message ID 20240522080328.12317-1-hailong.liu@oppo.com (mailing list archive)
State New
Delegated to: Jiri Kosina
Headers show
Series HID: Use kvzalloc instead of kzalloc in hid_register_field() | expand

Commit Message

Hailong Liu May 22, 2024, 8:03 a.m. UTC 
From: "Hailong.Liu" <hailong.liu@oppo.com>

The function hid_register_field() might allocate more than 32k, which
would use order-4 contiguous memory if the parameter usage exceeds
1024. However, after the system runs for a while, the memory can
become heavily fragmented. This increases the likelihood of order-4 page
allocation failure. Here’s the relevant log.

[71553.093623]kworker/1: 0: page allocation failure: order:4, mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0
[71553.093669]Workqueue: events uhid_device_add_worker
[71553.093683]Call trace:
[71553.093687]: dump_backtrace+0xf4/0x118
[71553.093696]: show_stack+0x18/0x24
[71553.093702]: dump_stack_lvl+0x60/0x7c
[71553.093710]: dump_stack+0x18/0x3c
[71553.093717]: warn_alloc+0xf4/0x174
[71553.093725]: __alloc_pages_slowpath+0x1ba0/0x1cac
[71553.093732]: __alloc_pages+0x460/0x560
[71553.093738]: __kmalloc_large_node+0xbc/0x1f8
[71553.093746]: __kmalloc+0x144/0x254
[71553.093752]: hid_add_field+0x13c/0x308
[71553.093758]: hid_parser_main+0x250/0x298
[71553.093765]: hid_open_report+0x214/0x30c
[71553.093771]: mt_probe+0x130/0x258
[71553.093778]: hid_device_probe+0x11c/0x1e4
[71553.093784]: really_probe+0xe4/0x388
[71553.093791]: __driver_probe_device+0xa0/0x12c
[71553.093798]: driver_probe_device+0x44/0x214
[71553.093804]: __device_attach_driver+0xdc/0x124
[71553.093812]: bus_for_each_drv+0x88/0xec
[71553.093818]: __device_attach+0x84/0x170
[71553.093824]: device_initial_probe+0x14/0x20
[71553.093831]: bus_probe_device+0x48/0xd0
[71553.093836]: device_add+0x248/0x928
[71553.093844]: hid_add_device+0xf8/0x1a4
[71553.093850]: uhid_device_add_worker+0x24/0x144
[71553.093857]: process_one_work+0x158/0x804
[71553.093865]: worker_thread+0x15c/0x494
[71553.093872]: kthread+0xf4/0x1e4
[71553.093880]: ret_from_fork+0x10/0x20

To fix the allocation failure, use kvzalloc() instead of kzalloc().

Signed-off-by: Hailong.Liu <hailong.liu@oppo.com>
---
 drivers/hid/hid-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--
2.34.1

Comments

Barry Song May 22, 2024, 9:07 a.m. UTC | #1 
On Wed, May 22, 2024 at 8:03 PM <hailong.liu@oppo.com> wrote:
>
> From: "Hailong.Liu" <hailong.liu@oppo.com>
>
> The function hid_register_field() might allocate more than 32k, which
> would use order-4 contiguous memory if the parameter usage exceeds
> 1024. However, after the system runs for a while, the memory can
> become heavily fragmented. This increases the likelihood of order-4 page
> allocation failure. Here’s the relevant log.
>
> [71553.093623]kworker/1: 0: page allocation failure: order:4, mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0
> [71553.093669]Workqueue: events uhid_device_add_worker
> [71553.093683]Call trace:
> [71553.093687]: dump_backtrace+0xf4/0x118
> [71553.093696]: show_stack+0x18/0x24
> [71553.093702]: dump_stack_lvl+0x60/0x7c
> [71553.093710]: dump_stack+0x18/0x3c
> [71553.093717]: warn_alloc+0xf4/0x174
> [71553.093725]: __alloc_pages_slowpath+0x1ba0/0x1cac
> [71553.093732]: __alloc_pages+0x460/0x560
> [71553.093738]: __kmalloc_large_node+0xbc/0x1f8
> [71553.093746]: __kmalloc+0x144/0x254
> [71553.093752]: hid_add_field+0x13c/0x308
> [71553.093758]: hid_parser_main+0x250/0x298
> [71553.093765]: hid_open_report+0x214/0x30c
> [71553.093771]: mt_probe+0x130/0x258
> [71553.093778]: hid_device_probe+0x11c/0x1e4
> [71553.093784]: really_probe+0xe4/0x388
> [71553.093791]: __driver_probe_device+0xa0/0x12c
> [71553.093798]: driver_probe_device+0x44/0x214
> [71553.093804]: __device_attach_driver+0xdc/0x124
> [71553.093812]: bus_for_each_drv+0x88/0xec
> [71553.093818]: __device_attach+0x84/0x170
> [71553.093824]: device_initial_probe+0x14/0x20
> [71553.093831]: bus_probe_device+0x48/0xd0
> [71553.093836]: device_add+0x248/0x928
> [71553.093844]: hid_add_device+0xf8/0x1a4
> [71553.093850]: uhid_device_add_worker+0x24/0x144
> [71553.093857]: process_one_work+0x158/0x804
> [71553.093865]: worker_thread+0x15c/0x494
> [71553.093872]: kthread+0xf4/0x1e4
> [71553.093880]: ret_from_fork+0x10/0x20
>
> To fix the allocation failure, use kvzalloc() instead of kzalloc().
>
> Signed-off-by: Hailong.Liu <hailong.liu@oppo.com>

This makes a lot of sense from the perspective of mm.

Acked-by: Barry Song <baohua@kernel.org>

> ---
>  drivers/hid/hid-core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index de7a477d6665..574ec4873f41 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -95,9 +95,9 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
>                 return NULL;
>         }
>
> -       field = kzalloc((sizeof(struct hid_field) +
> -                        usages * sizeof(struct hid_usage) +
> -                        3 * usages * sizeof(unsigned int)), GFP_KERNEL);
> +       field = kvzalloc((sizeof(struct hid_field) +
> +                         usages * sizeof(struct hid_usage) +
> +                         3 * usages * sizeof(unsigned int)), GFP_KERNEL);
>         if (!field)
>                 return NULL;
>
> @@ -661,7 +661,7 @@ static void hid_free_report(struct hid_report *report)
>         kfree(report->field_entries);
>
>         for (n = 0; n < report->maxfield; n++)
> -               kfree(report->field[n]);
> +               kvfree(report->field[n]);
>         kfree(report);
>  }
>
> --
> 2.34.1
>

Thanks
Barry
Jiri Kosina June 4, 2024, 7:58 a.m. UTC | #2 
On Wed, 22 May 2024, hailong.liu@oppo.com wrote:

> From: "Hailong.Liu" <hailong.liu@oppo.com>
> 
> The function hid_register_field() might allocate more than 32k, which
> would use order-4 contiguous memory if the parameter usage exceeds
> 1024. However, after the system runs for a while, the memory can
> become heavily fragmented. This increases the likelihood of order-4 page
> allocation failure. Here’s the relevant log.
> 
> [71553.093623]kworker/1: 0: page allocation failure: order:4, mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), nodemask=(null),cpuset=/,mems_allowed=0
> [71553.093669]Workqueue: events uhid_device_add_worker
> [71553.093683]Call trace:
> [71553.093687]: dump_backtrace+0xf4/0x118
> [71553.093696]: show_stack+0x18/0x24
> [71553.093702]: dump_stack_lvl+0x60/0x7c
> [71553.093710]: dump_stack+0x18/0x3c
> [71553.093717]: warn_alloc+0xf4/0x174
> [71553.093725]: __alloc_pages_slowpath+0x1ba0/0x1cac
> [71553.093732]: __alloc_pages+0x460/0x560
> [71553.093738]: __kmalloc_large_node+0xbc/0x1f8
> [71553.093746]: __kmalloc+0x144/0x254
> [71553.093752]: hid_add_field+0x13c/0x308
> [71553.093758]: hid_parser_main+0x250/0x298
> [71553.093765]: hid_open_report+0x214/0x30c
> [71553.093771]: mt_probe+0x130/0x258
> [71553.093778]: hid_device_probe+0x11c/0x1e4
> [71553.093784]: really_probe+0xe4/0x388
> [71553.093791]: __driver_probe_device+0xa0/0x12c
> [71553.093798]: driver_probe_device+0x44/0x214
> [71553.093804]: __device_attach_driver+0xdc/0x124
> [71553.093812]: bus_for_each_drv+0x88/0xec
> [71553.093818]: __device_attach+0x84/0x170
> [71553.093824]: device_initial_probe+0x14/0x20
> [71553.093831]: bus_probe_device+0x48/0xd0
> [71553.093836]: device_add+0x248/0x928
> [71553.093844]: hid_add_device+0xf8/0x1a4
> [71553.093850]: uhid_device_add_worker+0x24/0x144
> [71553.093857]: process_one_work+0x158/0x804
> [71553.093865]: worker_thread+0x15c/0x494
> [71553.093872]: kthread+0xf4/0x1e4
> [71553.093880]: ret_from_fork+0x10/0x20
> 
> To fix the allocation failure, use kvzalloc() instead of kzalloc().
> 
> Signed-off-by: Hailong.Liu <hailong.liu@oppo.com>

Applied, thanks.
diff mbox series

Patch

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index de7a477d6665..574ec4873f41 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -95,9 +95,9 @@  static struct hid_field *hid_register_field(struct hid_report *report, unsigned
 		return NULL;
 	}

-	field = kzalloc((sizeof(struct hid_field) +
-			 usages * sizeof(struct hid_usage) +
-			 3 * usages * sizeof(unsigned int)), GFP_KERNEL);
+	field = kvzalloc((sizeof(struct hid_field) +
+			  usages * sizeof(struct hid_usage) +
+			  3 * usages * sizeof(unsigned int)), GFP_KERNEL);
 	if (!field)
 		return NULL;

@@ -661,7 +661,7 @@  static void hid_free_report(struct hid_report *report)
 	kfree(report->field_entries);

 	for (n = 0; n < report->maxfield; n++)
-		kfree(report->field[n]);
+		kvfree(report->field[n]);
 	kfree(report);
 }