On Tue, Aug 30, 2022 at 4:03 PM David Hildenbrand <david@xxxxxxxxxx> wrote: > > On 30.08.22 04:41, Zhaoyang Huang wrote: > > On Mon, Aug 29, 2022 at 8:19 PM David Hildenbrand <david@xxxxxxxxxx> wrote: > >> > >> On 26.08.22 05:23, Zhaoyang Huang wrote: > >>> On Fri, Aug 26, 2022 at 11:13 AM zhaoyang.huang > >>> <zhaoyang.huang@xxxxxxxxxx> wrote: > >>>> > >>>> From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx> > >>>> > >>>> It is no need to scan reserved page, skip it. > >>>> > >>>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx> > >>>> --- > >>>> 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 >
