[RFC,V11,17/21] Btrfs: subpagesize-blocksize: Use (eb->start, seq) as search key for tree modification log.
diff mbox

Message ID 1433172176-8742-18-git-send-email-chandan@linux.vnet.ibm.com
State New
Headers show

Commit Message

Chandan Rajendra June 1, 2015, 3:22 p.m. UTC
In subpagesize-blocksize a page can map multiple extent buffers and hence
using (page index, seq) as the search key is incorrect. For example, searching
through tree modification log tree can return an entry associated with the
first extent buffer mapped by the page (if such an entry exists), when we are
actually searching for entries associated with extent buffers that are mapped
at position 2 or more in the page.

Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
---
 fs/btrfs/ctree.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

Liu Bo July 20, 2015, 2:46 p.m. UTC | #1
On Mon, Jun 01, 2015 at 08:52:52PM +0530, Chandan Rajendra wrote:
> In subpagesize-blocksize a page can map multiple extent buffers and hence
> using (page index, seq) as the search key is incorrect. For example, searching
> through tree modification log tree can return an entry associated with the
> first extent buffer mapped by the page (if such an entry exists), when we are
> actually searching for entries associated with extent buffers that are mapped
> at position 2 or more in the page.

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

Thanks,

-liubo

> 
> Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> ---
>  fs/btrfs/ctree.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index ba6fbb0..47310d3 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -311,7 +311,7 @@ struct tree_mod_root {
>  
>  struct tree_mod_elem {
>  	struct rb_node node;
> -	u64 index;		/* shifted logical */
> +	u64 logical;
>  	u64 seq;
>  	enum mod_log_op op;
>  
> @@ -435,11 +435,11 @@ void btrfs_put_tree_mod_seq(struct btrfs_fs_info *fs_info,
>  
>  /*
>   * key order of the log:
> - *       index -> sequence
> + *       node/leaf start address -> sequence
>   *
> - * the index is the shifted logical of the *new* root node for root replace
> - * operations, or the shifted logical of the affected block for all other
> - * operations.
> + * The 'start address' is the logical address of the *new* root node
> + * for root replace operations, or the logical address of the affected
> + * block for all other operations.
>   *
>   * Note: must be called with write lock (tree_mod_log_write_lock).
>   */
> @@ -460,9 +460,9 @@ __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm)
>  	while (*new) {
>  		cur = container_of(*new, struct tree_mod_elem, node);
>  		parent = *new;
> -		if (cur->index < tm->index)
> +		if (cur->logical < tm->logical)
>  			new = &((*new)->rb_left);
> -		else if (cur->index > tm->index)
> +		else if (cur->logical > tm->logical)
>  			new = &((*new)->rb_right);
>  		else if (cur->seq < tm->seq)
>  			new = &((*new)->rb_left);
> @@ -523,7 +523,7 @@ alloc_tree_mod_elem(struct extent_buffer *eb, int slot,
>  	if (!tm)
>  		return NULL;
>  
> -	tm->index = eb->start >> PAGE_CACHE_SHIFT;
> +	tm->logical = eb->start;
>  	if (op != MOD_LOG_KEY_ADD) {
>  		btrfs_node_key(eb, &tm->key, slot);
>  		tm->blockptr = btrfs_node_blockptr(eb, slot);
> @@ -588,7 +588,7 @@ tree_mod_log_insert_move(struct btrfs_fs_info *fs_info,
>  		goto free_tms;
>  	}
>  
> -	tm->index = eb->start >> PAGE_CACHE_SHIFT;
> +	tm->logical = eb->start;
>  	tm->slot = src_slot;
>  	tm->move.dst_slot = dst_slot;
>  	tm->move.nr_items = nr_items;
> @@ -699,7 +699,7 @@ tree_mod_log_insert_root(struct btrfs_fs_info *fs_info,
>  		goto free_tms;
>  	}
>  
> -	tm->index = new_root->start >> PAGE_CACHE_SHIFT;
> +	tm->logical = new_root->start;
>  	tm->old_root.logical = old_root->start;
>  	tm->old_root.level = btrfs_header_level(old_root);
>  	tm->generation = btrfs_header_generation(old_root);
> @@ -739,16 +739,15 @@ __tree_mod_log_search(struct btrfs_fs_info *fs_info, u64 start, u64 min_seq,
>  	struct rb_node *node;
>  	struct tree_mod_elem *cur = NULL;
>  	struct tree_mod_elem *found = NULL;
> -	u64 index = start >> PAGE_CACHE_SHIFT;
>  
>  	tree_mod_log_read_lock(fs_info);
>  	tm_root = &fs_info->tree_mod_log;
>  	node = tm_root->rb_node;
>  	while (node) {
>  		cur = container_of(node, struct tree_mod_elem, node);
> -		if (cur->index < index) {
> +		if (cur->logical < start) {
>  			node = node->rb_left;
> -		} else if (cur->index > index) {
> +		} else if (cur->logical > start) {
>  			node = node->rb_right;
>  		} else if (cur->seq < min_seq) {
>  			node = node->rb_left;
> @@ -1228,9 +1227,10 @@ __tree_mod_log_oldest_root(struct btrfs_fs_info *fs_info,
>  		return NULL;
>  
>  	/*
> -	 * the very last operation that's logged for a root is the replacement
> -	 * operation (if it is replaced at all). this has the index of the *new*
> -	 * root, making it the very first operation that's logged for this root.
> +	 * the very last operation that's logged for a root is the
> +	 * replacement operation (if it is replaced at all). this has
> +	 * the logical address of the *new* root, making it the very
> +	 * first operation that's logged for this root.
>  	 */
>  	while (1) {
>  		tm = tree_mod_log_search_oldest(fs_info, root_logical,
> @@ -1334,7 +1334,7 @@ __tree_mod_log_rewind(struct btrfs_fs_info *fs_info, struct extent_buffer *eb,
>  		if (!next)
>  			break;
>  		tm = container_of(next, struct tree_mod_elem, node);
> -		if (tm->index != first_tm->index)
> +		if (tm->logical != first_tm->logical)
>  			break;
>  	}
>  	tree_mod_log_read_unlock(fs_info);
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index ba6fbb0..47310d3 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -311,7 +311,7 @@  struct tree_mod_root {
 
 struct tree_mod_elem {
 	struct rb_node node;
-	u64 index;		/* shifted logical */
+	u64 logical;
 	u64 seq;
 	enum mod_log_op op;
 
@@ -435,11 +435,11 @@  void btrfs_put_tree_mod_seq(struct btrfs_fs_info *fs_info,
 
 /*
  * key order of the log:
- *       index -> sequence
+ *       node/leaf start address -> sequence
  *
- * the index is the shifted logical of the *new* root node for root replace
- * operations, or the shifted logical of the affected block for all other
- * operations.
+ * The 'start address' is the logical address of the *new* root node
+ * for root replace operations, or the logical address of the affected
+ * block for all other operations.
  *
  * Note: must be called with write lock (tree_mod_log_write_lock).
  */
@@ -460,9 +460,9 @@  __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm)
 	while (*new) {
 		cur = container_of(*new, struct tree_mod_elem, node);
 		parent = *new;
-		if (cur->index < tm->index)
+		if (cur->logical < tm->logical)
 			new = &((*new)->rb_left);
-		else if (cur->index > tm->index)
+		else if (cur->logical > tm->logical)
 			new = &((*new)->rb_right);
 		else if (cur->seq < tm->seq)
 			new = &((*new)->rb_left);
@@ -523,7 +523,7 @@  alloc_tree_mod_elem(struct extent_buffer *eb, int slot,
 	if (!tm)
 		return NULL;
 
-	tm->index = eb->start >> PAGE_CACHE_SHIFT;
+	tm->logical = eb->start;
 	if (op != MOD_LOG_KEY_ADD) {
 		btrfs_node_key(eb, &tm->key, slot);
 		tm->blockptr = btrfs_node_blockptr(eb, slot);
@@ -588,7 +588,7 @@  tree_mod_log_insert_move(struct btrfs_fs_info *fs_info,
 		goto free_tms;
 	}
 
-	tm->index = eb->start >> PAGE_CACHE_SHIFT;
+	tm->logical = eb->start;
 	tm->slot = src_slot;
 	tm->move.dst_slot = dst_slot;
 	tm->move.nr_items = nr_items;
@@ -699,7 +699,7 @@  tree_mod_log_insert_root(struct btrfs_fs_info *fs_info,
 		goto free_tms;
 	}
 
-	tm->index = new_root->start >> PAGE_CACHE_SHIFT;
+	tm->logical = new_root->start;
 	tm->old_root.logical = old_root->start;
 	tm->old_root.level = btrfs_header_level(old_root);
 	tm->generation = btrfs_header_generation(old_root);
@@ -739,16 +739,15 @@  __tree_mod_log_search(struct btrfs_fs_info *fs_info, u64 start, u64 min_seq,
 	struct rb_node *node;
 	struct tree_mod_elem *cur = NULL;
 	struct tree_mod_elem *found = NULL;
-	u64 index = start >> PAGE_CACHE_SHIFT;
 
 	tree_mod_log_read_lock(fs_info);
 	tm_root = &fs_info->tree_mod_log;
 	node = tm_root->rb_node;
 	while (node) {
 		cur = container_of(node, struct tree_mod_elem, node);
-		if (cur->index < index) {
+		if (cur->logical < start) {
 			node = node->rb_left;
-		} else if (cur->index > index) {
+		} else if (cur->logical > start) {
 			node = node->rb_right;
 		} else if (cur->seq < min_seq) {
 			node = node->rb_left;
@@ -1228,9 +1227,10 @@  __tree_mod_log_oldest_root(struct btrfs_fs_info *fs_info,
 		return NULL;
 
 	/*
-	 * the very last operation that's logged for a root is the replacement
-	 * operation (if it is replaced at all). this has the index of the *new*
-	 * root, making it the very first operation that's logged for this root.
+	 * the very last operation that's logged for a root is the
+	 * replacement operation (if it is replaced at all). this has
+	 * the logical address of the *new* root, making it the very
+	 * first operation that's logged for this root.
 	 */
 	while (1) {
 		tm = tree_mod_log_search_oldest(fs_info, root_logical,
@@ -1334,7 +1334,7 @@  __tree_mod_log_rewind(struct btrfs_fs_info *fs_info, struct extent_buffer *eb,
 		if (!next)
 			break;
 		tm = container_of(next, struct tree_mod_elem, node);
-		if (tm->index != first_tm->index)
+		if (tm->logical != first_tm->logical)
 			break;
 	}
 	tree_mod_log_read_unlock(fs_info);