* [PATCH] mm: skip reserved page for kmem leak scanning
@ 2022-08-26 3:12 zhaoyang.huang
2022-08-26 3:23 ` Zhaoyang Huang
0 siblings, 1 reply; 6+ messages in thread
From: zhaoyang.huang @ 2022-08-26 3:12 UTC (permalink / raw)
To: Andrew Morton, Catalin Marinas, Zhaoyang Huang, linux-mm,
linux-kernel, ke.wang
From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
It is no need to scan reserved page, skip it.
Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
---
mm/kmemleak.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index a182f5d..c546250 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
if (page_zone(page) != zone)
continue;
/* only scan if page is in use */
- if (page_count(page) == 0)
+ if (page_count(page) == 0 && !PageReserved(page))
continue;
scan_block(page, page + 1, NULL);
if (!(pfn & 63))
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: skip reserved page for kmem leak scanning
2022-08-26 3:12 [PATCH] mm: skip reserved page for kmem leak scanning zhaoyang.huang
@ 2022-08-26 3:23 ` Zhaoyang Huang
2022-08-29 12:18 ` David Hildenbrand
0 siblings, 1 reply; 6+ messages in thread
From: Zhaoyang Huang @ 2022-08-26 3:23 UTC (permalink / raw)
To: zhaoyang.huang
Cc: Andrew Morton, Catalin Marinas, open list:MEMORY MANAGEMENT, LKML,
Ke Wang
On Fri, Aug 26, 2022 at 11:13 AM zhaoyang.huang
<zhaoyang.huang@unisoc.com> wrote:
>
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>
> It is no need to scan reserved page, skip it.
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> ---
> mm/kmemleak.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index a182f5d..c546250 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
> if (page_zone(page) != zone)
> continue;
> /* only scan if page is in use */
> - if (page_count(page) == 0)
> + if (page_count(page) == 0 || PageReserved(page))
Sorry for previous stupid code by my faint, correct it here
> continue;
> scan_block(page, page + 1, NULL);
> if (!(pfn & 63))
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: skip reserved page for kmem leak scanning
2022-08-26 3:23 ` Zhaoyang Huang
@ 2022-08-29 12:18 ` David Hildenbrand
2022-08-30 2:41 ` Zhaoyang Huang
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2022-08-29 12:18 UTC (permalink / raw)
To: Zhaoyang Huang, zhaoyang.huang
Cc: Andrew Morton, Catalin Marinas, open list:MEMORY MANAGEMENT, LKML,
Ke Wang
On 26.08.22 05:23, Zhaoyang Huang wrote:
> On Fri, Aug 26, 2022 at 11:13 AM zhaoyang.huang
> <zhaoyang.huang@unisoc.com> wrote:
>>
>> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>>
>> It is no need to scan reserved page, skip it.
>>
>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>> ---
>> mm/kmemleak.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>> index a182f5d..c546250 100644
>> --- a/mm/kmemleak.c
>> +++ b/mm/kmemleak.c
>> @@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
>> if (page_zone(page) != zone)
>> continue;
>> /* only scan if page is in use */
>> - if (page_count(page) == 0)
>> + if (page_count(page) == 0 || PageReserved(page))
> Sorry for previous stupid code by my faint, correct it here
Did you even test the initial patch?
I wonder why we should consider this change
(a) I doubt it's a performance issue. If it is, please provide numbers
before/after.
(b) We'll stop scanning early allocations. As the memmap is usually
allocated early during boot ... we'll stop scanning essentially the
whole mmap and that whole loop would be dead code? What am i
missing?
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: skip reserved page for kmem leak scanning
2022-08-29 12:18 ` David Hildenbrand
@ 2022-08-30 2:41 ` Zhaoyang Huang
2022-08-30 8:03 ` David Hildenbrand
0 siblings, 1 reply; 6+ messages in thread
From: Zhaoyang Huang @ 2022-08-30 2:41 UTC (permalink / raw)
To: David Hildenbrand
Cc: zhaoyang.huang, Andrew Morton, Catalin Marinas,
open list:MEMORY MANAGEMENT, LKML, Ke Wang
On Mon, Aug 29, 2022 at 8:19 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 26.08.22 05:23, Zhaoyang Huang wrote:
> > On Fri, Aug 26, 2022 at 11:13 AM zhaoyang.huang
> > <zhaoyang.huang@unisoc.com> wrote:
> >>
> >> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> >>
> >> It is no need to scan reserved page, skip it.
> >>
> >> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> >> ---
> >> mm/kmemleak.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> >> index a182f5d..c546250 100644
> >> --- a/mm/kmemleak.c
> >> +++ b/mm/kmemleak.c
> >> @@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
> >> if (page_zone(page) != zone)
> >> continue;
> >> /* only scan if page is in use */
> >> - if (page_count(page) == 0)
> >> + if (page_count(page) == 0 || PageReserved(page))
> > Sorry for previous stupid code by my faint, correct it here
>
> Did you even test the initial patch?
>
> I wonder why we should consider this change
>
> (a) I doubt it's a performance issue. If it is, please provide numbers
> before/after.
For Android-like SOC systems where AP(cpu runs linux) are one of the
memory consumers which are composed of other processors such as modem,
isp,wcn etc. The reserved memory occupies a certain number of
memory(could be 30% of MemTotal) which makes scan reserved pages
pointless.
> (b) We'll stop scanning early allocations. As the memmap is usually
> allocated early during boot ... we'll stop scanning essentially the
> whole mmap and that whole loop would be dead code? What am i
> missing?
memmap refers to pages here? If we can surpass these as it exist
permanently during life period. Besides, I wonder if PageLRU should
also be skipped?
- if (page_count(page) == 0)
+ if (page_count(page) == 0 ||
PageReserved(page) || PageLRU(page))
>
> --
> Thanks,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: skip reserved page for kmem leak scanning
2022-08-30 2:41 ` Zhaoyang Huang
@ 2022-08-30 8:03 ` David Hildenbrand
2022-08-30 8:52 ` Zhaoyang Huang
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2022-08-30 8:03 UTC (permalink / raw)
To: Zhaoyang Huang
Cc: zhaoyang.huang, Andrew Morton, Catalin Marinas,
open list:MEMORY MANAGEMENT, LKML, Ke Wang
On 30.08.22 04:41, Zhaoyang Huang wrote:
> On Mon, Aug 29, 2022 at 8:19 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 26.08.22 05:23, Zhaoyang Huang wrote:
>>> On Fri, Aug 26, 2022 at 11:13 AM zhaoyang.huang
>>> <zhaoyang.huang@unisoc.com> wrote:
>>>>
>>>> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>>>>
>>>> It is no need to scan reserved page, skip it.
>>>>
>>>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>>>> ---
>>>> mm/kmemleak.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>>>> index a182f5d..c546250 100644
>>>> --- a/mm/kmemleak.c
>>>> +++ b/mm/kmemleak.c
>>>> @@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
>>>> if (page_zone(page) != zone)
>>>> continue;
>>>> /* only scan if page is in use */
>>>> - if (page_count(page) == 0)
>>>> + if (page_count(page) == 0 || PageReserved(page))
>>> Sorry for previous stupid code by my faint, correct it here
>>
>> Did you even test the initial patch?
>>
>> I wonder why we should consider this change
>>
>> (a) I doubt it's a performance issue. If it is, please provide numbers
>> before/after.
> For Android-like SOC systems where AP(cpu runs linux) are one of the
> memory consumers which are composed of other processors such as modem,
> isp,wcn etc. The reserved memory occupies a certain number of
> memory(could be 30% of MemTotal) which makes scan reserved pages
> pointless.
But we only scan the memmap (struct page) here and not the actual
memory. Do you have any performance numbers showing that there is even
an observable change?
>> (b) We'll stop scanning early allocations. As the memmap is usually
>> allocated early during boot ... we'll stop scanning essentially the
>> whole mmap and that whole loop would be dead code? What am i
>> missing?
> memmap refers to pages here? If we can surpass these as it exist
> permanently during life period. Besides, I wonder if PageLRU should
> also be skipped?
> - if (page_count(page) == 0)
> + if (page_count(page) == 0 ||
> PageReserved(page) || PageLRU(page))
I think we need a really good justification to start poking holes into
the memmap scanner. I'm no expert on this code (and under which
circumstances we actually might find referenced objects in a memmap),
though.
But we should be careful with that.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: skip reserved page for kmem leak scanning
2022-08-30 8:03 ` David Hildenbrand
@ 2022-08-30 8:52 ` Zhaoyang Huang
0 siblings, 0 replies; 6+ messages in thread
From: Zhaoyang Huang @ 2022-08-30 8:52 UTC (permalink / raw)
To: David Hildenbrand
Cc: zhaoyang.huang, Andrew Morton, Catalin Marinas,
open list:MEMORY MANAGEMENT, LKML, Ke Wang
On Tue, Aug 30, 2022 at 4:03 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 30.08.22 04:41, Zhaoyang Huang wrote:
> > On Mon, Aug 29, 2022 at 8:19 PM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 26.08.22 05:23, Zhaoyang Huang wrote:
> >>> On Fri, Aug 26, 2022 at 11:13 AM zhaoyang.huang
> >>> <zhaoyang.huang@unisoc.com> wrote:
> >>>>
> >>>> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> >>>>
> >>>> It is no need to scan reserved page, skip it.
> >>>>
> >>>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> >>>> ---
> >>>> mm/kmemleak.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> >>>> index a182f5d..c546250 100644
> >>>> --- a/mm/kmemleak.c
> >>>> +++ b/mm/kmemleak.c
> >>>> @@ -1471,7 +1471,7 @@ static void kmemleak_scan(void)
> >>>> if (page_zone(page) != zone)
> >>>> continue;
> >>>> /* only scan if page is in use */
> >>>> - if (page_count(page) == 0)
> >>>> + if (page_count(page) == 0 || PageReserved(page))
> >>> Sorry for previous stupid code by my faint, correct it here
> >>
> >> Did you even test the initial patch?
> >>
> >> I wonder why we should consider this change
> >>
> >> (a) I doubt it's a performance issue. If it is, please provide numbers
> >> before/after.
> > For Android-like SOC systems where AP(cpu runs linux) are one of the
> > memory consumers which are composed of other processors such as modem,
> > isp,wcn etc. The reserved memory occupies a certain number of
> > memory(could be 30% of MemTotal) which makes scan reserved pages
> > pointless.
>
> But we only scan the memmap (struct page) here and not the actual
> memory. Do you have any performance numbers showing that there is even
> an observable change?
>
> >> (b) We'll stop scanning early allocations. As the memmap is usually
> >> allocated early during boot ... we'll stop scanning essentially the
> >> whole mmap and that whole loop would be dead code? What am i
> >> missing?
> > memmap refers to pages here? If we can surpass these as it exist
> > permanently during life period. Besides, I wonder if PageLRU should
> > also be skipped?
> > - if (page_count(page) == 0)
> > + if (page_count(page) == 0 ||
> > PageReserved(page) || PageLRU(page))
>
> I think we need a really good justification to start poking holes into
> the memmap scanner. I'm no expert on this code (and under which
> circumstances we actually might find referenced objects in a memmap),
> though.
>
> But we should be careful with that.
Agree. It may be helpless as kmemleak is for debugging purposes. Nack
this patch by myself. Sorry for interrupt.
>
> --
> Thanks,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-30 8:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-26 3:12 [PATCH] mm: skip reserved page for kmem leak scanning zhaoyang.huang
2022-08-26 3:23 ` Zhaoyang Huang
2022-08-29 12:18 ` David Hildenbrand
2022-08-30 2:41 ` Zhaoyang Huang
2022-08-30 8:03 ` David Hildenbrand
2022-08-30 8:52 ` Zhaoyang Huang
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 NNTP newsgroup(s).