lkml.org 
[lkml]   [2019]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] pstore/ram: Replace dummy_data heap memory with stack memory
On Tue, Jan 22, 2019 at 2:37 PM Yue Hu <huyue2@yulong.com> wrote:
>
> From e37cbd4d22eae55c034536817b22d429ba0ae27a Mon Sep 17 00:00:00 2001

Can you check your MUA? It turns out this email was damaged in a few ways:
- something broke enough that it never appeared on patchwork (maybe
not on lkml either?)
- lines are wrapped in the body

> From: Yue Hu <huyue2@yulong.com>
> Date: Mon, 21 Jan 2019 18:20:41 +0800
> Subject: [PATCH] pstore/ram: Replace dummy_data heap memory with stack
> memory

like here

>
> In ramoops_register_dummy() dummy_data is allocated via kzalloc()
> then it will always occupy the heap space after register platform
> device via platform_device_register_data(), but it will not be
> used any more. So let's free it for system usage, replace it with
> stack memory is better due to small size.
>
> Signed-off-by: Yue Hu <huyue2@yulong.com>
> ---
> fs/pstore/ram.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 96f7d32..8db1f7f 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -110,7 +110,6 @@ struct ramoops_context {
> };
>
> static struct platform_device *dummy;
> -static struct ramoops_platform_data *dummy_data;
>
> static int ramoops_pstore_open(struct pstore_info *psi)
> {
> @@ -896,13 +895,12 @@ static inline void ramoops_unregister_dummy(void)
> {
> platform_device_unregister(dummy);
> dummy = NULL;
> -
> - kfree(dummy_data);
> - dummy_data = NULL;
> }
>
> static void __init ramoops_register_dummy(void)
> {
> + struct ramoops_platform_data pdata;
> +
> /*
> * Prepare a dummy platform data structure to carry the module
> * parameters. If mem_size isn't set, then there are no module
> @@ -913,30 +911,24 @@ static void __init ramoops_register_dummy(void)
>
> pr_info("using module parameters\n");
>
> - dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL);

The zeroing here is required.

> - if (!dummy_data) {
> - pr_info("could not allocate pdata\n");
> - return;
> - }
> -
> - dummy_data->mem_size = mem_size;
> - dummy_data->mem_address = mem_address;
> - dummy_data->mem_type = mem_type;
> - dummy_data->record_size = record_size;
> - dummy_data->console_size = ramoops_console_size;
> - dummy_data->ftrace_size = ramoops_ftrace_size;
> - dummy_data->pmsg_size = ramoops_pmsg_size;
> - dummy_data->dump_oops = dump_oops;
> - dummy_data->flags = RAMOOPS_FLAG_FTRACE_PER_CPU;

I added a memset() to zero here to compensate.

> + pdata.mem_size = mem_size;
> + pdata.mem_address = mem_address;
> + pdata.mem_type = mem_type;
> + pdata.record_size = record_size;
> + pdata.console_size = ramoops_console_size;
> + pdata.ftrace_size = ramoops_ftrace_size;
> + pdata.pmsg_size = ramoops_pmsg_size;
> + pdata.dump_oops = dump_oops;
> + pdata.flags = RAMOOPS_FLAG_FTRACE_PER_CPU;
>
> /*
> * For backwards compatibility ramoops.ecc=1 means 16 bytes ECC
> * (using 1 byte for ECC isn't much of use anyway).
> */
> - dummy_data->ecc_info.ecc_size = ramoops_ecc == 1 ? 16 :
> ramoops_ecc;
> + pdata.ecc_info.ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc;
>
> dummy = platform_device_register_data(NULL, "ramoops", -1,
> - dummy_data, sizeof(struct
> ramoops_platform_data));
> + &pdata, sizeof(struct ramoops_platform_data));

I made this more robust by using sizeof(pdata) instead of an explicit struct.

> if (IS_ERR(dummy)) {
> pr_info("could not create platform device: %ld\n",
> PTR_ERR(dummy));
> --
> 1.9.1

The result should be visible in linux-next soon.

--
Kees Cook

\
 
 \ /
  Last update: 2019-01-22 04:33    [W:0.025 / U:4.020 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site