"Huang, Ying" <[email protected]> writes:
> Hi, Dave,
>
> Dave Hansen <[email protected]> writes:
>
>> On 08/09/2016 09:17 AM, Huang, Ying wrote:
>>> File pages uses a set of radix tags (DIRTY, TOWRITE, WRITEBACK) to
>>> accelerate finding the pages with the specific tag in the the radix tree
>>> during writing back an inode. But for anonymous pages in swap cache,
>>> there are no inode based writeback. So there is no need to find the
>>> pages with some writeback tags in the radix tree. It is no necessary to
>>> touch radix tree writeback tags for pages in swap cache.
>>
>> Seems simple enough. Do we do any of this unnecessary work for the
>> other radix tree tags? If so, maybe we should just fix this once and
>> for all. Could we, for instance, WARN_ONCE() in radix_tree_tag_set() if
>> it sees a swap mapping get handed in there?
>
> Good idea! I will do that and try to catch other places if any.
I tested all (18) anonymous pages related test cases in vm-scalability
with a debug patch to WARN_ONCE for all swap mapping tag operations.
There are no other tag operations for swap mapping caught. Below is the
patch I used for debugging.
Best Regards,
Huang, Ying
----------------------------------------->
dbg: find all tag operations for swap cache
diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 4c45105..9a239ec 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -106,16 +106,24 @@ struct radix_tree_node {
/* root tags are stored in gfp_mask, shifted by __GFP_BITS_SHIFT */
struct radix_tree_root {
+ bool swap;
gfp_t gfp_mask;
struct radix_tree_node __rcu *rnode;
};
#define RADIX_TREE_INIT(mask) { \
+ .swap = false, \
.gfp_mask = (mask), \
.rnode = NULL, \
}
-#define RADIX_TREE(name, mask) \
+#define RADIX_TREE_INIT_SWAP(mask) { \
+ .swap = true, \
+ .gfp_mask = (mask), \
+ .rnode = NULL, \
+}
+
+#define RADIX_TREE(name, mask) \
struct radix_tree_root name = RADIX_TREE_INIT(mask)
#define INIT_RADIX_TREE(root, mask) \
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 1b7bf73..51677bf 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -765,6 +765,8 @@ void *radix_tree_tag_set(struct radix_tree_root *root,
struct radix_tree_node *node, *parent;
unsigned long maxindex;
+ WARN_ON_ONCE(root->swap);
+
radix_tree_load_root(root, &node, &maxindex);
BUG_ON(index > maxindex);
@@ -828,6 +830,8 @@ void *radix_tree_tag_clear(struct radix_tree_root *root,
unsigned long maxindex;
int uninitialized_var(offset);
+ WARN_ON_ONCE(root->swap);
+
radix_tree_load_root(root, &node, &maxindex);
if (index > maxindex)
return NULL;
@@ -867,6 +871,8 @@ int radix_tree_tag_get(struct radix_tree_root *root,
struct radix_tree_node *node, *parent;
unsigned long maxindex;
+ WARN_ON_ONCE(root->swap);
+
if (!root_tag_get(root, tag))
return 0;
@@ -1050,6 +1056,8 @@ unsigned long radix_tree_range_tag_if_tagged(struct
radix_tree_root *root,
unsigned long tagged = 0;
unsigned long index = *first_indexp;
+ WARN_ON_ONCE(root->swap);
+
radix_tree_load_root(root, &child, &maxindex);
last_index = min(last_index, maxindex);
if (index > last_index)
@@ -1240,6 +1248,8 @@ radix_tree_gang_lookup_tag(struct radix_tree_root *root,
void **results,
void **slot;
unsigned int ret = 0;
+ WARN_ON_ONCE(root->swap);
+
if (unlikely(!max_items))
return 0;
@@ -1281,6 +1291,8 @@ radix_tree_gang_lookup_tag_slot(struct radix_tree_root
*root, void ***results,
void **slot;
unsigned int ret = 0;
+ WARN_ON_ONCE(root->swap);
+
if (unlikely(!max_items))
return 0;
@@ -1590,6 +1602,8 @@ struct radix_tree_node *radix_tree_replace_clear_tags(
struct radix_tree_node *node;
void **slot;
+ WARN_ON_ONCE(root->swap);
+
__radix_tree_lookup(root, index, &node, &slot);
if (node) {
diff --git a/mm/swap_state.c b/mm/swap_state.c
index c8310a3..0059653 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -34,7 +34,7 @@ static const struct address_space_operations swap_aops = {
struct address_space swapper_spaces[MAX_SWAPFILES] = {
[0 ... MAX_SWAPFILES - 1] = {
- .page_tree = RADIX_TREE_INIT(GFP_ATOMIC|__GFP_NOWARN),
+ .page_tree = RADIX_TREE_INIT_SWAP(GFP_ATOMIC|__GFP_NOWARN),
.i_mmap_writable = ATOMIC_INIT(0),
.a_ops = &swap_aops,
}
