linux-kernel - Re: [patch 9/9] mm: keep page cache radix tree nodes in check

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20140127023112.GF14369@bbox>
Date:	Mon, 27 Jan 2014 11:31:12 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <andi@...stfloor.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Bob Liu <bob.liu@...cle.com>,
	Christoph Hellwig <hch@...radead.org>,
	Dave Chinner <david@...morbit.com>,
	Greg Thelen <gthelen@...gle.com>,
	Hugh Dickins <hughd@...gle.com>, Jan Kara <jack@...e.cz>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Luigi Semenzato <semenzato@...gle.com>,
	Mel Gorman <mgorman@...e.de>,
	Metin Doslu <metin@...usdata.com>,
	Michel Lespinasse <walken@...gle.com>,
	Ozgun Erdogan <ozgun@...usdata.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Rik van Riel <riel@...hat.com>,
	Roman Gushchin <klamm@...dex-team.ru>,
	Ryan Mallon <rmallon@...il.com>, Tejun Heo <tj@...nel.org>,
	Vlastimil Babka <vbabka@...e.cz>, linux-mm@...ck.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 9/9] mm: keep page cache radix tree nodes in check

On Thu, Jan 23, 2014 at 02:22:12PM -0500, Johannes Weiner wrote:
> On Thu, Jan 23, 2014 at 02:20:14PM +0900, Minchan Kim wrote:
> > On Wed, Jan 22, 2014 at 01:42:17PM -0500, Johannes Weiner wrote:
> > > On Mon, Jan 13, 2014 at 04:39:47PM +0900, Minchan Kim wrote:
> > > > On Fri, Jan 10, 2014 at 01:10:43PM -0500, Johannes Weiner wrote:
> > > > > @@ -123,9 +129,39 @@ static void page_cache_tree_delete(struct address_space *mapping,
> > > > >  		 * same time and miss a shadow entry.
> > > > >  		 */
> > > > >  		smp_wmb();
> > > > > -	} else
> > > > > -		radix_tree_delete(&mapping->page_tree, page->index);
> > > > > +	}
> > > > >  	mapping->nrpages--;
> > > > > +
> > > > > +	if (!node) {
> > > > > +		/* Clear direct pointer tags in root node */
> > > > > +		mapping->page_tree.gfp_mask &= __GFP_BITS_MASK;
> > > > > +		radix_tree_replace_slot(slot, shadow);
> > > > > +		return;
> > > > > +	}
> > > > > +
> > > > > +	/* Clear tree tags for the removed page */
> > > > > +	index = page->index;
> > > > > +	offset = index & RADIX_TREE_MAP_MASK;
> > > > > +	for (tag = 0; tag < RADIX_TREE_MAX_TAGS; tag++) {
> > > > > +		if (test_bit(offset, node->tags[tag]))
> > > > > +			radix_tree_tag_clear(&mapping->page_tree, index, tag);
> > > > > +	}
> > > > > +
> > > > > +	/* Delete page, swap shadow entry */
> > > > > +	radix_tree_replace_slot(slot, shadow);
> > > > > +	node->count--;
> > > > > +	if (shadow)
> > > > > +		node->count += 1U << RADIX_TREE_COUNT_SHIFT;
> > > > 
> > > > Nitpick2:
> > > > It should be a function of workingset.c rather than exposing
> > > > RADIX_TREE_COUNT_SHIFT?
> > > > 
> > > > IMO, It would be better to provide some accessor functions here, too.
> > > 
> > > The shadow maintenance and node lifetime management are pretty
> > > interwoven to share branches and reduce instructions as these are
> > > common paths.  I don't see how this could result in cleaner code while
> > > keeping these advantages.
> > 
> > What I want is just put a inline accessor in somewhere like workingset.h
> > 
> > static inline void inc_shadow_entry(struct radix_tree_node *node)
> > {
> >     node->count += 1U << RADIX_TREE_COUNT_MASK;
> > }
> > 
> > So, anyone don't need to know that node->count upper bits present
> > count of shadow entry.
> 
> Okay, but then you have to cover lower bits as well, without explicit
> higher bit access it would be confusing to use the mask for lower
> bits.
> 
> Something like the following?

LGTM.

-- 
Kind regards,
Minchan Kim
--
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/