linux-kernel - Re: [PATCH 06/19] mm: store mapcount for compound page separate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141118234145.GA4116@hori1.linux.bs1.fc.nec.co.jp>
Date:	Tue, 18 Nov 2014 23:41:08 +0000
From:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:	"Kirill A. Shutemov" <kirill@...temov.name>
CC:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	"Dave Hansen" <dave.hansen@...el.com>,
	Hugh Dickins <hughd@...gle.com>, Mel Gorman <mgorman@...e.de>,
	Rik van Riel <riel@...hat.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Christoph Lameter <cl@...two.org>,
	Steve Capper <steve.capper@...aro.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Michal Hocko <mhocko@...e.cz>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH 06/19] mm: store mapcount for compound page separate

On Tue, Nov 18, 2014 at 11:58:11AM +0200, Kirill A. Shutemov wrote:
> 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?

Sorry, I was wrong, please ignore this comment.

> > > @@ -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.

This splitting is fine. I meant that these printk()s are for one series
of message, so setting the same log level looks reasonable to me.

> > > @@ -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.

Ah, OK.

Thanks,
Naoya Horiguchi

> > > @@ -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
> --
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/