btrfs: reada: Avoid many times of empty loop

PATCH New Zhao Lei 1 participant 11 messages Dec 31, 2015 Btrfs file system

lore

Share & tools

lore

Zhao Lei

author

2015-12-31 15:06 UTC #0 lore raw

cover btrfs: reada: Avoid many times of empty loop

This is some cleanup, bugfix and enhance for reada, tested by a script running scrub and relatice trace log.

Zhao Lei (10):
btrfs: reada: Avoid many times of empty loop
btrfs: reada: Move is_need_to_readahead contition earlier
btrfs: reada: add all reachable mirrors into reada device list
btrfs: reada: bypass adding extent when all zone failed
btrfs: reada: Remove level argument in severial functions
btrfs: reada: move reada_extent_put() to place after

    __readahead_hook()

btrfs: reada: Pass reada_extent into __readahead_hook() directly
btrfs: reada: Use fs_info instead of root in __readahead_hook's
argument
btrfs: reada: Jump into cleanup in direct way for __readahead_hook()
btrfs: reada: Fix a debug code typo

 fs/btrfs/ctree.h   |   4 +-
 fs/btrfs/disk-io.c |  22 +++----
 fs/btrfs/reada.c   | 173 ++++++++++++++++++++++++++---------------------------
 3 files changed, 98 insertions(+), 101 deletions(-)

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:07 UTC #1 lore raw

PATCH 1/10 btrfs: reada: Avoid many times of empty loop

We can see following loop(10000 times) in trace_log:

 [   75.416137] ZL_DEBUG: reada_start_machine_dev:730: pid=771 comm=kworker/u2:3 re->ref_cnt ffff88003741e0c0 1 -> 2
 [   75.417413] ZL_DEBUG: reada_extent_put:524: pid=771 comm=kworker/u2:3 re = ffff88003741e0c0, refcnt = 2 -> 1
 [   75.418611] ZL_DEBUG: __readahead_hook:129: pid=771 comm=kworker/u2:3 re->ref_cnt ffff88003741e0c0 1 -> 2
 [   75.419793] ZL_DEBUG: reada_extent_put:524: pid=771 comm=kworker/u2:3 re = ffff88003741e0c0, refcnt = 2 -> 1

 [   75.421016] ZL_DEBUG: reada_start_machine_dev:730: pid=771 comm=kworker/u2:3 re->ref_cnt ffff88003741e0c0 1 -> 2
 [   75.422324] ZL_DEBUG: reada_extent_put:524: pid=771 comm=kworker/u2:3 re = ffff88003741e0c0, refcnt = 2 -> 1
 [   75.423661] ZL_DEBUG: __readahead_hook:129: pid=771 comm=kworker/u2:3 re->ref_cnt ffff88003741e0c0 1 -> 2
 [   75.424882] ZL_DEBUG: reada_extent_put:524: pid=771 comm=kworker/u2:3 re = ffff88003741e0c0, refcnt = 2 -> 1

...(10000 times)

 [  124.101672] ZL_DEBUG: reada_start_machine_dev:730: pid=771 comm=kworker/u2:3 re->ref_cnt ffff88003741e0c0 1 -> 2
 [  124.102850] ZL_DEBUG: reada_extent_put:524: pid=771 comm=kworker/u2:3 re = ffff88003741e0c0, refcnt = 2 -> 1
 [  124.104008] ZL_DEBUG: __readahead_hook:129: pid=771 comm=kworker/u2:3 re->ref_cnt ffff88003741e0c0 1 -> 2
 [  124.105121] ZL_DEBUG: reada_extent_put:524: pid=771 comm=kworker/u2:3 re = ffff88003741e0c0, refcnt = 2 -> 1

Reason:
If more than one user trigger reada in same extent, the first task finished setting of reada data struct and call reada_start_machine() to start, and the second task only add a ref_count but have not add reada_extctl struct completely, the reada_extent can not finished all jobs, and will be selected in __reada_start_machine() for 10000 times(total times in __reada_start_machine()).

