linux-kernel - Re: [PATCH] memory tier: Remove unused default_dram_perf_ref

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7db244ee-91ba-4d45-8c54-dc98a75d8501@redhat.com>
Date: Fri, 20 Sep 2024 02:43:41 -0400
From: Waiman Long <longman@...hat.com>
To: "Huang, Ying" <ying.huang@...el.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Alistair Popple <apopple@...dia.com>, Dave Jiang <dave.jiang@...el.com>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] memory tier: Remove unused default_dram_perf_ref_source

On 9/20/24 02:26, Huang, Ying wrote:
> Hi, Waiman,
>
> Waiman Long <longman@...hat.com> writes:
>
>> Commit 3718c02dbd4c ("acpi, hmat: calculate abstract distance with HMAT")
>> added a default_dram_perf_ref_source variable that was initialized but
>> never used. This causes kmemleak to report the following memory leak:
>>
>> unreferenced object 0xff11000225a47b60 (size 16):
>>    comm "swapper/0", pid 1, jiffies 4294761654
>>    hex dump (first 16 bytes):
>>      41 43 50 49 20 48 4d 41 54 00 c1 4b 7d b7 75 7c  ACPI HMAT..K}.u|
>>    backtrace (crc e6d0e7b2):
>>      [<ffffffff95d5afdb>] __kmalloc_node_track_caller_noprof+0x36b/0x440
>>      [<ffffffff95c276d6>] kstrdup+0x36/0x60
>>      [<ffffffff95dfabfa>] mt_set_default_dram_perf+0x23a/0x2c0
>>      [<ffffffff9ad64733>] hmat_init+0x2b3/0x660
>>      [<ffffffff95203cec>] do_one_initcall+0x11c/0x5c0
>>      [<ffffffff9ac9cfc4>] do_initcalls+0x1b4/0x1f0
>>      [<ffffffff9ac9d52e>] kernel_init_freeable+0x4ae/0x520
>>      [<ffffffff97c789cc>] kernel_init+0x1c/0x150
>>      [<ffffffff952aecd1>] ret_from_fork+0x31/0x70
>>      [<ffffffff9520b18a>] ret_from_fork_asm+0x1a/0x30
> Good catch!
>
>> Fix this memory leak by removing default_dram_perf_ref_source.
> We should have used that information.  So, I suggest the fix as below.
> Is it OK for you?
>
> ---------------------------8<---------------------------------------
>  From 32e6d70f531718cf99064a43fdffc8639aedcc5c Mon Sep 17 00:00:00 2001
> From: Huang Ying <ying.huang@...el.com>
> Date: Fri, 20 Sep 2024 09:47:40 +0800
> Subject: [PATCH] memory tiers: use default_dram_perf_ref_source in log message
>
> Commit 3718c02dbd4c ("acpi, hmat: calculate abstract distance with HMAT")
> added a default_dram_perf_ref_source variable that was initialized but
> never used. This causes kmemleak to report the following memory leak:
>
> unreferenced object 0xff11000225a47b60 (size 16):
>    comm "swapper/0", pid 1, jiffies 4294761654
>    hex dump (first 16 bytes):
>      41 43 50 49 20 48 4d 41 54 00 c1 4b 7d b7 75 7c  ACPI HMAT..K}.u|
>    backtrace (crc e6d0e7b2):
>      [<ffffffff95d5afdb>] __kmalloc_node_track_caller_noprof+0x36b/0x440
>      [<ffffffff95c276d6>] kstrdup+0x36/0x60
>      [<ffffffff95dfabfa>] mt_set_default_dram_perf+0x23a/0x2c0
>      [<ffffffff9ad64733>] hmat_init+0x2b3/0x660
>      [<ffffffff95203cec>] do_one_initcall+0x11c/0x5c0
>      [<ffffffff9ac9cfc4>] do_initcalls+0x1b4/0x1f0
>      [<ffffffff9ac9d52e>] kernel_init_freeable+0x4ae/0x520
>      [<ffffffff97c789cc>] kernel_init+0x1c/0x150
>      [<ffffffff952aecd1>] ret_from_fork+0x31/0x70
>      [<ffffffff9520b18a>] ret_from_fork_asm+0x1a/0x30
>
> This reminds us that we forget to use the performance data source
> information.  So, use the variable in the error log message to help
> identify the root cause of inconsistent performance number.
>
> Fixes: 3718c02dbd4c ("acpi, hmat: calculate abstract distance with HMAT")
> Signed-off-by: "Huang, Ying" <ying.huang@...el.com>
> Reported-by: Waiman Long <longman@...hat.com>
> ---
>   mm/memory-tiers.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
> index 4775b3a3dabe..0f5ba5c6e0c6 100644
> --- a/mm/memory-tiers.c
> +++ b/mm/memory-tiers.c
> @@ -749,10 +749,10 @@ int mt_set_default_dram_perf(int nid, struct access_coordinate *perf,
>   		pr_info(
>   "memory-tiers: the performance of DRAM node %d mismatches that of the reference\n"
>   "DRAM node %d.\n", nid, default_dram_perf_ref_nid);
> -		pr_info("  performance of reference DRAM node %d:\n",
> -			default_dram_perf_ref_nid);
> +		pr_info("  performance of reference DRAM node %d from %s:\n",
> +			default_dram_perf_ref_nid, default_dram_perf_ref_source);
>   		dump_hmem_attrs(&default_dram_perf, "    ");
> -		pr_info("  performance of DRAM node %d:\n", nid);
> +		pr_info("  performance of DRAM node %d from %s:\n", nid, source);
>   		dump_hmem_attrs(perf, "    ");
>   		pr_info(
>   "  disable default DRAM node performance based abstract distance algorithm.\n");

That looks good for me. As I am not sure how this variable should be 
used, I just removed it in my patch.

Acked-by: Waiman Long <longman@...hat.com>