* [PATCH 0/3] Some bugfix about kmemleak
@ 2023-09-27 3:59 Liu Shixin
2023-09-27 3:59 ` [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem Liu Shixin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Liu Shixin @ 2023-09-27 3:59 UTC (permalink / raw
To: Catalin Marinas, Patrick Wang, Andrew Morton
Cc: linux-mm, linux-kernel, Liu Shixin
There are three bugfix to improve the accuracy of detection and the print
info of debug mode.
Liu Shixin (3):
bootmem: use kmemleak_free_part_phys in
free_bootmem_page/put_page_bootmem
mm/kmemleak: fix partially freeing unknown object warning
mm/kmemleak: fix print format of pointer in pr_debug()
include/linux/bootmem_info.h | 2 +-
mm/bootmem_info.c | 2 +-
mm/kmemleak.c | 34 ++++++++++++++++++++--------------
3 files changed, 22 insertions(+), 16 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem
2023-09-27 3:59 [PATCH 0/3] Some bugfix about kmemleak Liu Shixin
@ 2023-09-27 3:59 ` Liu Shixin
2023-09-27 16:05 ` Andrew Morton
2023-09-27 17:00 ` Catalin Marinas
2023-09-27 3:59 ` [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning Liu Shixin
2023-09-27 3:59 ` [PATCH 3/3] mm/kmemleak: fix print format of pointer in pr_debug() Liu Shixin
2 siblings, 2 replies; 9+ messages in thread
From: Liu Shixin @ 2023-09-27 3:59 UTC (permalink / raw
To: Catalin Marinas, Patrick Wang, Andrew Morton
Cc: linux-mm, linux-kernel, Liu Shixin
Since kmemleak_alloc_phys() rather than kmemleak_alloc() was called from
memblock_alloc_range_nid(), kmemleak_free_part_phys() should be used to
delete kmemleak object in free_bootmem_page() and put_page_bootmem().
Fixes: 028725e73375 ("bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page")
Fixes: dd0ff4d12dd2 ("bootmem: remove the vmemmap pages from kmemleak in put_page_bootmem")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
include/linux/bootmem_info.h | 2 +-
mm/bootmem_info.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/bootmem_info.h b/include/linux/bootmem_info.h
index e1a3c9c9754c..cffa38a73618 100644
--- a/include/linux/bootmem_info.h
+++ b/include/linux/bootmem_info.h
@@ -60,7 +60,7 @@ static inline void get_page_bootmem(unsigned long info, struct page *page,
static inline void free_bootmem_page(struct page *page)
{
- kmemleak_free_part(page_to_virt(page), PAGE_SIZE);
+ kmemleak_free_part_phys(PFN_PHYS(page_to_pfn(page)), PAGE_SIZE);
free_reserved_page(page);
}
#endif
diff --git a/mm/bootmem_info.c b/mm/bootmem_info.c
index b1efebfcf94b..fa7cb0c87c03 100644
--- a/mm/bootmem_info.c
+++ b/mm/bootmem_info.c
@@ -34,7 +34,7 @@ void put_page_bootmem(struct page *page)
ClearPagePrivate(page);
set_page_private(page, 0);
INIT_LIST_HEAD(&page->lru);
- kmemleak_free_part(page_to_virt(page), PAGE_SIZE);
+ kmemleak_free_part_phys(PFN_PHYS(page_to_pfn(page)), PAGE_SIZE);
free_reserved_page(page);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning
2023-09-27 3:59 [PATCH 0/3] Some bugfix about kmemleak Liu Shixin
2023-09-27 3:59 ` [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem Liu Shixin
@ 2023-09-27 3:59 ` Liu Shixin
2023-09-27 17:06 ` Catalin Marinas
2023-09-27 3:59 ` [PATCH 3/3] mm/kmemleak: fix print format of pointer in pr_debug() Liu Shixin
2 siblings, 1 reply; 9+ messages in thread
From: Liu Shixin @ 2023-09-27 3:59 UTC (permalink / raw
To: Catalin Marinas, Patrick Wang, Andrew Morton
Cc: linux-mm, linux-kernel, Liu Shixin
delete_object_part() can be called by multiple callers in the same time.
If an object is found and removed by a caller, and then another caller
try to find it too, it failed and return directly. The secound part still
be recorded by kmemleak even if it has alreadly been freed to buddy.
With DEBUG on, kmemleak will report the following warning:
kmemleak: Partially freeing unknown object at 0xa1af86000 (size 4096)
CPU: 0 PID: 742 Comm: test_huge Not tainted 6.6.0-rc3kmemleak+ #54
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x37/0x50
kmemleak_free_part_phys+0x50/0x60
hugetlb_vmemmap_optimize+0x172/0x290
? __pfx_vmemmap_remap_pte+0x10/0x10
__prep_new_hugetlb_folio+0xe/0x30
prep_new_hugetlb_folio.isra.0+0xe/0x40
alloc_fresh_hugetlb_folio+0xc3/0xd0
alloc_surplus_hugetlb_folio.constprop.0+0x6e/0xd0
hugetlb_acct_memory.part.0+0xe6/0x2a0
hugetlb_reserve_pages+0x110/0x2c0
hugetlbfs_file_mmap+0x11d/0x1b0
mmap_region+0x248/0x9a0
? hugetlb_get_unmapped_area+0x15c/0x2d0
do_mmap+0x38b/0x580
vm_mmap_pgoff+0xe6/0x190
ksys_mmap_pgoff+0x18a/0x1f0
do_syscall_64+0x3f/0x90
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
Fixes: 53238a60dd4a ("kmemleak: Allow partial freeing of memory blocks")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
mm/kmemleak.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 54c2c90d3abc..5a2bbd85df57 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -208,6 +208,8 @@ static struct rb_root object_tree_root = RB_ROOT;
static struct rb_root object_phys_tree_root = RB_ROOT;
/* protecting the access to object_list, object_tree_root (or object_phys_tree_root) */
static DEFINE_RAW_SPINLOCK(kmemleak_lock);
+/* Serial delete_object_part() to ensure all objects is deleted correctly */
+static DEFINE_RAW_SPINLOCK(delete_object_part_mutex);
/* allocation caches for kmemleak internal data */
static struct kmem_cache *object_cache;
@@ -784,13 +786,16 @@ static void delete_object_part(unsigned long ptr, size_t size, bool is_phys)
{
struct kmemleak_object *object;
unsigned long start, end;
+ unsigned long flags;
+ raw_spin_lock_irqsave(&delete_object_part_mutex, flags);
object = find_and_remove_object(ptr, 1, is_phys);
if (!object) {
#ifdef DEBUG
kmemleak_warn("Partially freeing unknown object at 0x%08lx (size %zu)\n",
ptr, size);
#endif
+ raw_spin_unlock_irqrestore(&delete_object_part_mutex, flags);
return;
}
@@ -809,6 +814,7 @@ static void delete_object_part(unsigned long ptr, size_t size, bool is_phys)
GFP_KERNEL, is_phys);
__delete_object(object);
+ raw_spin_unlock_irqrestore(&delete_object_part_mutex, flags);
}
static void __paint_it(struct kmemleak_object *object, int color)
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] mm/kmemleak: fix print format of pointer in pr_debug()
2023-09-27 3:59 [PATCH 0/3] Some bugfix about kmemleak Liu Shixin
2023-09-27 3:59 ` [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem Liu Shixin
2023-09-27 3:59 ` [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning Liu Shixin
@ 2023-09-27 3:59 ` Liu Shixin
2023-09-28 9:27 ` Catalin Marinas
2 siblings, 1 reply; 9+ messages in thread
From: Liu Shixin @ 2023-09-27 3:59 UTC (permalink / raw
To: Catalin Marinas, Patrick Wang, Andrew Morton
Cc: linux-mm, linux-kernel, Liu Shixin
With 0x%p, the pointer will be hashed and print (____ptrval____) instead.
And with 0x%pa, the pointer can be successfully printed but with duplicate
prefixes, which looks like:
kmemleak: kmemleak_free(0x(____ptrval____))
kmemleak: kmemleak_free_part_phys(0x0x0000000a1af86000)
Use %pa instead of 0x%p or 0x%pa to print the pointer, and use 0x%px for
__percpu pointer to prevent crash. Then the print will be like:
kmemleak: kmemleak_free(0xffff9111c145b020)
kmemleak: kmemleak_free_percpu(0x00000000000333b0)
kmemleak: kmemleak_free_part_phys(0x0000000a1af80000)
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
mm/kmemleak.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 5a2bbd85df57..c1053db98f55 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -981,7 +981,7 @@ static void object_no_scan(unsigned long ptr)
void __ref kmemleak_alloc(const void *ptr, size_t size, int min_count,
gfp_t gfp)
{
- pr_debug("%s(0x%p, %zu, %d)\n", __func__, ptr, size, min_count);
+ pr_debug("%s(%pa, %zu, %d)\n", __func__, ptr, size, min_count);
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
create_object((unsigned long)ptr, size, min_count, gfp);
@@ -1002,7 +1002,7 @@ void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
{
unsigned int cpu;
- pr_debug("%s(0x%p, %zu)\n", __func__, ptr, size);
+ pr_debug("%s(0x%px, %zu)\n", __func__, ptr, size);
/*
* Percpu allocations are only scanned and not reported as leaks
@@ -1026,7 +1026,7 @@ EXPORT_SYMBOL_GPL(kmemleak_alloc_percpu);
*/
void __ref kmemleak_vmalloc(const struct vm_struct *area, size_t size, gfp_t gfp)
{
- pr_debug("%s(0x%p, %zu)\n", __func__, area, size);
+ pr_debug("%s(%pa, %zu)\n", __func__, area, size);
/*
* A min_count = 2 is needed because vm_struct contains a reference to
@@ -1049,7 +1049,7 @@ EXPORT_SYMBOL_GPL(kmemleak_vmalloc);
*/
void __ref kmemleak_free(const void *ptr)
{
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr);
if (kmemleak_free_enabled && ptr && !IS_ERR(ptr))
delete_object_full((unsigned long)ptr);
@@ -1067,7 +1067,7 @@ EXPORT_SYMBOL_GPL(kmemleak_free);
*/
void __ref kmemleak_free_part(const void *ptr, size_t size)
{
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr);
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
delete_object_part((unsigned long)ptr, size, false);
@@ -1085,7 +1085,7 @@ void __ref kmemleak_free_percpu(const void __percpu *ptr)
{
unsigned int cpu;
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(0x%px)\n", __func__, ptr);
if (kmemleak_free_enabled && ptr && !IS_ERR(ptr))
for_each_possible_cpu(cpu)
@@ -1106,7 +1106,7 @@ void __ref kmemleak_update_trace(const void *ptr)
struct kmemleak_object *object;
unsigned long flags;
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr);
if (!kmemleak_enabled || IS_ERR_OR_NULL(ptr))
return;
@@ -1137,7 +1137,7 @@ EXPORT_SYMBOL(kmemleak_update_trace);
*/
void __ref kmemleak_not_leak(const void *ptr)
{
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr);
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
make_gray_object((unsigned long)ptr);
@@ -1155,7 +1155,7 @@ EXPORT_SYMBOL(kmemleak_not_leak);
*/
void __ref kmemleak_ignore(const void *ptr)
{
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr) ;
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
make_black_object((unsigned long)ptr, false);
@@ -1175,7 +1175,7 @@ EXPORT_SYMBOL(kmemleak_ignore);
*/
void __ref kmemleak_scan_area(const void *ptr, size_t size, gfp_t gfp)
{
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr);
if (kmemleak_enabled && ptr && size && !IS_ERR(ptr))
add_scan_area((unsigned long)ptr, size, gfp);
@@ -1193,7 +1193,7 @@ EXPORT_SYMBOL(kmemleak_scan_area);
*/
void __ref kmemleak_no_scan(const void *ptr)
{
- pr_debug("%s(0x%p)\n", __func__, ptr);
+ pr_debug("%s(%pa)\n", __func__, ptr);
if (kmemleak_enabled && ptr && !IS_ERR(ptr))
object_no_scan((unsigned long)ptr);
@@ -1209,7 +1209,7 @@ EXPORT_SYMBOL(kmemleak_no_scan);
*/
void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, gfp_t gfp)
{
- pr_debug("%s(0x%pa, %zu)\n", __func__, &phys, size);
+ pr_debug("%s(%pa, %zu)\n", __func__, &phys, size);
if (kmemleak_enabled)
/*
@@ -1229,7 +1229,7 @@ EXPORT_SYMBOL(kmemleak_alloc_phys);
*/
void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
{
- pr_debug("%s(0x%pa)\n", __func__, &phys);
+ pr_debug("%s(%pa)\n", __func__, &phys);
if (kmemleak_enabled)
delete_object_part((unsigned long)phys, size, true);
@@ -1243,7 +1243,7 @@ EXPORT_SYMBOL(kmemleak_free_part_phys);
*/
void __ref kmemleak_ignore_phys(phys_addr_t phys)
{
- pr_debug("%s(0x%pa)\n", __func__, &phys);
+ pr_debug("%s(%pa)\n", __func__, &phys);
if (kmemleak_enabled)
make_black_object((unsigned long)phys, true);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem
2023-09-27 3:59 ` [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem Liu Shixin
@ 2023-09-27 16:05 ` Andrew Morton
2023-09-27 17:00 ` Catalin Marinas
1 sibling, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2023-09-27 16:05 UTC (permalink / raw
To: Liu Shixin; +Cc: Catalin Marinas, Patrick Wang, linux-mm, linux-kernel
On Wed, 27 Sep 2023 11:59:21 +0800 Liu Shixin <liushixin2@huawei.com> wrote:
> Since kmemleak_alloc_phys() rather than kmemleak_alloc() was called from
> memblock_alloc_range_nid(), kmemleak_free_part_phys() should be used to
> delete kmemleak object in free_bootmem_page() and put_page_bootmem().
>
> Fixes: 028725e73375 ("bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page")
> Fixes: dd0ff4d12dd2 ("bootmem: remove the vmemmap pages from kmemleak in put_page_bootmem")
Having two Fixes: is awkward. If someone is considering backporting
this patch into earlier kernels then which Fixes: target should they
use to decide whether the fix is applicable?
So I think it would be best if this patch was split into two patches,
please.
And let's decide whther these fixes should be backported into -stable
kernels. For that, please alter the changelogs so they tell us what are
the userspace-visible effect of the bugs.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem
2023-09-27 3:59 ` [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem Liu Shixin
2023-09-27 16:05 ` Andrew Morton
@ 2023-09-27 17:00 ` Catalin Marinas
1 sibling, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2023-09-27 17:00 UTC (permalink / raw
To: Liu Shixin; +Cc: Patrick Wang, Andrew Morton, linux-mm, linux-kernel
On Wed, Sep 27, 2023 at 11:59:21AM +0800, Liu Shixin wrote:
> Since kmemleak_alloc_phys() rather than kmemleak_alloc() was called from
> memblock_alloc_range_nid(), kmemleak_free_part_phys() should be used to
> delete kmemleak object in free_bootmem_page() and put_page_bootmem().
>
> Fixes: 028725e73375 ("bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page")
> Fixes: dd0ff4d12dd2 ("bootmem: remove the vmemmap pages from kmemleak in put_page_bootmem")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> include/linux/bootmem_info.h | 2 +-
> mm/bootmem_info.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bootmem_info.h b/include/linux/bootmem_info.h
> index e1a3c9c9754c..cffa38a73618 100644
> --- a/include/linux/bootmem_info.h
> +++ b/include/linux/bootmem_info.h
> @@ -60,7 +60,7 @@ static inline void get_page_bootmem(unsigned long info, struct page *page,
>
> static inline void free_bootmem_page(struct page *page)
> {
> - kmemleak_free_part(page_to_virt(page), PAGE_SIZE);
> + kmemleak_free_part_phys(PFN_PHYS(page_to_pfn(page)), PAGE_SIZE);
We have a page_to_phys(), though it looks like the generic
implementation (wrongly) assumes a dma_addr_t type rather than
phys_addr_t. We have phys_do_dma() for such conversion as it needs to
take the device into account, so I guess most page_to_phys() uses under
drivers/ are wrong.
Maybe as a separate series clean-up those page_to_phys() uses in
drivers.
> free_reserved_page(page);
> }
> #endif
> diff --git a/mm/bootmem_info.c b/mm/bootmem_info.c
> index b1efebfcf94b..fa7cb0c87c03 100644
> --- a/mm/bootmem_info.c
> +++ b/mm/bootmem_info.c
> @@ -34,7 +34,7 @@ void put_page_bootmem(struct page *page)
> ClearPagePrivate(page);
> set_page_private(page, 0);
> INIT_LIST_HEAD(&page->lru);
> - kmemleak_free_part(page_to_virt(page), PAGE_SIZE);
> + kmemleak_free_part_phys(PFN_PHYS(page_to_pfn(page)), PAGE_SIZE);
> free_reserved_page(page);
> }
> }
> --
> 2.25.1
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning
2023-09-27 3:59 ` [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning Liu Shixin
@ 2023-09-27 17:06 ` Catalin Marinas
2023-09-28 1:22 ` Liu Shixin
0 siblings, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2023-09-27 17:06 UTC (permalink / raw
To: Liu Shixin; +Cc: Patrick Wang, Andrew Morton, linux-mm, linux-kernel
On Wed, Sep 27, 2023 at 11:59:22AM +0800, Liu Shixin wrote:
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 54c2c90d3abc..5a2bbd85df57 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -208,6 +208,8 @@ static struct rb_root object_tree_root = RB_ROOT;
> static struct rb_root object_phys_tree_root = RB_ROOT;
> /* protecting the access to object_list, object_tree_root (or object_phys_tree_root) */
> static DEFINE_RAW_SPINLOCK(kmemleak_lock);
> +/* Serial delete_object_part() to ensure all objects is deleted correctly */
> +static DEFINE_RAW_SPINLOCK(delete_object_part_mutex);
Don't call this mutex, it implies sleeping.
>
> /* allocation caches for kmemleak internal data */
> static struct kmem_cache *object_cache;
> @@ -784,13 +786,16 @@ static void delete_object_part(unsigned long ptr, size_t size, bool is_phys)
> {
> struct kmemleak_object *object;
> unsigned long start, end;
> + unsigned long flags;
>
> + raw_spin_lock_irqsave(&delete_object_part_mutex, flags);
> object = find_and_remove_object(ptr, 1, is_phys);
> if (!object) {
> #ifdef DEBUG
> kmemleak_warn("Partially freeing unknown object at 0x%08lx (size %zu)\n",
> ptr, size);
> #endif
> + raw_spin_unlock_irqrestore(&delete_object_part_mutex, flags);
I prefer a goto out and a single place for unlocking.
However, we already take the kmemleak_lock in find_and_remove_object().
So better to open-code that function here and avoid introducing a new
lock. __create_object() may need a new bool argument, no_lock or
something. Or just split it into separate functions for allocating the
kmemleak structure and adding it to the corresponding trees/lists under
a lock.
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning
2023-09-27 17:06 ` Catalin Marinas
@ 2023-09-28 1:22 ` Liu Shixin
0 siblings, 0 replies; 9+ messages in thread
From: Liu Shixin @ 2023-09-28 1:22 UTC (permalink / raw
To: Catalin Marinas; +Cc: Patrick Wang, Andrew Morton, linux-mm, linux-kernel
On 2023/9/28 1:06, Catalin Marinas wrote:
> On Wed, Sep 27, 2023 at 11:59:22AM +0800, Liu Shixin wrote:
>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>> index 54c2c90d3abc..5a2bbd85df57 100644
>> --- a/mm/kmemleak.c
>> +++ b/mm/kmemleak.c
>> @@ -208,6 +208,8 @@ static struct rb_root object_tree_root = RB_ROOT;
>> static struct rb_root object_phys_tree_root = RB_ROOT;
>> /* protecting the access to object_list, object_tree_root (or object_phys_tree_root) */
>> static DEFINE_RAW_SPINLOCK(kmemleak_lock);
>> +/* Serial delete_object_part() to ensure all objects is deleted correctly */
>> +static DEFINE_RAW_SPINLOCK(delete_object_part_mutex);
> Don't call this mutex, it implies sleeping.
Sorry, I used to define it as a mutex lock and forgot to change it.
>
>>
>> /* allocation caches for kmemleak internal data */
>> static struct kmem_cache *object_cache;
>> @@ -784,13 +786,16 @@ static void delete_object_part(unsigned long ptr, size_t size, bool is_phys)
>> {
>> struct kmemleak_object *object;
>> unsigned long start, end;
>> + unsigned long flags;
>>
>> + raw_spin_lock_irqsave(&delete_object_part_mutex, flags);
>> object = find_and_remove_object(ptr, 1, is_phys);
>> if (!object) {
>> #ifdef DEBUG
>> kmemleak_warn("Partially freeing unknown object at 0x%08lx (size %zu)\n",
>> ptr, size);
>> #endif
>> + raw_spin_unlock_irqrestore(&delete_object_part_mutex, flags);
> I prefer a goto out and a single place for unlocking.
>
> However, we already take the kmemleak_lock in find_and_remove_object().
> So better to open-code that function here and avoid introducing a new
> lock. __create_object() may need a new bool argument, no_lock or
> something. Or just split it into separate functions for allocating the
> kmemleak structure and adding it to the corresponding trees/lists under
> a lock.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] mm/kmemleak: fix print format of pointer in pr_debug()
2023-09-27 3:59 ` [PATCH 3/3] mm/kmemleak: fix print format of pointer in pr_debug() Liu Shixin
@ 2023-09-28 9:27 ` Catalin Marinas
0 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2023-09-28 9:27 UTC (permalink / raw
To: Liu Shixin; +Cc: Patrick Wang, Andrew Morton, linux-mm, linux-kernel
On Wed, Sep 27, 2023 at 11:59:23AM +0800, Liu Shixin wrote:
> With 0x%p, the pointer will be hashed and print (____ptrval____) instead.
> And with 0x%pa, the pointer can be successfully printed but with duplicate
> prefixes, which looks like:
>
> kmemleak: kmemleak_free(0x(____ptrval____))
> kmemleak: kmemleak_free_part_phys(0x0x0000000a1af86000)
>
> Use %pa instead of 0x%p or 0x%pa to print the pointer, and use 0x%px for
> __percpu pointer to prevent crash. Then the print will be like:
Why not %px in all cases?
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-28 9:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-27 3:59 [PATCH 0/3] Some bugfix about kmemleak Liu Shixin
2023-09-27 3:59 ` [PATCH 1/3] bootmem: use kmemleak_free_part_phys in free_bootmem_page/put_page_bootmem Liu Shixin
2023-09-27 16:05 ` Andrew Morton
2023-09-27 17:00 ` Catalin Marinas
2023-09-27 3:59 ` [PATCH 2/3] mm/kmemleak: fix partially freeing unknown object warning Liu Shixin
2023-09-27 17:06 ` Catalin Marinas
2023-09-28 1:22 ` Liu Shixin
2023-09-27 3:59 ` [PATCH 3/3] mm/kmemleak: fix print format of pointer in pr_debug() Liu Shixin
2023-09-28 9:27 ` Catalin Marinas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).