Fix:
For a reada_extent without job, we don't need to run it, just return 0 to let caller break.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/btrfs/reada.c +1 -1

index c65b42f..fb21bf0 100644
@@ -713,7 +713,7 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	logical = re->logical;
 
 	spin_lock(&re->lock);
-	if (re->scheduled_for == NULL) {
+	if (!re->scheduled_for && !list_empty(&re->extctl)) {
 		re->scheduled_for = dev;
 		need_kick = 1;
 	}

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:07 UTC #2 lore raw

PATCH 2/10 btrfs: reada: Move is_need_to_readahead contition earlier

In reply to Zhao Lei

Move is_need_to_readahead contition earlier to avoid useless loop to get relative data for readahead.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

fs/btrfs/reada.c +9 ~~-11~~

index fb21bf0..dcc5b69 100644
@@ -665,7 +665,6 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	u64 logical;
 	int ret;
 	int i;
-	int need_kick = 0;
 
 	spin_lock(&fs_info->reada_lock);
 	if (dev->reada_curr_zone == NULL) {

@@ -701,6 +700,15 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 
 	spin_unlock(&fs_info->reada_lock);
 
+	spin_lock(&re->lock);
+		if (re->scheduled_for || list_empty(&re->extctl)) {
+			spin_unlock(&re->lock);
+			reada_extent_put(fs_info, re);
+			return 0;
+		}
+		re->scheduled_for = dev;
+	spin_unlock(&re->lock);
+
 	/*
 	 * find mirror num
 	 */

@@ -712,18 +720,8 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	}
 	logical = re->logical;
 
-	spin_lock(&re->lock);
-	if (!re->scheduled_for && !list_empty(&re->extctl)) {
-		re->scheduled_for = dev;
-		need_kick = 1;
-	}
-	spin_unlock(&re->lock);
-
 	reada_extent_put(fs_info, re);
 
-	if (!need_kick)
-		return 0;
-
 	atomic_inc(&dev->reada_in_flight);
 	ret = reada_tree_block_flagged(fs_info->extent_root, logical,
 			mirror_num, &eb);

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:06 UTC #3 lore raw

PATCH 3/10 btrfs: reada: add all reachable mirrors into reada device list

In reply to Zhao Lei

If some device is not reachable, we should bypass and continus addingb next, instead of break on bad device.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

fs/btrfs/reada.c +9 ~~-11~~

index dcc5b69..7733a09 100644
@@ -328,7 +328,6 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 	u64 length;
 	int real_stripes;
 	int nzones = 0;
-	int i;
 	unsigned long index = logical >> PAGE_CACHE_SHIFT;
 	int dev_replace_is_ongoing;

@@ -380,9 +379,9 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 		dev = bbio->stripes[nzones].dev;
 		zone = reada_find_zone(fs_info, dev, logical, bbio);
 		if (!zone)
-			break;
+			continue;
 
-		re->zones[nzones] = zone;
+		re->zones[re->nzones++] = zone;
 		spin_lock(&zone->lock);
 		if (!zone->elems)
 			kref_get(&zone->refcnt);

@@ -392,8 +391,7 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 		kref_put(&zone->refcnt, reada_zone_release);
 		spin_unlock(&fs_info->reada_lock);
 	}
