| 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
| ||
|
Date: Thu, 23 Jan 2014 14:22:12 -0500
From: Johannes Weiner <hannes@...xchg.org>
To: Minchan Kim <minchan@...nel.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: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?
---
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 102e37bc82d5..b33171a3673c 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -266,6 +266,36 @@ bool workingset_refault(void *shadow);
void workingset_activation(struct page *page);
extern struct list_lru workingset_shadow_nodes;
+static inline unsigned int workingset_node_pages(struct radix_tree_node *node)
+{
+ return node->count & RADIX_TREE_COUNT_MASK;
+}
+
+static inline void workingset_node_pages_inc(struct radix_tree_node *node)
+{
+ return node->count++;
+}
+
+static inline void workingset_node_pages_dec(struct radix_tree_node *node)
+{
+ return node->count--;
+}
+
+static inline unsigned int workingset_node_shadows(struct radix_tree_node *node)
+{
+ return node->count >> RADIX_TREE_COUNT_SHIFT;
+}
+
+static inline void workingset_node_shadows_inc(struct radix_tree_node *node)
+{
+ node->count += 1U << RADIX_TREE_COUNT_SHIFT;
+}
+
+static inline void workingset_node_shadows_dec(struct radix_tree_node *node)
+{
+ node->count -= 1U << RADIX_TREE_COUNT_SHIFT;
+}
+
/* linux/mm/page_alloc.c */
extern unsigned long totalram_pages;
extern unsigned long totalreserve_pages;
diff --git a/mm/filemap.c b/mm/filemap.c
index a63e89484d18..ac7f62db9ccd 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -149,9 +149,9 @@ static void page_cache_tree_delete(struct address_space *mapping,
/* Delete page, swap shadow entry */
radix_tree_replace_slot(slot, shadow);
- node->count--;
+ workingset_node_pages_dec(node);
if (shadow)
- node->count += 1U << RADIX_TREE_COUNT_SHIFT;
+ workingset_node_shadow_inc(node);
else
if (__radix_tree_delete_node(&mapping->page_tree, node))
return;
@@ -163,7 +163,7 @@ static void page_cache_tree_delete(struct address_space *mapping,
* list_empty() test is safe as node->private_list is
* protected by mapping->tree_lock.
*/
- if (!(node->count & RADIX_TREE_COUNT_MASK) &&
+ if (!workingset_node_pages(node) &&
list_empty(&node->private_list)) {
node->private_data = mapping;
list_lru_add(&workingset_shadow_nodes, &node->private_list);
@@ -531,12 +531,12 @@ static int page_cache_tree_insert(struct address_space *mapping,
*shadowp = p;
mapping->nrshadows--;
if (node)
- node->count -= 1U << RADIX_TREE_COUNT_SHIFT;
+ workingset_node_shadows_dec(node);
}
radix_tree_replace_slot(slot, page);
mapping->nrpages++;
if (node) {
- node->count++;
+ workingset_node_pages_inc(node);
/*
* Don't track node that contains actual pages.
*
diff --git a/mm/truncate.c b/mm/truncate.c
index c7a0d02a03eb..9cb54b7525dc 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -46,7 +46,7 @@ static void clear_exceptional_entry(struct address_space *mapping,
mapping->nrshadows--;
if (!node)
goto unlock;
- node->count -= 1U << RADIX_TREE_COUNT_SHIFT;
+ workingset_node_shadows_dec(node);
/*
* Don't track node without shadow entries.
*
@@ -54,7 +54,7 @@ static void clear_exceptional_entry(struct address_space *mapping,
* The list_empty() test is safe as node->private_list is
* protected by mapping->tree_lock.
*/
- if (!(node->count >> RADIX_TREE_COUNT_SHIFT) &&
+ if (!workingset_node_shadows(node) &&
!list_empty(&node->private_list))
list_lru_del(&workingset_shadow_nodes, &node->private_list);
__radix_tree_delete_node(&mapping->page_tree, node);
--
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/
Powered by blists - more mailing lists