diff mbox

[2/5] Btrfs: add branch prediction hints in the read page end IO function

Message ID 1373520339-13870-2-git-send-email-miaox@cn.fujitsu.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Miao Xie July 11, 2013, 5:25 a.m. UTC
Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/extent_io.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Chris Mason July 11, 2013, 2:31 p.m. UTC | #1
Do you have benchmark numbers for how much these help?  I hesitate to
bring in the likely/unlikely unless we can see it on the benchmarks.

(The patch does look fine though)

-chris
--
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
David Sterba July 12, 2013, 10:19 p.m. UTC | #2
On Thu, Jul 11, 2013 at 10:31:22AM -0400, Chris Mason wrote:
> Do you have benchmark numbers for how much these help?  I hesitate to
> bring in the likely/unlikely unless we can see it on the benchmarks.

Seconded, I doubt that this particular function gets any measurable
speed improvement with the prediction hints. They're eg suitable for
branches that almost never happen like error conditions or shortcuts to
the exit.

There's a config option CONFIG_PROFILE_ANNOTATED_BRANCHES to profile
all likely/unlikely, so if you want to see the hint effects yourself
feel free to experiment with that.

Theres' 'perf branch' patchset, able to gather information from the
Branch Trace Store (BTS), http://lwn.net/Articles/444885/ .

There probably are functions in btrfs code where even a small
improvement could bring some performance, first guess is
btrfs_search_slot + the callees, but I've never seen it up in the perf
top profile.

david
--
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
diff mbox

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 4bfbcc5..c9b28cf 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2503,7 +2503,7 @@  static void end_bio_extent_readpage(struct bio *bio, int err)
 
 		spin_lock(&tree->lock);
 		state = find_first_extent_bit_state(tree, start, EXTENT_LOCKED);
-		if (state && state->start == start) {
+		if (likely(state && state->start == start)) {
 			/*
 			 * take a reference on the state, unlock will drop
 			 * the ref
@@ -2513,7 +2513,8 @@  static void end_bio_extent_readpage(struct bio *bio, int err)
 		spin_unlock(&tree->lock);
 
 		mirror = io_bio->mirror_num;
-		if (uptodate && tree->ops && tree->ops->readpage_end_io_hook) {
+		if (likely(uptodate && tree->ops &&
+			   tree->ops->readpage_end_io_hook)) {
 			ret = tree->ops->readpage_end_io_hook(page, start, end,
 							      state, mirror);
 			if (ret)
@@ -2522,12 +2523,15 @@  static void end_bio_extent_readpage(struct bio *bio, int err)
 				clean_io_failure(start, page);
 		}
 
-		if (!uptodate && tree->ops && tree->ops->readpage_io_failed_hook) {
+		if (likely(uptodate))
+			goto readpage_ok;
+
+		if (tree->ops && tree->ops->readpage_io_failed_hook) {
 			ret = tree->ops->readpage_io_failed_hook(page, mirror);
 			if (!ret && !err &&
 			    test_bit(BIO_UPTODATE, &bio->bi_flags))
 				uptodate = 1;
-		} else if (!uptodate) {
+		} else {
 			/*
 			 * The generic bio_readpage_error handles errors the
 			 * following way: If possible, new read requests are
@@ -2548,7 +2552,7 @@  static void end_bio_extent_readpage(struct bio *bio, int err)
 				continue;
 			}
 		}
-
+readpage_ok:
 		if (uptodate && tree->track_uptodate) {
 			set_extent_uptodate(tree, start, end, &cached,
 					    GFP_ATOMIC);