-	re->nzones = nzones;
-	if (nzones == 0) {
+	if (re->nzones == 0) {
 		/* not a single zone found, error and out */
 		goto error;
 	}

@@ -418,8 +416,9 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 	prev_dev = NULL;
 	dev_replace_is_ongoing = btrfs_dev_replace_is_ongoing(
 			&fs_info->dev_replace);
-	for (i = 0; i < nzones; ++i) {
-		dev = bbio->stripes[i].dev;
+	for (nzones = 0; nzones < re->nzones; ++nzones) {
+		dev = re->zones[nzones]->device;
+
 		if (dev == prev_dev) {
 			/*
 			 * in case of DUP, just add the first zone. As both

@@ -450,8 +449,8 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 		prev_dev = dev;
 		ret = radix_tree_insert(&dev->reada_extents, index, re);
 		if (ret) {
-			while (--i >= 0) {
-				dev = bbio->stripes[i].dev;
+			while (--nzones >= 0) {
+				dev = re->zones[nzones]->device;
 				BUG_ON(dev == NULL);
 				/* ignore whether the entry was inserted */
 				radix_tree_delete(&dev->reada_extents, index);

@@ -470,10 +469,9 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 	return re;
 
 error:
-	while (nzones) {
+	for (nzones = 0; nzones < re->nzones; ++nzones) {
 		struct reada_zone *zone;
 
-		--nzones;
 		zone = re->zones[nzones];
 		kref_get(&zone->refcnt);
 		spin_lock(&zone->lock);

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:07 UTC #4 lore raw

PATCH 4/10 btrfs: reada: bypass adding extent when all zone failed

In reply to Zhao Lei

When failed adding all dev_zones for a reada_extent, the extent will have no chance to be selected to run, and keep in memory for ever.

We should bypass this extent to avoid above case.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 5 +++++
 1 file changed, 5 insertions(+)

fs/btrfs/reada.c +5 -0

index 7733a09..ef9457e 100644
@@ -330,6 +330,7 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 	int nzones = 0;
 	unsigned long index = logical >> PAGE_CACHE_SHIFT;
 	int dev_replace_is_ongoing;
+	int have_zone = 0;
 
 	spin_lock(&fs_info->reada_lock);
 	re = radix_tree_lookup(&fs_info->reada_tree, index);

@@ -461,10 +462,14 @@ static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 			btrfs_dev_replace_unlock(&fs_info->dev_replace);
 			goto error;
 		}
+		have_zone = 1;
 	}
 	spin_unlock(&fs_info->reada_lock);
 	btrfs_dev_replace_unlock(&fs_info->dev_replace);
 
+	if (!have_zone)
+		goto error;
+
 	btrfs_put_bbio(bbio);
 	return re;

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:07 UTC #5 lore raw

PATCH 5/10 btrfs: reada: Remove level argument in severial functions

In reply to Zhao Lei

level is not used in severial functions, remove them from arguments, and remove relative code for get its value.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

fs/btrfs/reada.c +6 -9

index ef9457e..66409f3 100644
@@ -101,7 +101,7 @@ static void reada_start_machine(struct btrfs_fs_info *fs_info);
 static void __reada_start_machine(struct btrfs_fs_info *fs_info);
 
 static int reada_add_block(struct reada_control *rc, u64 logical,
-			   struct btrfs_key *top, int level, u64 generation);
+			   struct btrfs_key *top, u64 generation);
 
 /* recurses */
 /* in case of err, eb might be NULL */

@@ -197,8 +197,7 @@ static int __readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 			if (rec->generation == generation &&
 			    btrfs_comp_cpu_keys(&key, &rc->key_end) < 0 &&
 			    btrfs_comp_cpu_keys(&next_key, &rc->key_start) > 0)
-				reada_add_block(rc, bytenr, &next_key,
-						level - 1, n_gen);
+				reada_add_block(rc, bytenr, &next_key, n_gen);
 		}
 	}
 	/*

@@ -315,7 +314,7 @@ static struct reada_zone *reada_find_zone(struct btrfs_fs_info *fs_info,
 
 static struct reada_extent *reada_find_extent(struct btrfs_root *root,
 					      u64 logical,
-					      struct btrfs_key *top, int level)
+					      struct btrfs_key *top)
 {
 	int ret;
 	struct reada_extent *re = NULL;

@@ -562,13 +561,13 @@ static void reada_control_release(struct kref *kref)
 }
 
 static int reada_add_block(struct reada_control *rc, u64 logical,
-			   struct btrfs_key *top, int level, u64 generation)
+			   struct btrfs_key *top, u64 generation)
 {
 	struct btrfs_root *root = rc->root;
 	struct reada_extent *re;
 	struct reada_extctl *rec;
 
-	re = reada_find_extent(root, logical, top, level); /* takes one ref */
+	re = reada_find_extent(root, logical, top); /* takes one ref */
 	if (!re)
 		return -1;

@@ -921,7 +920,6 @@ struct reada_control *btrfs_reada_add(struct btrfs_root *root,
 	struct reada_control *rc;
 	u64 start;
 	u64 generation;
-	int level;
 	int ret;
 	struct extent_buffer *node;
 	static struct btrfs_key max_key = {

@@ -944,11 +942,10 @@ struct reada_control *btrfs_reada_add(struct btrfs_root *root,
 
 	node = btrfs_root_node(root);
 	start = node->start;
-	level = btrfs_header_level(node);
 	generation = btrfs_header_generation(node);
 	free_extent_buffer(node);
 
-	ret = reada_add_block(rc, start, &max_key, level, generation);
+	ret = reada_add_block(rc, start, &max_key, generation);
 	if (ret) {
 		kfree(rc);
 		return ERR_PTR(ret);

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:08 UTC #6 lore raw

PATCH 6/10 btrfs: reada: move reada_extent_put to place after __readahead_hook()

In reply to Zhao Lei

We can't release reada_extent earlier than __readahead_hook(), because __readahead_hook() still need to use it, it is necessary to hode a refcnt to avoid it be freed.

Actually it is not a problem after my patch named:
Avoid many times of empty loop
It make reada_extent in above line include at least one reada_extctl, which keeps additional one refcnt for reada_extent.

But we still need this patch to make the code in pretty logic.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/btrfs/reada.c +2 -2

index 66409f3..7015906 100644
@@ -722,8 +722,6 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	}
 	logical = re->logical;
 
-	reada_extent_put(fs_info, re);
-
 	atomic_inc(&dev->reada_in_flight);
 	ret = reada_tree_block_flagged(fs_info->extent_root, logical,
 			mirror_num, &eb);

@@ -735,6 +733,8 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	if (eb)
 		free_extent_buffer(eb);
 
+	reada_extent_put(fs_info, re);
+
 	return 1;
 
 }

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:07 UTC #7 lore raw

PATCH 7/10 btrfs: reada: Pass reada_extent into __readahead_hook directly

In reply to Zhao Lei

reada_start_machine_dev() already have reada_extent pointer, pass it into __readahead_hook() directly instead of search radix_tree will make code run faster.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

fs/btrfs/reada.c +24 ~~-21~~

index 7015906..7668066 100644
@@ -105,33 +105,21 @@ static int reada_add_block(struct reada_control *rc, u64 logical,
 
 /* recurses */
 /* in case of err, eb might be NULL */
-static int __readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
-			    u64 start, int err)
+static void __readahead_hook(struct btrfs_root *root, struct reada_extent *re,
+			     struct extent_buffer *eb, u64 start, int err)
 {
 	int level = 0;
 	int nritems;
 	int i;
 	u64 bytenr;
 	u64 generation;
-	struct reada_extent *re;
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct list_head list;
-	unsigned long index = start >> PAGE_CACHE_SHIFT;
 	struct btrfs_device *for_dev;
 
 	if (eb)
 		level = btrfs_header_level(eb);
 
-	/* find extent */
-	spin_lock(&fs_info->reada_lock);
-	re = radix_tree_lookup(&fs_info->reada_tree, index);
-	if (re)
-		re->refcnt++;
-	spin_unlock(&fs_info->reada_lock);
-
-	if (!re)
-		return -1;
-
 	spin_lock(&re->lock);
 	/*
 	 * just take the full list from the extent. afterwards we

@@ -221,11 +209,11 @@ static int __readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 
 		reada_extent_put(fs_info, re);	/* one ref for each entry */
 	}
-	reada_extent_put(fs_info, re);	/* our ref */
+
 	if (for_dev)
 		atomic_dec(&for_dev->reada_in_flight);
 
-	return 0;
+	return;
 }
 
 /*

@@ -235,12 +223,27 @@ static int __readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
 			 u64 start, int err)
 {
-	int ret;
+	int ret = 0;
+	struct reada_extent *re;
+	struct btrfs_fs_info *fs_info = root->fs_info;
 
-	ret = __readahead_hook(root, eb, start, err);
+	/* find extent */
+	spin_lock(&fs_info->reada_lock);
+	re = radix_tree_lookup(&fs_info->reada_tree,
+			       start >> PAGE_CACHE_SHIFT);
+	if (re)
+		re->refcnt++;
+	spin_unlock(&fs_info->reada_lock);
+	if (!re) {
+		ret = -1;
+		goto start_machine;
+	}
 
-	reada_start_machine(root->fs_info);
+	__readahead_hook(fs_info, re, eb, start, err);
+	reada_extent_put(fs_info, re);	/* our ref */
 
+start_machine:
+	reada_start_machine(fs_info);
 	return ret;
 }

@@ -726,9 +729,9 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	ret = reada_tree_block_flagged(fs_info->extent_root, logical,
 			mirror_num, &eb);
 	if (ret)
-		__readahead_hook(fs_info->extent_root, NULL, logical, ret);
+		__readahead_hook(fs_info->extent_root, re, NULL, logical, ret);
 	else if (eb)
-		__readahead_hook(fs_info->extent_root, eb, eb->start, ret);
+		__readahead_hook(fs_info->extent_root, re, eb, eb->start, ret);
 
 	if (eb)
 		free_extent_buffer(eb);

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:08 UTC #8 lore raw

PATCH 8/10 btrfs: reada: Use fs_info instead of root in __readahead_hook's argument

In reply to Zhao Lei

What __readahead_hook() need exactly is fs_info, no need to convert fs_info to root in caller and convert back in __readahead_hook()

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/ctree.h   |  4 ++--
 fs/btrfs/disk-io.c | 22 +++++++++++-----------
 fs/btrfs/reada.c   | 23 +++++++++++------------
 3 files changed, 24 insertions(+), 25 deletions(-)

fs/btrfs/ctree.h +2 -2

index 54e7b0d..0912f89 100644
@@ -4358,8 +4358,8 @@ struct reada_control *btrfs_reada_add(struct btrfs_root *root,
 			      struct btrfs_key *start, struct btrfs_key *end);
 int btrfs_reada_wait(void *handle);
 void btrfs_reada_detach(void *handle);
-int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
-			 u64 start, int err);
+int btree_readahead_hook(struct btrfs_fs_info *fs_info,
+			 struct extent_buffer *eb, u64 start, int err);
 
 static inline int is_fstree(u64 rootid)
 {

fs/btrfs/disk-io.c +11 ~~-11~~

index 974be09..9d120e4 100644
@@ -604,6 +604,7 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 	int found_level;
 	struct extent_buffer *eb;
 	struct btrfs_root *root = BTRFS_I(page->mapping->host)->root;
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	int ret = 0;
 	int reads_done;

@@ -629,21 +630,21 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 
 	found_start = btrfs_header_bytenr(eb);
 	if (found_start != eb->start) {
-		btrfs_err_rl(eb->fs_info, "bad tree block start %llu %llu",
-			       found_start, eb->start);
+		btrfs_err_rl(fs_info, "bad tree block start %llu %llu",
+			     found_start, eb->start);
 		ret = -EIO;
 		goto err;
 	}
-	if (check_tree_block_fsid(root->fs_info, eb)) {
-		btrfs_err_rl(eb->fs_info, "bad fsid on block %llu",
-			       eb->start);
+	if (check_tree_block_fsid(fs_info, eb)) {
+		btrfs_err_rl(fs_info, "bad fsid on block %llu",
+			     eb->start);
 		ret = -EIO;
 		goto err;
 	}
 	found_level = btrfs_header_level(eb);
 	if (found_level >= BTRFS_MAX_LEVEL) {
-		btrfs_err(root->fs_info, "bad tree block level %d",
-			   (int)btrfs_header_level(eb));
+		btrfs_err(fs_info, "bad tree block level %d",
+			  (int)btrfs_header_level(eb));
 		ret = -EIO;
 		goto err;
 	}

@@ -651,7 +652,7 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 	btrfs_set_buffer_lockdep_class(btrfs_header_owner(eb),
 				       eb, found_level);
 
-	ret = csum_tree_block(root->fs_info, eb, 1);
+	ret = csum_tree_block(fs_info, eb, 1);
 	if (ret) {
 		ret = -EIO;
 		goto err;

@@ -672,7 +673,7 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 err:
 	if (reads_done &&
 	    test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
-		btree_readahead_hook(root, eb, eb->start, ret);
+		btree_readahead_hook(fs_info, eb, eb->start, ret);
 
 	if (ret) {
 		/*

@@ -691,14 +692,13 @@ out:
 static int btree_io_failed_hook(struct page *page, int failed_mirror)
 {
 	struct extent_buffer *eb;
-	struct btrfs_root *root = BTRFS_I(page->mapping->host)->root;
 
 	eb = (struct extent_buffer *)page->private;
 	set_bit(EXTENT_BUFFER_READ_ERR, &eb->bflags);
 	eb->read_mirror = failed_mirror;
 	atomic_dec(&eb->io_pages);
 	if (test_and_clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags))
-		btree_readahead_hook(root, eb, eb->start, -EIO);
+		btree_readahead_hook(eb->fs_info, eb, eb->start, -EIO);
 	return -EIO;	/* we fixed nothing */
 }

fs/btrfs/reada.c +11 ~~-12~~

index 7668066..869bb1c 100644
@@ -105,15 +105,15 @@ static int reada_add_block(struct reada_control *rc, u64 logical,
 
 /* recurses */
 /* in case of err, eb might be NULL */
-static void __readahead_hook(struct btrfs_root *root, struct reada_extent *re,
-			     struct extent_buffer *eb, u64 start, int err)
+static void __readahead_hook(struct btrfs_fs_info *fs_info,
+			     struct reada_extent *re, struct extent_buffer *eb,
+			     u64 start, int err)
 {
 	int level = 0;
 	int nritems;
 	int i;
 	u64 bytenr;
 	u64 generation;
-	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct list_head list;
 	struct btrfs_device *for_dev;

@@ -176,10 +176,10 @@ static void __readahead_hook(struct btrfs_root *root, struct reada_extent *re,
 			 */
 #ifdef DEBUG
 			if (rec->generation != generation) {
-				btrfs_debug(root->fs_info,
-					   "generation mismatch for (%llu,%d,%llu) %llu != %llu",
-				       key.objectid, key.type, key.offset,
-				       rec->generation, generation);
+				btrfs_debug(fs_info,
+					    "generation mismatch for (%llu,%d,%llu) %llu != %llu",
+					    key.objectid, key.type, key.offset,
+					    rec->generation, generation);
 			}
 #endif
 			if (rec->generation == generation &&

@@ -220,12 +220,11 @@ static void __readahead_hook(struct btrfs_root *root, struct reada_extent *re,
  * start is passed separately in case eb in NULL, which may be the case with
  * failed I/O
  */
-int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
-			 u64 start, int err)
+int btree_readahead_hook(struct btrfs_fs_info *fs_info,
+			 struct extent_buffer *eb, u64 start, int err)
 {
 	int ret = 0;
 	struct reada_extent *re;
-	struct btrfs_fs_info *fs_info = root->fs_info;
 
 	/* find extent */
 	spin_lock(&fs_info->reada_lock);

@@ -729,9 +728,9 @@ static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
 	ret = reada_tree_block_flagged(fs_info->extent_root, logical,
 			mirror_num, &eb);
 	if (ret)
-		__readahead_hook(fs_info->extent_root, re, NULL, logical, ret);
+		__readahead_hook(fs_info, re, NULL, logical, ret);
 	else if (eb)
-		__readahead_hook(fs_info->extent_root, re, eb, eb->start, ret);
+		__readahead_hook(fs_info, re, eb, eb->start, ret);
 
 	if (eb)
 		free_extent_buffer(eb);

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:07 UTC #9 lore raw

PATCH 9/10 btrfs: reada: Jump into cleanup in direct way for __readahead_hook()

In reply to Zhao Lei

Current code set nritems to 0 to make for_loop useless to bypass it, and set generation's value which is not necessary.
Jump into cleanup directly is better choise.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

fs/btrfs/reada.c +21 ~~-19~~

index 869bb1c..902f899 100644
@@ -130,26 +130,26 @@ static void __readahead_hook(struct btrfs_fs_info *fs_info,
 	re->scheduled_for = NULL;
 	spin_unlock(&re->lock);
 
-	if (err == 0) {
-		nritems = level ? btrfs_header_nritems(eb) : 0;
-		generation = btrfs_header_generation(eb);
-		/*
-		 * FIXME: currently we just set nritems to 0 if this is a leaf,
-		 * effectively ignoring the content. In a next step we could
-		 * trigger more readahead depending from the content, e.g.
-		 * fetch the checksums for the extents in the leaf.
-		 */
-	} else {
-		/*
-		 * this is the error case, the extent buffer has not been
-		 * read correctly. We won't access anything from it and
-		 * just cleanup our data structures. Effectively this will
-		 * cut the branch below this node from read ahead.
-		 */
-		nritems = 0;
-		generation = 0;
-	}
+	/*
+	 * this is the error case, the extent buffer has not been
+	 * read correctly. We won't access anything from it and
+	 * just cleanup our data structures. Effectively this will
+	 * cut the branch below this node from read ahead.
+	 */
+	if (err)
+		goto cleanup;
 
+	/*
+	 * FIXME: currently we just set nritems to 0 if this is a leaf,
+	 * effectively ignoring the content. In a next step we could
+	 * trigger more readahead depending from the content, e.g.
+	 * fetch the checksums for the extents in the leaf.
+	 */
+	if (!level)
+		goto cleanup;
+
+	nritems = btrfs_header_nritems(eb);
+	generation = btrfs_header_generation(eb);
 	for (i = 0; i < nritems; i++) {
 		struct reada_extctl *rec;
 		u64 n_gen;

@@ -188,6 +188,8 @@ static void __readahead_hook(struct btrfs_fs_info *fs_info,
 				reada_add_block(rc, bytenr, &next_key, n_gen);
 		}
 	}
+
+cleanup:
 	/*
 	 * free extctl records
 	 */

Signature · 1 line

1.8.5.1

Zhao Lei

author

2015-12-31 15:08 UTC #10 lore raw

PATCH 10/10 btrfs: reada: Fix a debug code typo

In reply to Zhao Lei

Remove one copy of loop to fix the typo of iterate zones.

Signed-off-by Zhao Lei <[email protected]>

---
 fs/btrfs/reada.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

fs/btrfs/reada.c +3 -8

index 902f899..53ee7b1 100644
@@ -898,14 +898,9 @@ static void dump_devs(struct btrfs_fs_info *fs_info, int all)
 			printk(KERN_CONT " zone %llu-%llu devs",
 				re->zones[i]->start,
 				re->zones[i]->end);
-			for (i = 0; i < re->nzones; ++i) {
-				printk(KERN_CONT " zone %llu-%llu devs",
-					re->zones[i]->start,
-					re->zones[i]->end);
-				for (j = 0; j < re->zones[i]->ndevs; ++j) {
-					printk(KERN_CONT " %lld",
-						re->zones[i]->devs[j]->devid);
-				}
+			for (j = 0; j < re->zones[i]->ndevs; ++j) {
+				printk(KERN_CONT " %lld",
+				       re->zones[i]->devs[j]->devid);
 			}
 		}
 		printk(KERN_CONT "\n");

Signature · 1 line

1.8.5.1