* [PATCH] memory tier: Remove unused default_dram_perf_ref_source
@ 2024-09-19 19:13 Waiman Long
2024-09-20 6:26 ` Huang, Ying
0 siblings, 1 reply; 3+ messages in thread
From: Waiman Long @ 2024-09-19 19:13 UTC (permalink / raw)
To: Andrew Morton, Huang Ying, Alistair Popple, Dave Jiang
Cc: linux-mm, linux-kernel, Waiman Long
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
Fix this memory leak by removing default_dram_perf_ref_source.
Fixes: 3718c02dbd4c ("acpi, hmat: calculate abstract distance with HMAT")
Signed-off-by: Waiman Long <longman@redhat.com>
---
mm/memory-tiers.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
index 4775b3a3dabe..baa5f20e3b78 100644
--- a/mm/memory-tiers.c
+++ b/mm/memory-tiers.c
@@ -119,7 +119,6 @@ static DEFINE_MUTEX(default_dram_perf_lock);
static bool default_dram_perf_error;
static struct access_coordinate default_dram_perf;
static int default_dram_perf_ref_nid = NUMA_NO_NODE;
-static const char *default_dram_perf_ref_source;
static inline struct memory_tier *to_memory_tier(struct device *device)
{
@@ -728,7 +727,6 @@ int mt_set_default_dram_perf(int nid, struct access_coordinate *perf,
if (default_dram_perf_ref_nid == NUMA_NO_NODE) {
default_dram_perf = *perf;
default_dram_perf_ref_nid = nid;
- default_dram_perf_ref_source = kstrdup(source, GFP_KERNEL);
return 0;
}
--
2.43.5
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] memory tier: Remove unused default_dram_perf_ref_source
2024-09-19 19:13 [PATCH] memory tier: Remove unused default_dram_perf_ref_source Waiman Long
@ 2024-09-20 6:26 ` Huang, Ying
2024-09-20 6:43 ` Waiman Long
0 siblings, 1 reply; 3+ messages in thread
From: Huang, Ying @ 2024-09-20 6:26 UTC (permalink / raw)
To: Waiman Long
Cc: Andrew Morton, Alistair Popple, Dave Jiang, linux-mm, linux-kernel
Hi, Waiman,
Waiman Long <longman@redhat.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@intel.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@intel.com>
Reported-by: Waiman Long <longman@redhat.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");
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] memory tier: Remove unused default_dram_perf_ref_source
2024-09-20 6:26 ` Huang, Ying
@ 2024-09-20 6:43 ` Waiman Long
0 siblings, 0 replies; 3+ messages in thread
From: Waiman Long @ 2024-09-20 6:43 UTC (permalink / raw)
To: Huang, Ying
Cc: Andrew Morton, Alistair Popple, Dave Jiang, linux-mm, linux-kernel
On 9/20/24 02:26, Huang, Ying wrote:
> Hi, Waiman,
>
> Waiman Long <longman@redhat.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@intel.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@intel.com>
> Reported-by: Waiman Long <longman@redhat.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@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-20 6:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-19 19:13 [PATCH] memory tier: Remove unused default_dram_perf_ref_source Waiman Long
2024-09-20 6:26 ` Huang, Ying
2024-09-20 6:43 ` Waiman Long
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox