Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751740AbdIAJMl (ORCPT ); Fri, 1 Sep 2017 05:12:41 -0400 Received: from forwardcorp1o.cmail.yandex.net ([37.9.109.47]:41982 "EHLO forwardcorp1o.cmail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751457AbdIAJMk (ORCPT ); Fri, 1 Sep 2017 05:12:40 -0400 Authentication-Results: smtpcorp1o.mail.yandex.net; dkim=pass header.i=@yandex-team.ru Subject: Re: [PATCH] mm/vmstats: add counters for the page frag cache To: Kyeongdon Kim , akpm@linux-foundation.org, sfr@canb.auug.org.au Cc: ying.huang@intel.com, vbabka@suse.cz, hannes@cmpxchg.org, xieyisheng1@huawei.com, luto@kernel.org, shli@fb.com, mhocko@suse.com, mgorman@techsingularity.net, hillf.zj@alibaba-inc.com, kemi.wang@intel.com, rientjes@google.com, bigeasy@linutronix.de, iamjoonsoo.kim@lge.com, bongkyu.kim@lge.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <1504222631-2635-1-git-send-email-kyeongdon.kim@lge.com> From: Konstantin Khlebnikov Message-ID: <50592560-af4d-302c-c0bc-1e854e35139d@yandex-team.ru> Date: Fri, 1 Sep 2017 12:12:36 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1504222631-2635-1-git-send-email-kyeongdon.kim@lge.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: ru-RU Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3298 Lines: 99 IMHO that's too much counters. Per-node NR_FRAGMENT_PAGES should be enough for guessing what's going on. Perf probes provides enough features for furhter debugging. On 01.09.2017 02:37, Kyeongdon Kim wrote: > There was a memory leak problem when we did stressful test > on Android device. > The root cause of this was from page_frag_cache alloc > and it was very hard to find out. > > We add to count the page frag allocation and free with function call. > The gap between pgfrag_alloc and pgfrag_free is good to to calculate > for the amount of page. > The gap between pgfrag_alloc_calls and pgfrag_free_calls is for > sub-indicator. > They can see trends of memory usage during the test. > Without it, it's difficult to check page frag usage so I believe we > should add it. > > Signed-off-by: Kyeongdon Kim > --- > include/linux/vm_event_item.h | 4 ++++ > mm/page_alloc.c | 9 +++++++-- > mm/vmstat.c | 4 ++++ > 3 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h > index d77bc35..75425d4 100644 > --- a/include/linux/vm_event_item.h > +++ b/include/linux/vm_event_item.h > @@ -110,6 +110,10 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT, > SWAP_RA, > SWAP_RA_HIT, > #endif > + PGFRAG_ALLOC, > + PGFRAG_FREE, > + PGFRAG_ALLOC_CALLS, > + PGFRAG_FREE_CALLS, > NR_VM_EVENT_ITEMS > }; > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index db2d25f..b3ddd76 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4322,6 +4322,7 @@ void __page_frag_cache_drain(struct page *page, unsigned int count) > free_hot_cold_page(page, false); > else > __free_pages_ok(page, order); > + __count_vm_events(PGFRAG_FREE, 1 << order); > } > } > EXPORT_SYMBOL(__page_frag_cache_drain); > @@ -4338,7 +4339,7 @@ void *page_frag_alloc(struct page_frag_cache *nc, > page = __page_frag_cache_refill(nc, gfp_mask); > if (!page) > return NULL; > - > + __count_vm_events(PGFRAG_ALLOC, 1 << compound_order(page)); > #if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) > /* if size can vary use size else just use PAGE_SIZE */ > size = nc->size; > @@ -4375,6 +4376,7 @@ void *page_frag_alloc(struct page_frag_cache *nc, > > nc->pagecnt_bias--; > nc->offset = offset; > + __count_vm_event(PGFRAG_ALLOC_CALLS); > > return nc->va + offset; > } > @@ -4387,8 +4389,11 @@ void page_frag_free(void *addr) > { > struct page *page = virt_to_head_page(addr); > > - if (unlikely(put_page_testzero(page))) > + if (unlikely(put_page_testzero(page))) { > + __count_vm_events(PGFRAG_FREE, 1 << compound_order(page)); > __free_pages_ok(page, compound_order(page)); > + } > + __count_vm_event(PGFRAG_FREE_CALLS); > } > EXPORT_SYMBOL(page_frag_free); > > diff --git a/mm/vmstat.c b/mm/vmstat.c > index 4bb13e7..c00fe05 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -1217,6 +1217,10 @@ const char * const vmstat_text[] = { > "swap_ra", > "swap_ra_hit", > #endif > + "pgfrag_alloc", > + "pgfrag_free", > + "pgfrag_alloc_calls", > + "pgfrag_free_calls", > #endif /* CONFIG_VM_EVENTS_COUNTERS */ > }; > #endif /* CONFIG_PROC_FS || CONFIG_SYSFS || CONFIG_NUMA */ >