* [PATCH -next 1/5] mm/kmemleak: make create_object return void
2020-09-21 2:00 [PATCH -next 0/5] mm/kmemleak：support for percpu memory leak detect Chen Jun
@ 2020-09-21 2:00 ` Chen Jun
2020-09-22 9:57 ` Catalin Marinas
2020-09-21 2:00 ` [PATCH -next 2/5] mm/kmemleak: skip update_checksum for OBJECT_NO_SCAN objects Chen Jun
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Chen Jun @ 2020-09-21 2:00 UTC (permalink / raw)
To: linux-kernel, linux-mm; +Cc: catalin.marinas, akpm, rui.xiang, weiyongjun1
From: Wei Yongjun <weiyongjun1@huawei.com>
No user cares about the return value of create_object,
so make it return void.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
---
mm/kmemleak.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index c0014d3b91c1..b3f603fd9fc3 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -567,8 +567,8 @@ static int __save_stack_trace(unsigned long *trace)
* Create the metadata (struct kmemleak_object) corresponding to an allocated
* memory block and add it to the object_list and object_tree_root.
*/
-static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
- int min_count, gfp_t gfp)
+static void create_object(unsigned long ptr, size_t size, int min_count,
+ gfp_t gfp)
{
unsigned long flags;
struct kmemleak_object *object, *parent;
@@ -579,7 +579,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
if (!object) {
pr_warn("Cannot allocate a kmemleak_object structure\n");
kmemleak_disable();
- return NULL;
+ return;
}
INIT_LIST_HEAD(&object->object_list);
@@ -640,7 +640,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
*/
dump_object_info(parent);
kmem_cache_free(object_cache, object);
- object = NULL;
goto out;
}
}
@@ -650,7 +649,6 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
list_add_tail_rcu(&object->object_list, &object_list);
out:
raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
- return object;
}
/*
--
2.25.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH -next 2/5] mm/kmemleak: skip update_checksum for OBJECT_NO_SCAN objects
2020-09-21 2:00 [PATCH -next 0/5] mm/kmemleak：support for percpu memory leak detect Chen Jun
2020-09-21 2:00 ` [PATCH -next 1/5] mm/kmemleak: make create_object return void Chen Jun
@ 2020-09-21 2:00 ` Chen Jun
2020-09-22 9:03 ` Catalin Marinas
2020-09-21 2:00 ` [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect Chen Jun
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Chen Jun @ 2020-09-21 2:00 UTC (permalink / raw)
To: linux-kernel, linux-mm; +Cc: catalin.marinas, akpm, rui.xiang, weiyongjun1
From: Wei Yongjun <weiyongjun1@huawei.com>
Objects marked with OBJECT_NO_SCAN are never scanned.
So there is no need to update checksum for them.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
---
mm/kmemleak.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index b3f603fd9fc3..c09c6b59eda6 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1166,6 +1166,10 @@ static bool update_checksum(struct kmemleak_object *object)
{
u32 old_csum = object->checksum;
+ /* always return false for not scan object */
+ if (object->flags & OBJECT_NO_SCAN)
+ return false;
+
kasan_disable_current();
kcsan_disable_current();
object->checksum = crc32(0, (void *)object->pointer, object->size);
--
2.25.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH -next 2/5] mm/kmemleak: skip update_checksum for OBJECT_NO_SCAN objects
2020-09-21 2:00 ` [PATCH -next 2/5] mm/kmemleak: skip update_checksum for OBJECT_NO_SCAN objects Chen Jun
@ 2020-09-22 9:03 ` Catalin Marinas
0 siblings, 0 replies; 12+ messages in thread
From: Catalin Marinas @ 2020-09-22 9:03 UTC (permalink / raw)
To: Chen Jun; +Cc: linux-kernel, linux-mm, akpm, rui.xiang, weiyongjun1
On Mon, Sep 21, 2020 at 02:00:04AM +0000, Chen Jun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Objects marked with OBJECT_NO_SCAN are never scanned.
> So there is no need to update checksum for them.
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> Signed-off-by: Chen Jun <chenjun102@huawei.com>
> ---
> mm/kmemleak.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index b3f603fd9fc3..c09c6b59eda6 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1166,6 +1166,10 @@ static bool update_checksum(struct kmemleak_object *object)
> {
> u32 old_csum = object->checksum;
>
> + /* always return false for not scan object */
> + if (object->flags & OBJECT_NO_SCAN)
> + return false;
The reason for OBJECT_NO_SCAN is to avoid introducing more false
negatives. The point of the checksum is to reduce the false positives -
i.e. an object that is being modified between scans won't be considered
a (transient) leak even if kmemleak couldn't find a reference pointer to
it.
So please drop this patch.
--
Catalin
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect
2020-09-21 2:00 [PATCH -next 0/5] mm/kmemleak：support for percpu memory leak detect Chen Jun
2020-09-21 2:00 ` [PATCH -next 1/5] mm/kmemleak: make create_object return void Chen Jun
2020-09-21 2:00 ` [PATCH -next 2/5] mm/kmemleak: skip update_checksum for OBJECT_NO_SCAN objects Chen Jun
@ 2020-09-21 2:00 ` Chen Jun
2020-09-22 9:57 ` Catalin Marinas
2020-09-21 2:00 ` [PATCH -next 4/5] mm/kmemleak-test: use %px instead of %p in print Chen Jun
2020-09-21 2:00 ` [PATCH -next 5/5] mm/kmemleak-test: Add a test case for alloc_percpu Chen Jun
4 siblings, 1 reply; 12+ messages in thread
From: Chen Jun @ 2020-09-21 2:00 UTC (permalink / raw)
To: linux-kernel, linux-mm; +Cc: catalin.marinas, akpm, rui.xiang, weiyongjun1
From: Wei Yongjun <weiyongjun1@huawei.com>
Currently the reporting of the percpu chunks leaking problem
are not supported. This patch introduces this function.
Since __percpu pointer is not pointing directly to the actual chunks,
this patch creates an object for __percpu pointer, but marks it as no
scan block, only check whether this pointer is referenced by other
blocks.
Introduce two global variables, min_percpu_addr and max_percpu_addr,
to store the range of valid percpu pointer values, in order to
speed up pointer lookup when scanning blocks.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
---
mm/kmemleak.c | 71 ++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 59 insertions(+), 12 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index c09c6b59eda6..feedb72f06f2 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -170,6 +170,8 @@ struct kmemleak_object {
#define OBJECT_NO_SCAN (1 << 2)
/* flag set to fully scan the object when scan_area allocation failed */
#define OBJECT_FULL_SCAN (1 << 3)
+/* flag set to percpu ptr object */
+#define OBJECT_PERCPU (1 << 4)
#define HEX_PREFIX " "
/* number of bytes to print per line; must be 16 or 32 */
@@ -212,6 +214,9 @@ static int kmemleak_error;
/* minimum and maximum address that may be valid pointers */
static unsigned long min_addr = ULONG_MAX;
static unsigned long max_addr;
+/* minimum and maximum address that may be valid percpu pointers */
+static unsigned long min_percpu_addr = ULONG_MAX;
+static unsigned long max_percpu_addr;
static struct task_struct *scan_thread;
/* used to avoid reporting of recently allocated objects */
@@ -283,6 +288,9 @@ static void hex_dump_object(struct seq_file *seq,
const u8 *ptr = (const u8 *)object->pointer;
size_t len;
+ if (object->flags & OBJECT_PERCPU)
+ ptr = this_cpu_ptr((void __percpu *)object->pointer);
+
/* limit the number of lines to HEX_MAX_LINES */
len = min_t(size_t, object->size, HEX_MAX_LINES * HEX_ROW_SIZE);
@@ -563,17 +571,32 @@ static int __save_stack_trace(unsigned long *trace)
return stack_trace_save(trace, MAX_TRACE, 2);
}
+static void __update_address_range(struct kmemleak_object *object)
+{
+ unsigned long ptr = object->pointer;
+ size_t size = object->size;
+ unsigned long untagged_ptr;
+
+ if (object->flags & OBJECT_PERCPU) {
+ min_percpu_addr = min(min_percpu_addr, ptr);
+ max_percpu_addr = max(max_percpu_addr, ptr + size);
+ } else {
+ untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
+ min_addr = min(min_addr, untagged_ptr);
+ max_addr = max(max_addr, untagged_ptr + size);
+ }
+}
+
/*
* Create the metadata (struct kmemleak_object) corresponding to an allocated
* memory block and add it to the object_list and object_tree_root.
*/
-static void create_object(unsigned long ptr, size_t size, int min_count,
- gfp_t gfp)
+static void __create_object(unsigned long ptr, size_t size, int min_count,
+ unsigned int obj_flags, gfp_t gfp)
{
unsigned long flags;
struct kmemleak_object *object, *parent;
struct rb_node **link, *rb_parent;
- unsigned long untagged_ptr;
object = mem_pool_alloc(gfp);
if (!object) {
@@ -587,7 +610,7 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
INIT_HLIST_HEAD(&object->area_list);
raw_spin_lock_init(&object->lock);
atomic_set(&object->use_count, 1);
- object->flags = OBJECT_ALLOCATED;
+ object->flags = OBJECT_ALLOCATED | obj_flags;
object->pointer = ptr;
object->size = size;
object->excess_ref = 0;
@@ -619,9 +642,7 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
raw_spin_lock_irqsave(&kmemleak_lock, flags);
- untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
- min_addr = min(min_addr, untagged_ptr);
- max_addr = max(max_addr, untagged_ptr + size);
+ __update_address_range(object);
link = &object_tree_root.rb_node;
rb_parent = NULL;
while (*link) {
@@ -651,6 +672,19 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
}
+static void create_object(unsigned long ptr, size_t size, int min_count,
+ gfp_t gfp)
+{
+ __create_object(ptr, size, min_count, 0, gfp);
+}
+
+static void create_object_percpu(unsigned long ptr, size_t size, int min_count,
+ gfp_t gfp)
+{
+ __create_object(ptr, size, min_count, OBJECT_PERCPU | OBJECT_NO_SCAN,
+ gfp);
+}
+
/*
* Mark the object as not allocated and schedule RCU freeing via put_object().
*/
@@ -912,10 +946,12 @@ void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
* Percpu allocations are only scanned and not reported as leaks
* (min_count is set to 0).
*/
- if (kmemleak_enabled && ptr && !IS_ERR(ptr))
+ if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
for_each_possible_cpu(cpu)
create_object((unsigned long)per_cpu_ptr(ptr, cpu),
size, 0, gfp);
+ create_object_percpu((unsigned long)ptr, size, 1, gfp);
+ }
}
EXPORT_SYMBOL_GPL(kmemleak_alloc_percpu);
@@ -991,10 +1027,12 @@ void __ref kmemleak_free_percpu(const void __percpu *ptr)
pr_debug("%s(0x%p)\n", __func__, ptr);
- if (kmemleak_free_enabled && ptr && !IS_ERR(ptr))
+ if (kmemleak_free_enabled && ptr && !IS_ERR(ptr)) {
for_each_possible_cpu(cpu)
delete_object_full((unsigned long)per_cpu_ptr(ptr,
cpu));
+ delete_object_full((unsigned long)ptr);
+ }
}
EXPORT_SYMBOL_GPL(kmemleak_free_percpu);
@@ -1224,6 +1262,17 @@ static int scan_should_stop(void)
return 0;
}
+static bool is_valid_address(unsigned long ptr)
+{
+ unsigned long untagged_ptr;
+
+ if (ptr >= min_percpu_addr && ptr < max_percpu_addr)
+ return true;
+
+ untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
+ return (untagged_ptr >= min_addr && untagged_ptr < max_addr);
+}
+
/*
* Scan a memory block (exclusive range) for valid pointers and add those
* found to the gray list.
@@ -1235,7 +1284,6 @@ static void scan_block(void *_start, void *_end,
unsigned long *start = PTR_ALIGN(_start, BYTES_PER_POINTER);
unsigned long *end = _end - (BYTES_PER_POINTER - 1);
unsigned long flags;
- unsigned long untagged_ptr;
raw_spin_lock_irqsave(&kmemleak_lock, flags);
for (ptr = start; ptr < end; ptr++) {
@@ -1250,8 +1298,7 @@ static void scan_block(void *_start, void *_end,
pointer = *ptr;
kasan_enable_current();
- untagged_ptr = (unsigned long)kasan_reset_tag((void *)pointer);
- if (untagged_ptr < min_addr || untagged_ptr >= max_addr)
+ if (!is_valid_address(pointer))
continue;
/*
--
2.25.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect
2020-09-21 2:00 ` [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect Chen Jun
@ 2020-09-22 9:57 ` Catalin Marinas
2020-09-28 14:08 ` chenjun (AM)
0 siblings, 1 reply; 12+ messages in thread
From: Catalin Marinas @ 2020-09-22 9:57 UTC (permalink / raw)
To: Chen Jun; +Cc: linux-kernel, linux-mm, akpm, rui.xiang, weiyongjun1
On Mon, Sep 21, 2020 at 02:00:05AM +0000, Chen Jun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Currently the reporting of the percpu chunks leaking problem
> are not supported. This patch introduces this function.
>
> Since __percpu pointer is not pointing directly to the actual chunks,
> this patch creates an object for __percpu pointer, but marks it as no
> scan block, only check whether this pointer is referenced by other
> blocks.
OK, so you wanted NO_SCAN to not touch the block at all, not even update
the checksum. Maybe better add a new flag, NO_ACCESS (and we could use
it to track ioremap leaks, it's been on my wishlist for years).
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index c09c6b59eda6..feedb72f06f2 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -283,6 +288,9 @@ static void hex_dump_object(struct seq_file *seq,
> const u8 *ptr = (const u8 *)object->pointer;
> size_t len;
>
> + if (object->flags & OBJECT_PERCPU)
> + ptr = this_cpu_ptr((void __percpu *)object->pointer);
You may want to print the CPU number as well since the information is
likely different on another CPU. Also, I think this context is
preemptable, so it's better with a get_cpu/put_cpu().
> @@ -651,6 +672,19 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
> raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> }
>
> +static void create_object(unsigned long ptr, size_t size, int min_count,
> + gfp_t gfp)
> +{
> + __create_object(ptr, size, min_count, 0, gfp);
> +}
> +
> +static void create_object_percpu(unsigned long ptr, size_t size, int min_count,
> + gfp_t gfp)
> +{
> + __create_object(ptr, size, min_count, OBJECT_PERCPU | OBJECT_NO_SCAN,
> + gfp);
> +}
> +
> /*
> * Mark the object as not allocated and schedule RCU freeing via put_object().
> */
> @@ -912,10 +946,12 @@ void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
> * Percpu allocations are only scanned and not reported as leaks
> * (min_count is set to 0).
> */
> - if (kmemleak_enabled && ptr && !IS_ERR(ptr))
> + if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
> for_each_possible_cpu(cpu)
> create_object((unsigned long)per_cpu_ptr(ptr, cpu),
> size, 0, gfp);
> + create_object_percpu((unsigned long)ptr, size, 1, gfp);
> + }
> }
A concern I have here is that ptr may overlap with an existing object
and the insertion in the rb tree will fail. For example, with !SMP,
ptr == per_cpu_ptr(ptr, 0), so create_object() will fail and kmemleak
gets disabled.
An option would to figure out how to allow overlapping ranges with rb
tree (or find a replacement for it if not possible).
Another option would be to have an additional structure to track the
__percpu pointers since they have their own range. If size is not
relevant, maybe go for an xarray, otherwise another rb tree (do we have
any instance of pointers referring some inner member of a __percpu
object?). The scan_object() function will have to search two trees.
--
Catalin
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect
2020-09-22 9:57 ` Catalin Marinas
@ 2020-09-28 14:08 ` chenjun (AM)
2020-09-28 14:16 ` Catalin Marinas
0 siblings, 1 reply; 12+ messages in thread
From: chenjun (AM) @ 2020-09-28 14:08 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-kernel, linux-mm, akpm, Xiangrui (Euler), weiyongjun (A)
Hi Catalin
Thanks for your opinions.
在 2020/9/22 17:58, Catalin Marinas 写道:
> On Mon, Sep 21, 2020 at 02:00:05AM +0000, Chen Jun wrote:
>> From: Wei Yongjun <weiyongjun1@huawei.com>
>>
>> Currently the reporting of the percpu chunks leaking problem
>> are not supported. This patch introduces this function.
>>
>> Since __percpu pointer is not pointing directly to the actual chunks,
>> this patch creates an object for __percpu pointer, but marks it as no
>> scan block, only check whether this pointer is referenced by other
>> blocks.
>
> OK, so you wanted NO_SCAN to not touch the block at all, not even update
> the checksum. Maybe better add a new flag, NO_ACCESS (and we could use
> it to track ioremap leaks, it's been on my wishlist for years).
>
I will add a new OBJECT_NO_ACCESS.
The checksum of the object will not be updated and its memory block will
not be scanned if the object marked with OBJECT_NO_ACCESS.
>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>> index c09c6b59eda6..feedb72f06f2 100644
>> --- a/mm/kmemleak.c
>> +++ b/mm/kmemleak.c
>> @@ -283,6 +288,9 @@ static void hex_dump_object(struct seq_file *seq,
>> const u8 *ptr = (const u8 *)object->pointer;
>> size_t len;
>>
>> + if (object->flags & OBJECT_PERCPU)
>> + ptr = this_cpu_ptr((void __percpu *)object->pointer);
>
> You may want to print the CPU number as well since the information is
> likely different on another CPU. Also, I think this context is
> preemptable, so it's better with a get_cpu/put_cpu().
>
I will print cpu number when dump the percpu object.
>> @@ -651,6 +672,19 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
>> raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
>> }
>>
>> +static void create_object(unsigned long ptr, size_t size, int min_count,
>> + gfp_t gfp)
>> +{
>> + __create_object(ptr, size, min_count, 0, gfp);
>> +}
>> +
>> +static void create_object_percpu(unsigned long ptr, size_t size, int min_count,
>> + gfp_t gfp)
>> +{
>> + __create_object(ptr, size, min_count, OBJECT_PERCPU | OBJECT_NO_SCAN,
>> + gfp);
>> +}
>> +
>> /*
>> * Mark the object as not allocated and schedule RCU freeing via put_object().
>> */
>> @@ -912,10 +946,12 @@ void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
>> * Percpu allocations are only scanned and not reported as leaks
>> * (min_count is set to 0).
>> */
>> - if (kmemleak_enabled && ptr && !IS_ERR(ptr))
>> + if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
>> for_each_possible_cpu(cpu)
>> create_object((unsigned long)per_cpu_ptr(ptr, cpu),
>> size, 0, gfp);
>> + create_object_percpu((unsigned long)ptr, size, 1, gfp);
>> + }
>> }
>
> A concern I have here is that ptr may overlap with an existing object
> and the insertion in the rb tree will fail. For example, with !SMP,
> ptr == per_cpu_ptr(ptr, 0), so create_object() will fail and kmemleak
> gets disabled.
>
> An option would to figure out how to allow overlapping ranges with rb
> tree (or find a replacement for it if not possible).
>
> Another option would be to have an additional structure to track the
> __percpu pointers since they have their own range. If size is not
> relevant, maybe go for an xarray, otherwise another rb tree (do we have
> any instance of pointers referring some inner member of a __percpu
> object?). The scan_object() function will have to search two trees.
>
I would like to use CONFIG_SMP to seprate code:
if SMP, we will create some objects for per_cpu_ptr(ptr, cpu) and an
object with OBJECT_NO_ACCESS for ptr.
if !SMP, we will not create object for per_cpu_ptr(ptr,cpu), but an
object without OBJECT_NO_ACCESS for ptr will be created.
What do you think about this opinion.
Waiting for your reply
Best wishes
Jun
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect
2020-09-28 14:08 ` chenjun (AM)
@ 2020-09-28 14:16 ` Catalin Marinas
0 siblings, 0 replies; 12+ messages in thread
From: Catalin Marinas @ 2020-09-28 14:16 UTC (permalink / raw)
To: chenjun (AM)
Cc: linux-kernel, linux-mm, akpm, Xiangrui (Euler), weiyongjun (A)
On Mon, Sep 28, 2020 at 02:08:29PM +0000, chenjun (AM) wrote:
> On Mon, Sep 21, 2020 at 02:00:05AM +0000, Chen Jun wrote:
> > From: Wei Yongjun <weiyongjun1@huawei.com>
> >> @@ -651,6 +672,19 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
> >> raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> >> }
> >>
> >> +static void create_object(unsigned long ptr, size_t size, int min_count,
> >> + gfp_t gfp)
> >> +{
> >> + __create_object(ptr, size, min_count, 0, gfp);
> >> +}
> >> +
> >> +static void create_object_percpu(unsigned long ptr, size_t size, int min_count,
> >> + gfp_t gfp)
> >> +{
> >> + __create_object(ptr, size, min_count, OBJECT_PERCPU | OBJECT_NO_SCAN,
> >> + gfp);
> >> +}
> >> +
> >> /*
> >> * Mark the object as not allocated and schedule RCU freeing via put_object().
> >> */
> >> @@ -912,10 +946,12 @@ void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
> >> * Percpu allocations are only scanned and not reported as leaks
> >> * (min_count is set to 0).
> >> */
> >> - if (kmemleak_enabled && ptr && !IS_ERR(ptr))
> >> + if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
> >> for_each_possible_cpu(cpu)
> >> create_object((unsigned long)per_cpu_ptr(ptr, cpu),
> >> size, 0, gfp);
> >> + create_object_percpu((unsigned long)ptr, size, 1, gfp);
> >> + }
> >> }
> >
> > A concern I have here is that ptr may overlap with an existing object
> > and the insertion in the rb tree will fail. For example, with !SMP,
> > ptr == per_cpu_ptr(ptr, 0), so create_object() will fail and kmemleak
> > gets disabled.
> >
> > An option would to figure out how to allow overlapping ranges with rb
> > tree (or find a replacement for it if not possible).
> >
> > Another option would be to have an additional structure to track the
> > __percpu pointers since they have their own range. If size is not
> > relevant, maybe go for an xarray, otherwise another rb tree (do we have
> > any instance of pointers referring some inner member of a __percpu
> > object?). The scan_object() function will have to search two trees.
>
> I would like to use CONFIG_SMP to seprate code:
> if SMP, we will create some objects for per_cpu_ptr(ptr, cpu) and an
> object with OBJECT_NO_ACCESS for ptr.
> if !SMP, we will not create object for per_cpu_ptr(ptr,cpu), but an
> object without OBJECT_NO_ACCESS for ptr will be created.
> What do you think about this opinion.
The !SMP case was just an example. Do you have a guarantee that the
value of the __percpu ptr doesn't clash with a linear map address?
--
Catalin
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH -next 4/5] mm/kmemleak-test: use %px instead of %p in print
2020-09-21 2:00 [PATCH -next 0/5] mm/kmemleak：support for percpu memory leak detect Chen Jun
` (2 preceding siblings ...)
2020-09-21 2:00 ` [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect Chen Jun
@ 2020-09-21 2:00 ` Chen Jun
2020-09-22 9:58 ` Catalin Marinas
2020-09-21 2:00 ` [PATCH -next 5/5] mm/kmemleak-test: Add a test case for alloc_percpu Chen Jun
4 siblings, 1 reply; 12+ messages in thread
From: Chen Jun @ 2020-09-21 2:00 UTC (permalink / raw)
To: linux-kernel, linux-mm; +Cc: catalin.marinas, akpm, rui.xiang, weiyongjun1
From: Wei Yongjun <weiyongjun1@huawei.com>
Real addresses are used for diagnose issues.
Convert %p with %px to print kernel addresses.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
---
mm/kmemleak-test.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/mm/kmemleak-test.c b/mm/kmemleak-test.c
index e19279ff6aa3..75fe1b8c3226 100644
--- a/mm/kmemleak-test.c
+++ b/mm/kmemleak-test.c
@@ -40,25 +40,25 @@ static int __init kmemleak_test_init(void)
pr_info("Kmemleak testing\n");
/* make some orphan objects */
- pr_info("kmalloc(32) = %p\n", kmalloc(32, GFP_KERNEL));
- pr_info("kmalloc(32) = %p\n", kmalloc(32, GFP_KERNEL));
- pr_info("kmalloc(1024) = %p\n", kmalloc(1024, GFP_KERNEL));
- pr_info("kmalloc(1024) = %p\n", kmalloc(1024, GFP_KERNEL));
- pr_info("kmalloc(2048) = %p\n", kmalloc(2048, GFP_KERNEL));
- pr_info("kmalloc(2048) = %p\n", kmalloc(2048, GFP_KERNEL));
- pr_info("kmalloc(4096) = %p\n", kmalloc(4096, GFP_KERNEL));
- pr_info("kmalloc(4096) = %p\n", kmalloc(4096, GFP_KERNEL));
+ pr_info("kmalloc(32) = %px\n", kmalloc(32, GFP_KERNEL));
+ pr_info("kmalloc(32) = %px\n", kmalloc(32, GFP_KERNEL));
+ pr_info("kmalloc(1024) = %px\n", kmalloc(1024, GFP_KERNEL));
+ pr_info("kmalloc(1024) = %px\n", kmalloc(1024, GFP_KERNEL));
+ pr_info("kmalloc(2048) = %px\n", kmalloc(2048, GFP_KERNEL));
+ pr_info("kmalloc(2048) = %px\n", kmalloc(2048, GFP_KERNEL));
+ pr_info("kmalloc(4096) = %px\n", kmalloc(4096, GFP_KERNEL));
+ pr_info("kmalloc(4096) = %px\n", kmalloc(4096, GFP_KERNEL));
#ifndef CONFIG_MODULES
- pr_info("kmem_cache_alloc(files_cachep) = %p\n",
+ pr_info("kmem_cache_alloc(files_cachep) = %px\n",
kmem_cache_alloc(files_cachep, GFP_KERNEL));
- pr_info("kmem_cache_alloc(files_cachep) = %p\n",
+ pr_info("kmem_cache_alloc(files_cachep) = %px\n",
kmem_cache_alloc(files_cachep, GFP_KERNEL));
#endif
- pr_info("vmalloc(64) = %p\n", vmalloc(64));
- pr_info("vmalloc(64) = %p\n", vmalloc(64));
- pr_info("vmalloc(64) = %p\n", vmalloc(64));
- pr_info("vmalloc(64) = %p\n", vmalloc(64));
- pr_info("vmalloc(64) = %p\n", vmalloc(64));
+ pr_info("vmalloc(64) = %px\n", vmalloc(64));
+ pr_info("vmalloc(64) = %px\n", vmalloc(64));
+ pr_info("vmalloc(64) = %px\n", vmalloc(64));
+ pr_info("vmalloc(64) = %px\n", vmalloc(64));
+ pr_info("vmalloc(64) = %px\n", vmalloc(64));
/*
* Add elements to a list. They should only appear as orphan
@@ -66,7 +66,7 @@ static int __init kmemleak_test_init(void)
*/
for (i = 0; i < 10; i++) {
elem = kzalloc(sizeof(*elem), GFP_KERNEL);
- pr_info("kzalloc(sizeof(*elem)) = %p\n", elem);
+ pr_info("kzalloc(sizeof(*elem)) = %px\n", elem);
if (!elem)
return -ENOMEM;
INIT_LIST_HEAD(&elem->list);
@@ -75,7 +75,7 @@ static int __init kmemleak_test_init(void)
for_each_possible_cpu(i) {
per_cpu(kmemleak_test_pointer, i) = kmalloc(129, GFP_KERNEL);
- pr_info("kmalloc(129) = %p\n",
+ pr_info("kmalloc(129) = %px\n",
per_cpu(kmemleak_test_pointer, i));
}
--
2.25.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH -next 5/5] mm/kmemleak-test: Add a test case for alloc_percpu
2020-09-21 2:00 [PATCH -next 0/5] mm/kmemleak：support for percpu memory leak detect Chen Jun
` (3 preceding siblings ...)
2020-09-21 2:00 ` [PATCH -next 4/5] mm/kmemleak-test: use %px instead of %p in print Chen Jun
@ 2020-09-21 2:00 ` Chen Jun
4 siblings, 0 replies; 12+ messages in thread
From: Chen Jun @ 2020-09-21 2:00 UTC (permalink / raw)
To: linux-kernel, linux-mm; +Cc: catalin.marinas, akpm, rui.xiang, weiyongjun1
After insmod kmemleak-test.ko, a leaking for alloc_percpu will be
reported as below:
kmemleak: alloc_percpu(sizeof(*elem)) = 00007dfdd26a2c40
:
:
unreferenced object 0x7dfdd26a2c40 (size 8):
comm "insmod", pid 183, jiffies 4294905864 (age 40.520s)
hex dump (first 8 bytes):
00 00 00 00 00 00 00 00 ........
backtrace:
[<(____ptrval____)>] pcpu_alloc+0x3ec/0x8c8
[<(____ptrval____)>] __alloc_percpu+0x18/0x28
[<(____ptrval____)>] 0xffff800008df525c
[<(____ptrval____)>] do_one_initcall+0x60/0x1d8
[<(____ptrval____)>] do_init_module+0x58/0x1d0
[<(____ptrval____)>] load_module+0x1d8c/0x23d0
[<(____ptrval____)>] __do_sys_finit_module+0xdc/0xf8
[<(____ptrval____)>] __arm64_sys_finit_module+0x20/0x30
[<(____ptrval____)>] el0_svc_common.constprop.3+0x68/0x170
[<(____ptrval____)>] do_el0_svc+0x24/0x90
[<(____ptrval____)>] el0_sync_handler+0x13c/0x1a8
[<(____ptrval____)>] el0_sync+0x158/0x180
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
---
mm/kmemleak-test.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/kmemleak-test.c b/mm/kmemleak-test.c
index 75fe1b8c3226..8f3e4a60166b 100644
--- a/mm/kmemleak-test.c
+++ b/mm/kmemleak-test.c
@@ -79,6 +79,9 @@ static int __init kmemleak_test_init(void)
per_cpu(kmemleak_test_pointer, i));
}
+ pr_info("alloc_percpu(sizeof(*elem)) = %px\n",
+ alloc_percpu(sizeof(*elem)));
+
return 0;
}
module_init(kmemleak_test_init);
--
2.25.0
^ permalink raw reply related [flat|nested] 12+ messages in thread