From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753819AbaKRJ6q (ORCPT ); Tue, 18 Nov 2014 04:58:46 -0500 Received: from mta-out1.inet.fi ([62.71.2.195]:48970 "EHLO kirsi1.inet.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753779AbaKRJ6o (ORCPT ); Tue, 18 Nov 2014 04:58:44 -0500 Date: Tue, 18 Nov 2014 11:58:11 +0200 From: "Kirill A. Shutemov" To: Naoya Horiguchi Cc: "Kirill A. Shutemov" , Andrew Morton , Andrea Arcangeli , Dave Hansen , Hugh Dickins , Mel Gorman , Rik van Riel , Vlastimil Babka , Christoph Lameter , Steve Capper , "Aneesh Kumar K.V" , Johannes Weiner , Michal Hocko , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" Subject: Re: [PATCH 06/19] mm: store mapcount for compound page separate Message-ID: <20141118095811.GA21774@node.dhcp.inet.fi> References: <1415198994-15252-1-git-send-email-kirill.shutemov@linux.intel.com> <1415198994-15252-7-git-send-email-kirill.shutemov@linux.intel.com> <20141118084337.GA16714@hori1.linux.bs1.fc.nec.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141118084337.GA16714@hori1.linux.bs1.fc.nec.co.jp> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 18, 2014 at 08:43:00AM +0000, Naoya Horiguchi wrote: > > @@ -1837,6 +1839,9 @@ static void __split_huge_page_refcount(struct page *page, > > atomic_sub(tail_count, &page->_count); > > BUG_ON(atomic_read(&page->_count) <= 0); > > > > + page->_mapcount = *compound_mapcount_ptr(page); > > Is atomic_set() necessary? Do you mean atomic_set(&page->_mapcount, atomic_read(compound_mapcount_ptr(page))); ? I don't see why we would need this. Simple assignment should work just fine. Or we have archs which will break? > > @@ -6632,10 +6637,12 @@ static void dump_page_flags(unsigned long flags) > > void dump_page_badflags(struct page *page, const char *reason, > > unsigned long badflags) > > { > > - printk(KERN_ALERT > > - "page:%p count:%d mapcount:%d mapping:%p index:%#lx\n", > > + pr_alert("page:%p count:%d mapcount:%d mapping:%p index:%#lx", > > page, atomic_read(&page->_count), page_mapcount(page), > > page->mapping, page->index); > > + if (PageCompound(page)) > > > + printk(" compound_mapcount: %d", compound_mapcount(page)); > > + printk("\n"); > > These two printk() should be pr_alert(), too? No. It will split the line into several messages in dmesg. > > @@ -986,9 +986,30 @@ void page_add_anon_rmap(struct page *page, > > void do_page_add_anon_rmap(struct page *page, > > struct vm_area_struct *vma, unsigned long address, int flags) > > { > > - int first = atomic_inc_and_test(&page->_mapcount); > > + bool compound = flags & RMAP_COMPOUND; > > + bool first; > > + > > + VM_BUG_ON_PAGE(!PageLocked(compound_head(page)), page); > > + > > + if (PageTransCompound(page)) { > > + struct page *head_page = compound_head(page); > > + > > + if (compound) { > > + VM_BUG_ON_PAGE(!PageTransHuge(page), page); > > + first = atomic_inc_and_test(compound_mapcount_ptr(page)); > > Is compound_mapcount_ptr() well-defined for tail pages? The page is head page, otherwise VM_BUG_ON on the line above would trigger. > > @@ -1032,10 +1052,19 @@ void page_add_new_anon_rmap(struct page *page, > > > > VM_BUG_ON(address < vma->vm_start || address >= vma->vm_end); > > SetPageSwapBacked(page); > > - atomic_set(&page->_mapcount, 0); /* increment count (starts at -1) */ > > if (compound) { > > + atomic_t *compound_mapcount; > > + > > VM_BUG_ON_PAGE(!PageTransHuge(page), page); > > + compound_mapcount = (atomic_t *)&page[1].mapping; > > You can use compound_mapcount_ptr() here. Right, thanks. -- Kirill A. Shutemov