Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933759AbbENOMf (ORCPT ); Thu, 14 May 2015 10:12:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:60765 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933361AbbENOMe (ORCPT ); Thu, 14 May 2015 10:12:34 -0400 Message-ID: <5554AD4D.9040000@suse.cz> Date: Thu, 14 May 2015 16:12:29 +0200 From: Vlastimil Babka User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: "Kirill A. Shutemov" , Andrew Morton , Andrea Arcangeli , Hugh Dickins CC: Dave Hansen , Mel Gorman , Rik van Riel , Christoph Lameter , Naoya Horiguchi , Steve Capper , "Aneesh Kumar K.V" , Johannes Weiner , Michal Hocko , Jerome Marchand , Sasha Levin , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCHv5 01/28] mm, proc: adjust PSS calculation References: <1429823043-157133-1-git-send-email-kirill.shutemov@linux.intel.com> <1429823043-157133-2-git-send-email-kirill.shutemov@linux.intel.com> In-Reply-To: <1429823043-157133-2-git-send-email-kirill.shutemov@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3937 Lines: 117 On 04/23/2015 11:03 PM, Kirill A. Shutemov wrote: > With new refcounting all subpages of the compound page are not nessessary > have the same mapcount. We need to take into account mapcount of every > sub-page. > > Signed-off-by: Kirill A. Shutemov > Tested-by: Sasha Levin Acked-by: Vlastimil Babka (some nitpicks below) > --- > fs/proc/task_mmu.c | 43 ++++++++++++++++++++++--------------------- > 1 file changed, 22 insertions(+), 21 deletions(-) > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index 956b75d61809..95bc384ee3f7 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -449,9 +449,10 @@ struct mem_size_stats { > }; > > static void smaps_account(struct mem_size_stats *mss, struct page *page, > - unsigned long size, bool young, bool dirty) > + bool compound, bool young, bool dirty) > { > - int mapcount; > + int i, nr = compound ? hpage_nr_pages(page) : 1; Why not just HPAGE_PMD_NR instead of hpage_nr_pages(page)? We already came here through a pmd mapping. Even if the page stopped being a hugepage meanwhile (I'm not sure if any locking prevents that or not?), it would be more accurate to continue assuming it's a hugepage, otherwise we account only the base page (formerly head) and skip the 511 formerly tail pages? Also, is there some shortcut way to tell us that we are the only one mapping the whole compound page, and nobody has any base pages, so we don't need to loop on each tail page? I guess not under the new design, right... > + unsigned long size = nr * PAGE_SIZE; > > if (PageAnon(page)) > mss->anonymous += size; > @@ -460,23 +461,23 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page, > /* Accumulate the size in pages that have been accessed. */ > if (young || PageReferenced(page)) > mss->referenced += size; > - mapcount = page_mapcount(page); > - if (mapcount >= 2) { > - u64 pss_delta; > > - if (dirty || PageDirty(page)) > - mss->shared_dirty += size; > - else > - mss->shared_clean += size; > - pss_delta = (u64)size << PSS_SHIFT; > - do_div(pss_delta, mapcount); > - mss->pss += pss_delta; > - } else { > - if (dirty || PageDirty(page)) > - mss->private_dirty += size; > - else > - mss->private_clean += size; > - mss->pss += (u64)size << PSS_SHIFT; > + for (i = 0; i < nr; i++) { > + int mapcount = page_mapcount(page + i); > + > + if (mapcount >= 2) { > + if (dirty || PageDirty(page + i)) > + mss->shared_dirty += PAGE_SIZE; > + else > + mss->shared_clean += PAGE_SIZE; > + mss->pss += (PAGE_SIZE << PSS_SHIFT) / mapcount; > + } else { > + if (dirty || PageDirty(page + i)) > + mss->private_dirty += PAGE_SIZE; > + else > + mss->private_clean += PAGE_SIZE; > + mss->pss += PAGE_SIZE << PSS_SHIFT; > + } That's 3 instances of "page + i", why not just use page and do a page++ in the for loop? > } > } > > @@ -500,7 +501,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr, > > if (!page) > return; > - smaps_account(mss, page, PAGE_SIZE, pte_young(*pte), pte_dirty(*pte)); > + > + smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte)); > } > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > @@ -516,8 +518,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, > if (IS_ERR_OR_NULL(page)) > return; > mss->anonymous_thp += HPAGE_PMD_SIZE; > - smaps_account(mss, page, HPAGE_PMD_SIZE, > - pmd_young(*pmd), pmd_dirty(*pmd)); > + smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd)); > } > #else > static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/