Btrfs devel - Jun 2011 - [PATCH v4 0/6] btrfs: generic readeahead interface

This series introduces a generic readahead interface for btrfs trees.
The intention is to use it to speed up scrub in a first run, but balance
is another hot candidate. In general, every tree walk could be accompanied
by a readahead. Deletion of large files comes to mind, where the fetching
of the csums takes most of the time.
Also the initial build-ups of free-space-caches and inode-allocator-caches
could be sped up.

To make testing easier, a simple ioctl interface is added to trigger a read-
ahead from user mode. It also implements a tree walk in the traditional way.

A simple demonstration from my 7-disk test btrfs:
 - enumerating the extent tree (traditional): 351s
 - enumerating the extent tree (readahead): 41s
 - enumerating extents+csum tree (readahead): 49s

The implementation is also tested with this tool in various combinations of
parallel reads of the same and of different trees.

The main changes from v1 are:
 - Switch from extent_state flags to extent_buffer flags.
 - Fix a race when triggering the read.
 - Fix a bug where only parts of the requested range where actually prefetched.
   The hit only when requesting parts of a tree, so the above numbers
doesn''t
   change.
Change from v2:
 - use rcu instead of transaction to protect root->node
Change from v3:
 - rebased to current for-linus
 - in v3 patch 1 was missing

Arne Jansen (7):
  btrfs: add an extra wait mode to read_extent_buffer_pages
  btrfs: add READAHEAD extent buffer flag
  btrfs: state information for readahead
  btrfs: initial readahead code and prototypes
  btrfs: hooks for readahead
  btrfs: test ioctl for readahead
  btrfs: use readahead API for scrub

 fs/btrfs/Makefile    |    3 +-
 fs/btrfs/ctree.h     |   13 +
 fs/btrfs/disk-io.c   |   84 ++++-
 fs/btrfs/disk-io.h   |    2 +
 fs/btrfs/extent_io.c |    8 +-
 fs/btrfs/extent_io.h |    4 +-
 fs/btrfs/ioctl.c     |   93 +++++-
 fs/btrfs/ioctl.h     |   16 +
 fs/btrfs/reada.c     |  994 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/scrub.c     |  116 +++----
 fs/btrfs/volumes.c   |    8 +
 fs/btrfs/volumes.h   |    8 +
 12 files changed, 1274 insertions(+), 75 deletions(-)
 create mode 100644 fs/btrfs/reada.c

-- 
1.7.3.4

--
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

read_extent_buffer_pages currently has two modes, either trigger a read
without waiting for anything, or wait for the I/O to finish. The former
also bails when it''s unable to lock the page. This patch now adds an
additional parameter to allow it to block on page lock, but don''t wait
for completion.

Signed-off-by: Arne Jansen <sensille@gmx.net>
---
 fs/btrfs/disk-io.c   |    4 ++--
 fs/btrfs/extent_io.c |    6 +++---
 fs/btrfs/extent_io.h |    3 ++-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1ac8db5d..cff882c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -325,7 +325,7 @@ static int btree_read_extent_buffer_pages(struct btrfs_root
*root,
 	clear_bit(EXTENT_BUFFER_CORRUPT, &eb->bflags);
 	io_tree = &BTRFS_I(root->fs_info->btree_inode)->io_tree;
 	while (1) {
-		ret = read_extent_buffer_pages(io_tree, eb, start, 1,
+		ret = read_extent_buffer_pages(io_tree, eb, start, 1, 1,
 					       btree_get_extent, mirror_num);
 		if (!ret &&
 		    !verify_parent_transid(io_tree, eb, parent_transid))
@@ -940,7 +940,7 @@ int readahead_tree_block(struct btrfs_root *root, u64
bytenr, u32 blocksize,
 	if (!buf)
 		return 0;
 	read_extent_buffer_pages(&BTRFS_I(btree_inode)->io_tree,
-				 buf, 0, 0, btree_get_extent, 0);
+				 buf, 0, 0, 0, btree_get_extent, 0);
 	free_extent_buffer(buf);
 	return ret;
 }
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index b181a94..b7f0b9b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3358,7 +3358,7 @@ int extent_buffer_uptodate(struct extent_io_tree *tree,
 
 int read_extent_buffer_pages(struct extent_io_tree *tree,
 			     struct extent_buffer *eb,
-			     u64 start, int wait,
+			     u64 start, int wait_lock, int wait_complete,
 			     get_extent_t *get_extent, int mirror_num)
 {
 	unsigned long i;
@@ -3392,7 +3392,7 @@ int read_extent_buffer_pages(struct extent_io_tree *tree,
 	num_pages = num_extent_pages(eb->start, eb->len);
 	for (i = start_i; i < num_pages; i++) {
 		page = extent_buffer_page(eb, i);
-		if (!wait) {
+		if (!wait_lock) {
 			if (!trylock_page(page))
 				goto unlock_exit;
 		} else {
@@ -3436,7 +3436,7 @@ int read_extent_buffer_pages(struct extent_io_tree *tree,
 	if (bio)
 		submit_one_bio(READ, bio, mirror_num, bio_flags);
 
-	if (ret || !wait)
+	if (ret || !wait_complete)
 		return ret;
 
 	for (i = start_i; i < num_pages; i++) {
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index a11a92e..5ef3735 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -241,7 +241,8 @@ struct extent_buffer *find_extent_buffer(struct
extent_io_tree *tree,
 					 u64 start, unsigned long len);
 void free_extent_buffer(struct extent_buffer *eb);
 int read_extent_buffer_pages(struct extent_io_tree *tree,
-			     struct extent_buffer *eb, u64 start, int wait,
+			     struct extent_buffer *eb, u64 start,
+			     int wait_lock, int wait_complete,
 			     get_extent_t *get_extent, int mirror_num);
 
 static inline void extent_buffer_get(struct extent_buffer *eb)
-- 
1.7.3.4

--
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

Add a READAHEAD extent buffer flag.
Add a function to trigger a read with this flag set.

Changes v2:
 - use extent buffer flags instead of extent state flags

Signed-off-by: Arne Jansen <sensille@gmx.net>
---
 fs/btrfs/disk-io.c   |   32 ++++++++++++++++++++++++++++++++
 fs/btrfs/disk-io.h   |    2 ++
 fs/btrfs/extent_io.h |    1 +
 3 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cff882c..20edd2d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -945,6 +945,38 @@ int readahead_tree_block(struct btrfs_root *root, u64
bytenr, u32 blocksize,
 	return ret;
 }
 
+int reada_tree_block_flagged(struct btrfs_root *root, u64 bytenr, u32
blocksize,
+			 int mirror_num, struct extent_buffer **eb)
+{
+	struct extent_buffer *buf = NULL;
+	struct inode *btree_inode = root->fs_info->btree_inode;
+	struct extent_io_tree *io_tree = &BTRFS_I(btree_inode)->io_tree;
+	int ret;
+
+	buf = btrfs_find_create_tree_block(root, bytenr, blocksize);
+	if (!buf)
+		return 0;
+
+	set_bit(EXTENT_BUFFER_READAHEAD, &buf->bflags);
+
+	ret = read_extent_buffer_pages(io_tree, buf, 0, 0, 1, btree_get_extent,
+				 mirror_num);
+	if (ret) {
+		free_extent_buffer(buf);
+		return ret;
+	}
+
+	if (test_bit(EXTENT_BUFFER_CORRUPT, &buf->bflags)) {
+		*eb = buf;
+		return -EIO;
+	} else if (extent_buffer_uptodate(io_tree, buf, NULL)) {
+		*eb = buf;
+	} else {
+		free_extent_buffer(buf);
+	}
+	return 0;
+}
+
 struct extent_buffer *btrfs_find_tree_block(struct btrfs_root *root,
 					    u64 bytenr, u32 blocksize)
 {
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index a0b610a..fb35c4e 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -40,6 +40,8 @@ struct extent_buffer *read_tree_block(struct btrfs_root *root,
u64 bytenr,
 				      u32 blocksize, u64 parent_transid);
 int readahead_tree_block(struct btrfs_root *root, u64 bytenr, u32 blocksize,
 			 u64 parent_transid);
+int reada_tree_block_flagged(struct btrfs_root *root, u64 bytenr, u32
blocksize,
+			 int mirror_num, struct extent_buffer **eb);
 struct extent_buffer *btrfs_find_create_tree_block(struct btrfs_root *root,
 						   u64 bytenr, u32 blocksize);
 int clean_tree_block(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 5ef3735..c1be100 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -32,6 +32,7 @@
 #define EXTENT_BUFFER_BLOCKING 1
 #define EXTENT_BUFFER_DIRTY 2
 #define EXTENT_BUFFER_CORRUPT 3
+#define EXTENT_BUFFER_READAHEAD 4	/* this got triggered by readahead */
 
 /* these are flags for extent_clear_unlock_delalloc */
 #define EXTENT_CLEAR_UNLOCK_PAGE 0x1
-- 
1.7.3.4

--
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

Add state information for readahead to btrfs_fs_info and btrfs_device

Changes v2:
 - don''t wait in radix_trees
 - add own set of workers for readahead

Signed-off-by: Arne Jansen <sensille@gmx.net>
---
 fs/btrfs/ctree.h   |    5 +++++
 fs/btrfs/disk-io.c |   11 +++++++++++
 fs/btrfs/volumes.c |    8 ++++++++
 fs/btrfs/volumes.h |    8 ++++++++
 4 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8e948ec..51534bf 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1033,6 +1033,7 @@ struct btrfs_fs_info {
 	struct btrfs_workers endio_write_workers;
 	struct btrfs_workers endio_freespace_worker;
 	struct btrfs_workers submit_workers;
+	struct btrfs_workers readahead_workers;
 	/*
 	 * fixup workers take dirty pages that didn''t properly go through
 	 * the cow mechanism and make them safe to write.  It happens
@@ -1115,6 +1116,10 @@ struct btrfs_fs_info {
 	u64 fs_state;
 
 	struct btrfs_delayed_root *delayed_root;
+
+	/* readahead tree */
+	spinlock_t reada_lock;
+	struct radix_tree_root reada_tree;
 };
 
 /*
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 20edd2d..a1f85d6 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1681,6 +1681,10 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	fs_info->defrag_inodes = RB_ROOT;
 	fs_info->trans_no_join = 0;
 
+	/* readahead state */
+	INIT_RADIX_TREE(&fs_info->reada_tree, GFP_NOFS & ~__GFP_WAIT);
+	spin_lock_init(&fs_info->reada_lock);
+
 	fs_info->thread_pool_size = min_t(unsigned long,
 					  num_online_cpus() + 2, 8);
 
@@ -1870,6 +1874,9 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	btrfs_init_workers(&fs_info->delayed_workers, "delayed-meta",
 			   fs_info->thread_pool_size,
 			   &fs_info->generic_worker);
+	btrfs_init_workers(&fs_info->readahead_workers, "readahead",
+			   fs_info->thread_pool_size,
+			   &fs_info->generic_worker);
 
 	/*
 	 * endios are largely parallel and should have a very
@@ -1880,6 +1887,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 
 	fs_info->endio_write_workers.idle_thresh = 2;
 	fs_info->endio_meta_write_workers.idle_thresh = 2;
+	fs_info->readahead_workers.idle_thresh = 2;
 
 	btrfs_start_workers(&fs_info->workers, 1);
 	btrfs_start_workers(&fs_info->generic_worker, 1);
@@ -1892,6 +1900,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	btrfs_start_workers(&fs_info->endio_write_workers, 1);
 	btrfs_start_workers(&fs_info->endio_freespace_worker, 1);
 	btrfs_start_workers(&fs_info->delayed_workers, 1);
+	btrfs_start_workers(&fs_info->readahead_workers, 1);
 
 	fs_info->bdi.ra_pages *= btrfs_super_num_devices(disk_super);
 	fs_info->bdi.ra_pages = max(fs_info->bdi.ra_pages,
@@ -2149,6 +2158,7 @@ fail_sb_buffer:
 	btrfs_stop_workers(&fs_info->endio_freespace_worker);
 	btrfs_stop_workers(&fs_info->submit_workers);
 	btrfs_stop_workers(&fs_info->delayed_workers);
+	btrfs_stop_workers(&fs_info->readahead_workers);
 fail_alloc:
 	kfree(fs_info->delayed_root);
 fail_iput:
@@ -2616,6 +2626,7 @@ int close_ctree(struct btrfs_root *root)
 	btrfs_stop_workers(&fs_info->endio_freespace_worker);
 	btrfs_stop_workers(&fs_info->submit_workers);
 	btrfs_stop_workers(&fs_info->delayed_workers);
+	btrfs_stop_workers(&fs_info->readahead_workers);
 
 	btrfs_close_devices(fs_info->fs_devices);
 	btrfs_mapping_tree_free(&fs_info->mapping_tree);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1efa56e..a1a4b02 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -349,6 +349,14 @@ static noinline int device_list_add(const char *path,
 		}
 		INIT_LIST_HEAD(&device->dev_alloc_list);
 
+		/* init readahead state */
+		spin_lock_init(&device->reada_lock);
+		device->reada_curr_zone = NULL;
+		atomic_set(&device->reada_in_flight, 0);
+		device->reada_next = 0;
+		INIT_RADIX_TREE(&device->reada_zones, GFP_NOFS & ~__GFP_WAIT);
+		INIT_RADIX_TREE(&device->reada_extents, GFP_NOFS & ~__GFP_WAIT);
+
 		mutex_lock(&fs_devices->device_list_mutex);
 		list_add_rcu(&device->dev_list, &fs_devices->devices);
 		mutex_unlock(&fs_devices->device_list_mutex);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 7c12d61..63ac242 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -91,6 +91,14 @@ struct btrfs_device {
 	struct btrfs_work work;
 	struct rcu_head rcu;
 	struct work_struct rcu_work;
+
+	/* readahead state */
+	spinlock_t reada_lock;
+	atomic_t reada_in_flight;
+	u64 reada_next;
+	struct reada_zone *reada_curr_zone;
+	struct radix_tree_root reada_zones;
+	struct radix_tree_root reada_extents;
 };
 
 struct btrfs_fs_devices {
-- 
1.7.3.4

--
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

This is the implementation for the generic read ahead framework.

To trigger a readahead, btrfs_reada_add must be called. It will start
a read ahead for the given range [start, end) on tree root. The returned
handle can either be used to wait on the readahead to finish
(btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).

The read ahead works as follows:
On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
reada_start_machine will then search for extents to prefetch and trigger
some reads. When a read finishes for a node, all contained node/leaf
pointers that lie in the given range will also be enqueued. The reads will
be triggered in sequential order, thus giving a big win over a naive
enumeration. It will also make use of multi-device layouts. Each disk
will have its on read pointer and all disks will by utilized in parallel.
Also will no two disks read both sides of a mirror simultaneously, as this
would waste seeking capacity. Instead both disks will read different parts
of the filesystem.
Any number of readaheads can be started in parallel. The read order will be
determined globally, i.e. 2 parallel readaheads will normally finish faster
than the 2 started one after another.

Changes v2:
 - protect root->node by transaction instead of node_lock
 - fix missed branches:
    The readahead had a too simple check to determine if a branch from
    a node should be checked or not. It now also records the upper bound
    of each node to see if the requested RA range lies within.
 - use KERN_CONT to debug output, to avoid line breaks
 - defer reada_start_machine to worker to avoid deadlock

Changes v3:
 - protect root->node by rcu

Signed-off-by: Arne Jansen <sensille@gmx.net>

use rcu to protect root node
---
 fs/btrfs/Makefile |    3 +-
 fs/btrfs/ctree.h  |    8 +
 fs/btrfs/reada.c  |  994 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1004 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 9b72dcf..58302ca 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -7,4 +7,5 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o
root-tree.o dir-item.o \
 	   extent_map.o sysfs.o struct-funcs.o xattr.o ordered-data.o \
 	   extent_io.o volumes.o async-thread.o ioctl.o locking.o orphan.o \
 	   export.o tree-log.o acl.o free-space-cache.o zlib.o lzo.o \
-	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o
+	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
+	   reada.o
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 51534bf..78519df 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2681,4 +2681,12 @@ int btrfs_scrub_cancel_devid(struct btrfs_root *root, u64
devid);
 int btrfs_scrub_progress(struct btrfs_root *root, u64 devid,
 			 struct btrfs_scrub_progress *progress);
 
+/* reada.c */
+void *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);
+
 #endif
diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
new file mode 100644
index 0000000..e99c65c
--- /dev/null
+++ b/fs/btrfs/reada.c
@@ -0,0 +1,994 @@
+/*
+ * Copyright (C) 2011 STRATO.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <linux/sched.h>
+#include <linux/pagemap.h>
+#include <linux/writeback.h>
+#include <linux/blkdev.h>
+#include <linux/rbtree.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include "ctree.h"
+#include "volumes.h"
+#include "disk-io.h"
+#include "transaction.h"
+
+#undef DEBUG
+
+/*
+ * This is the implementation for the generic read ahead framework.
+ *
+ * To trigger a readahead, btrfs_reada_add must be called. It will start
+ * a read ahead for the given range [start, end) on tree root. The returned
+ * handle can either be used to wait on the readahead to finish
+ * (btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).
+ *
+ * The read ahead works as follows:
+ * On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
+ * reada_start_machine will then search for extents to prefetch and trigger
+ * some reads. When a read finishes for a node, all contained node/leaf
+ * pointers that lie in the given range will also be enqueued. The reads will
+ * be triggered in sequential order, thus giving a big win over a naive
+ * enumeration. It will also make use of multi-device layouts. Each disk
+ * will have its on read pointer and all disks will by utilized in parallel.
+ * Also will no two disks read both sides of a mirror simultaneously, as this
+ * would waste seeking capacity. Instead both disks will read different parts
+ * of the filesystem.
+ * Any number of readaheads can be started in parallel. The read order will be
+ * determined globally, i.e. 2 parallel readaheads will normally finish faster
+ * than the 2 started one after another.
+ */
+
+#define MAX_MIRRORS 2
+#define MAX_IN_FLIGHT 6
+
+struct reada_control {
+	struct btrfs_root	*root;		/* tree to prefetch */
+	struct btrfs_key	key_start;
+	struct btrfs_key	key_end;	/* exclusive */
+	spinlock_t		lock;
+	u64			elems;
+	u64			read_total;
+	struct kref		refcnt;
+	wait_queue_head_t	wait;
+};
+
+struct reada_extctl {
+	struct list_head	list;
+	struct reada_control	*rc;
+	u64			generation;
+};
+
+struct reada_extent {
+	u64			logical;
+	struct btrfs_key	top;
+	u32			blocksize;
+	int			err;
+	struct list_head	extctl;
+	struct kref		refcnt;
+	spinlock_t		lock;
+	struct reada_zone	*zones[MAX_MIRRORS];
+	int			nzones;
+	struct btrfs_device	*scheduled_for;
+};
+
+struct reada_zone {
+	u64			start;
+	u64			end;
+	u64			elems;
+	struct list_head	list;
+	spinlock_t		lock;
+	int			locked;
+	struct btrfs_device	*device;
+	struct btrfs_device	*devs[MAX_MIRRORS]; /* full list, incl self */
+	int			ndevs;
+	struct kref		refcnt;
+};
+
+struct reada_machine_work {
+	struct btrfs_work	work;
+	struct btrfs_fs_info	*fs_info;
+};
+
+static void reada_extent_put(struct btrfs_fs_info *, struct reada_extent *);
+static void reada_zone_put(struct reada_zone *);
+static void reada_control_put(struct reada_control *rc);
+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_key_cmp(struct btrfs_key *a, struct btrfs_key *b)
+{
+	if (a->objectid > b->objectid)
+		return 1;
+	if (a->objectid < b->objectid)
+		return -1;
+	if (a->type > b->type)
+		return 1;
+	if (a->type < b->type)
+		return -1;
+	if (a->offset > b->offset)
+		return 1;
+	if (a->offset < b->offset)
+		return -1;
+	return 0;
+}
+
+static int reada_add_block(struct reada_control *rc, u64 logical,
+			   struct btrfs_key *top, int level, u64 generation);
+
+/* 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)
+{
+	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)
+		kref_get(&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
+	 * don''t need the lock anymore
+	 */
+	list_replace_init(&re->extctl, &list);
+	for_dev = re->scheduled_for;
+	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;
+	}
+
+	for (i = 0; i < nritems; i++) {
+		struct reada_extctl *rec;
+		u64 n_gen;
+		struct btrfs_key key;
+		struct btrfs_key next_key;
+
+		btrfs_node_key_to_cpu(eb, &key, i);
+		if (i + 1 < nritems)
+			btrfs_node_key_to_cpu(eb, &next_key, i + 1);
+		else
+			next_key = re->top;
+		bytenr = btrfs_node_blockptr(eb, i);
+		n_gen = btrfs_node_ptr_generation(eb, i);
+
+		list_for_each_entry(rec, &list, list) {
+			struct reada_control *rc = rec->rc;
+
+			/*
+			 * if the generation doesn''t match, just ignore this
+			 * extctl. This will probably cut off a branch from
+			 * prefetch. Alternatively one could start a new (sub-)
+			 * prefetch for this branch, starting again from root.
+			 * FIXME: move the generation check out of this loop
+			 */
+#ifdef DEBUG
+			if (rec->generation != generation) {
+				printk(KERN_DEBUG "generation mismatch for "
+						"(%llu,%d,%llu) %llu != %llu\n",
+				       key.objectid, key.type, key.offset,
+				       rec->generation, generation);
+			}
+#endif
+			if (rec->generation == generation &&
+			    reada_key_cmp(&key, &rc->key_end) < 0 &&
+			    reada_key_cmp(&next_key, &rc->key_start) > 0)
+				reada_add_block(rc, bytenr, &next_key,
+						level - 1, n_gen);
+		}
+	}
+	/*
+	 * free extctl records
+	 */
+	while (!list_empty(&list)) {
+		struct reada_control *rc;
+		struct reada_extctl *rec;
+
+		rec = list_first_entry(&list, struct reada_extctl, list);
+		list_del(&rec->list);
+		rc = rec->rc;
+		kfree(rec);
+
+		kref_get(&rc->refcnt);
+		spin_lock(&rc->lock);
+		--rc->elems;
+		if (rc->elems == 0) {
+			reada_control_put(rc);
+			wake_up(&rc->wait);
+		}
+		spin_unlock(&rc->lock);
+		reada_control_put(rc);
+
+		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;
+}
+
+/*
+ * 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 ret;
+
+	ret = __readahead_hook(root, eb, start, err);
+
+	reada_start_machine(root->fs_info);
+
+	return ret;
+}
+
+static struct reada_zone *reada_find_zone(struct btrfs_fs_info *fs_info,
+					  struct btrfs_device *dev, u64 logical,
+					  struct btrfs_multi_bio *multi)
+{
+	int ret;
+	int looped = 0;
+	struct reada_zone *zone;
+	struct btrfs_block_group_cache *cache = NULL;
+	u64 start;
+	u64 end;
+	int i;
+
+again:
+	zone = NULL;
+	spin_lock(&fs_info->reada_lock);
+	ret = radix_tree_gang_lookup(&dev->reada_zones, (void **)&zone,
+				     logical >> PAGE_CACHE_SHIFT, 1);
+	if (ret == 1)
+		kref_get(&zone->refcnt);
+	spin_unlock(&fs_info->reada_lock);
+
+	if (ret == 1) {
+		if (logical >= zone->start && logical < zone->end)
+			return zone;
+		spin_lock(&fs_info->reada_lock);
+		reada_zone_put(zone);
+		spin_unlock(&fs_info->reada_lock);
+	}
+
+	if (looped)
+		return NULL;
+
+	cache = btrfs_lookup_block_group(fs_info, logical);
+	if (!cache)
+		return NULL;
+
+	start = cache->key.objectid;
+	end = start + cache->key.offset - 1;
+	btrfs_put_block_group(cache);
+
+	zone = kzalloc(sizeof(*zone), GFP_NOFS);
+	if (!zone)
+		return NULL;
+
+	zone->start = start;
+	zone->end = end;
+	INIT_LIST_HEAD(&zone->list);
+	spin_lock_init(&zone->lock);
+	zone->locked = 0;
+	kref_init(&zone->refcnt);
+	zone->elems = 0;
+	zone->device = dev; /* our device always sits at index 0 */
+	for (i = 0; i < multi->num_stripes; ++i) {
+		/* bounds have already been checked */
+		zone->devs[i] = multi->stripes[i].dev;
+	}
+	zone->ndevs = multi->num_stripes;
+
+	spin_lock(&fs_info->reada_lock);
+	ret = radix_tree_insert(&dev->reada_zones,
+				(unsigned long)zone->end >> PAGE_CACHE_SHIFT,
+				zone);
+	spin_unlock(&fs_info->reada_lock);
+
+	if (ret) {
+		kfree(zone);
+		looped = 1;
+		goto again;
+	}
+
+	return zone;
+}
+
+static struct reada_extent *reada_find_extent(struct btrfs_root *root,
+					      u64 logical,
+					      struct btrfs_key *top, int level)
+{
+	int ret;
+	int looped = 0;
+	struct reada_extent *re = NULL;
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
+	struct btrfs_multi_bio *multi = NULL;
+	struct btrfs_device *dev;
+	u32 blocksize;
+	u64 length;
+	int nzones = 0;
+	int i;
+	unsigned long index = logical >> PAGE_CACHE_SHIFT;
+
+again:
+	spin_lock(&fs_info->reada_lock);
+	re = radix_tree_lookup(&fs_info->reada_tree, index);
+	if (re)
+		kref_get(&re->refcnt);
+	spin_unlock(&fs_info->reada_lock);
+
+	if (re || looped)
+		return re;
+
+	re = kzalloc(sizeof(*re), GFP_NOFS);
+	if (!re)
+		return NULL;
+
+	blocksize = btrfs_level_size(root, level);
+	re->logical = logical;
+	re->blocksize = blocksize;
+	re->top = *top;
+	INIT_LIST_HEAD(&re->extctl);
+	spin_lock_init(&re->lock);
+	kref_init(&re->refcnt);
+
+	/*
+	 * map block
+	 */
+	length = blocksize;
+	ret = btrfs_map_block(map_tree, REQ_WRITE, logical, &length, &multi,
0);
+	if (ret || !multi || length < blocksize)
+		goto error;
+
+	if (multi->num_stripes > MAX_MIRRORS) {
+		printk(KERN_ERR "btrfs readahead: more than %d copies not "
+				"supported", MAX_MIRRORS);
+		goto error;
+	}
+
+	for (nzones = 0; nzones < multi->num_stripes; ++nzones) {
+		struct reada_zone *zone;
+
+		dev = multi->stripes[nzones].dev;
+		zone = reada_find_zone(fs_info, dev, logical, multi);
+		if (!zone)
+			break;
+
+		re->zones[nzones] = zone;
+		spin_lock(&zone->lock);
+		if (!zone->elems)
+			kref_get(&zone->refcnt);
+		++zone->elems;
+		spin_unlock(&zone->lock);
+		spin_lock(&fs_info->reada_lock);
+		reada_zone_put(zone);
+		spin_unlock(&fs_info->reada_lock);
+	}
+	re->nzones = nzones;
+	if (nzones == 0) {
+		/* not a single zone found, error and out */
+		goto error;
+	}
+
+	/* insert extent in reada_tree + all per-device trees, all or nothing */
+	spin_lock(&fs_info->reada_lock);
+	ret = radix_tree_insert(&fs_info->reada_tree, index, re);
+	if (ret) {
+		spin_unlock(&fs_info->reada_lock);
+		if (ret != -ENOMEM) {
+			/* someone inserted the extent in the meantime */
+			looped = 1;
+		}
+		goto error;
+	}
+	for (i = 0; i < nzones; ++i) {
+		dev = multi->stripes[i].dev;
+		ret = radix_tree_insert(&dev->reada_extents, index, re);
+		if (ret) {
+			while (--i >= 0) {
+				dev = multi->stripes[i].dev;
+				BUG_ON(dev == NULL);
+				radix_tree_delete(&dev->reada_extents, index);
+			}
+			BUG_ON(fs_info == NULL);
+			radix_tree_delete(&fs_info->reada_tree, index);
+			spin_unlock(&fs_info->reada_lock);
+			goto error;
+		}
+	}
+	spin_unlock(&fs_info->reada_lock);
+
+	return re;
+
+error:
+	while (nzones) {
+		struct reada_zone *zone;
+
+		--nzones;
+		zone = re->zones[nzones];
+		kref_get(&zone->refcnt);
+		spin_lock(&zone->lock);
+		--zone->elems;
+		if (zone->elems == 0) {
+			/*
+			 * no fs_info->reada_lock needed, as this can''t be
+			 * the last ref
+			 */
+			reada_zone_put(zone);
+		}
+		spin_unlock(&zone->lock);
+
+		spin_lock(&fs_info->reada_lock);
+		reada_zone_put(zone);
+		spin_unlock(&fs_info->reada_lock);
+	}
+	kfree(re);
+	if (looped)
+		goto again;
+	return NULL;
+}
+
+static void reada_kref_dummy(struct kref *kr)
+{
+}
+
+static void reada_extent_put(struct btrfs_fs_info *fs_info,
+			     struct reada_extent *re)
+{
+	int i;
+	unsigned long index = re->logical >> PAGE_CACHE_SHIFT;
+
+	spin_lock(&fs_info->reada_lock);
+	if (!kref_put(&re->refcnt, reada_kref_dummy)) {
+		spin_unlock(&fs_info->reada_lock);
+		return;
+	}
+
+	radix_tree_delete(&fs_info->reada_tree, index);
+	for (i = 0; i < re->nzones; ++i) {
+		struct reada_zone *zone = re->zones[i];
+
+		radix_tree_delete(&zone->device->reada_extents, index);
+	}
+
+	spin_unlock(&fs_info->reada_lock);
+
+	for (i = 0; i < re->nzones; ++i) {
+		struct reada_zone *zone = re->zones[i];
+
+		kref_get(&zone->refcnt);
+		spin_lock(&zone->lock);
+		--zone->elems;
+		if (zone->elems == 0) {
+			/* no fs_info->reada_lock needed, as this can''t be
+			 * the last ref */
+			reada_zone_put(zone);
+		}
+		spin_unlock(&zone->lock);
+
+		spin_lock(&fs_info->reada_lock);
+		reada_zone_put(zone);
+		spin_unlock(&fs_info->reada_lock);
+	}
+	if (re->scheduled_for)
+		atomic_dec(&re->scheduled_for->reada_in_flight);
+
+	kfree(re);
+}
+
+/* call it with fs_info->reada_lock held */
+static void reada_zone_put(struct reada_zone *zone)
+{
+	if (!kref_put(&zone->refcnt, reada_kref_dummy))
+		return;
+
+	radix_tree_delete(&zone->device->reada_zones,
+			  zone->end >> PAGE_CACHE_SHIFT);
+
+	kfree(zone);
+
+	return;
+}
+
+static void reada_control_put(struct reada_control *rc)
+{
+	if (kref_put(&rc->refcnt, reada_kref_dummy)) {
+		kfree(rc);
+		return;
+	}
+}
+
+static int reada_add_block(struct reada_control *rc, u64 logical,
+			   struct btrfs_key *top, int level, 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 */
+	if (!re)
+		return -1;
+
+	rec = kzalloc(sizeof(*rec), GFP_NOFS);
+	if (!rec) {
+		reada_extent_put(root->fs_info, re);
+		return -1;
+	}
+
+	rec->rc = rc;
+	rec->generation = generation;
+	spin_lock(&rc->lock);
+	++rc->elems;
+	spin_unlock(&rc->lock);
+
+	spin_lock(&re->lock);
+	list_add_tail(&rec->list, &re->extctl);
+	spin_unlock(&re->lock);
+
+	/* leave the ref on the extent */
+
+	return 0;
+}
+
+/*
+ * called with fs_info->reada_lock held
+ */
+static void reada_peer_zones_set_lock(struct reada_zone *zone, int lock)
+{
+	int i;
+	unsigned long index = zone->end >> PAGE_CACHE_SHIFT;
+
+	for (i = 0; i < zone->ndevs; ++i) {
+		struct reada_zone *peer;
+		peer = radix_tree_lookup(&zone->devs[i]->reada_zones, index);
+		if (peer && peer->device != zone->device)
+			peer->locked = lock;
+	}
+}
+
+/*
+ * called with fs_info->reada_lock held
+ */
+static int reada_pick_zone(struct btrfs_device *dev)
+{
+	struct reada_zone *top_zone = NULL;
+	struct reada_zone *top_locked_zone = NULL;
+	u64 top_elems = 0;
+	u64 top_locked_elems = 0;
+	unsigned long index = 0;
+	int ret;
+
+	if (dev->reada_curr_zone) {
+		reada_peer_zones_set_lock(dev->reada_curr_zone, 0);
+		reada_zone_put(dev->reada_curr_zone);
+		dev->reada_curr_zone = NULL;
+	}
+	/* pick the zone with the most elements */
+	while (1) {
+		struct reada_zone *zone;
+
+		ret = radix_tree_gang_lookup(&dev->reada_zones,
+					     (void **)&zone, index, 1);
+		if (ret == 0)
+			break;
+		index = (zone->end >> PAGE_CACHE_SHIFT) + 1;
+		if (zone->locked) {
+			if (zone->elems > top_locked_elems) {
+				top_locked_elems = zone->elems;
+				top_locked_zone = zone;
+			}
+		} else {
+			if (zone->elems > top_elems) {
+				top_elems = zone->elems;
+				top_zone = zone;
+			}
+		}
+	}
+	if (top_zone)
+		dev->reada_curr_zone = top_zone;
+	else if (top_locked_zone)
+		dev->reada_curr_zone = top_locked_zone;
+	else
+		return 0;
+
+	dev->reada_next = dev->reada_curr_zone->start;
+	kref_get(&dev->reada_curr_zone->refcnt);
+	reada_peer_zones_set_lock(dev->reada_curr_zone, 1);
+
+	return 1;
+}
+
+static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
+				   struct btrfs_device *dev)
+{
+	struct reada_extent *re = NULL;
+	int mirror_num = 0;
+	struct extent_buffer *eb = NULL;
+	u64 logical;
+	u32 blocksize;
+	int ret;
+	int i;
+	int need_kick = 0;
+
+	spin_lock(&fs_info->reada_lock);
+	if (dev->reada_curr_zone == NULL) {
+		ret = reada_pick_zone(dev);
+		if (!ret) {
+			spin_unlock(&fs_info->reada_lock);
+			return 0;
+		}
+	}
+	/*
+	 * FIXME currently we issue the reads one extent at a time. If we have
+	 * a contiguous block of extents, we could also coagulate them or use
+	 * plugging to speed things up
+	 */
+	ret = radix_tree_gang_lookup(&dev->reada_extents, (void **)&re,
+				     dev->reada_next >> PAGE_CACHE_SHIFT, 1);
+	if (ret == 0 || re->logical >= dev->reada_curr_zone->end) {
+		ret = reada_pick_zone(dev);
+		if (!ret) {
+			spin_unlock(&fs_info->reada_lock);
+			return 0;
+		}
+		re = NULL;
+		ret = radix_tree_gang_lookup(&dev->reada_extents, (void **)&re,
+					dev->reada_next >> PAGE_CACHE_SHIFT, 1);
+	}
+	if (ret == 0) {
+		spin_unlock(&fs_info->reada_lock);
+		return 0;
+	}
+	dev->reada_next = re->logical + re->blocksize;
+	kref_get(&re->refcnt);
+
+	spin_unlock(&fs_info->reada_lock);
+
+	/*
+	 * find mirror num
+	 */
+	for (i = 0; i < re->nzones; ++i) {
+		if (re->zones[i]->device == dev) {
+			mirror_num = i + 1;
+			break;
+		}
+	}
+	logical = re->logical;
+	blocksize = re->blocksize;
+
+	spin_lock(&re->lock);
+	if (re->scheduled_for == NULL) {
+		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, blocksize,
+			 mirror_num, &eb);
+	if (eb) {
+		__readahead_hook(fs_info->extent_root, eb, eb->start, ret);
+		free_extent_buffer(eb);
+	}
+
+	return 1;
+
+}
+
+static void reada_start_machine_worker(struct btrfs_work *work)
+{
+	struct reada_machine_work *rmw;
+	struct btrfs_fs_info *fs_info;
+
+	rmw = container_of(work, struct reada_machine_work, work);
+	fs_info = rmw->fs_info;
+
+	kfree(rmw);
+
+	__reada_start_machine(fs_info);
+}
+
+static void __reada_start_machine(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_device *device;
+	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
+	u64 enqueued;
+	u64 total = 0;
+	int i;
+
+	do {
+		enqueued = 0;
+		list_for_each_entry(device, &fs_devices->devices, dev_list) {
+			if (atomic_read(&device->reada_in_flight) <
+			    MAX_IN_FLIGHT)
+				enqueued += reada_start_machine_dev(fs_info,
+								    device);
+		}
+		total += enqueued;
+	} while (enqueued && total < 10000);
+
+	if (enqueued == 0)
+		return;
+
+	/*
+	 * If everything is already in the cache, this is effectively single
+	 * threaded. To a) not hold the caller for too long and b) to utilize
+	 * more cores, we broke the loop above after 10000 iterations and now
+	 * enqueue to workers to finish it. This will distribute the load to
+	 * the cores.
+	 * FIXME we might need our own workqueue here, with an idle threshold
+	 * of one. Also these worker are relatively long-running.
+	 */
+	for (i = 0; i < 2; ++i)
+		reada_start_machine(fs_info);
+}
+
+static void reada_start_machine(struct btrfs_fs_info *fs_info)
+{
+	struct reada_machine_work *rmw;
+
+	rmw = kzalloc(sizeof(*rmw), GFP_NOFS);
+	if (!rmw) {
+		/* FIXME we cannot handle this properly right now */
+		BUG();
+	}
+	rmw->work.func = reada_start_machine_worker;
+	rmw->fs_info = fs_info;
+
+	btrfs_queue_worker(&fs_info->readahead_workers, &rmw->work);
+}
+
+#ifdef DEBUG
+static void dump_devs(struct btrfs_fs_info *fs_info, int all)
+{
+	struct btrfs_device *device;
+	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
+	unsigned long index;
+	int ret;
+	int i;
+	int j;
+	int cnt;
+
+	spin_lock(&fs_info->reada_lock);
+	list_for_each_entry(device, &fs_devices->devices, dev_list) {
+		printk(KERN_DEBUG "dev %lld has %d in flight\n", device->devid,
+			atomic_read(&device->reada_in_flight));
+		index = 0;
+		while (1) {
+			struct reada_zone *zone;
+			ret = radix_tree_gang_lookup(&device->reada_zones,
+						     (void **)&zone, index, 1);
+			if (ret == 0)
+				break;
+			printk(KERN_DEBUG "  zone %llu-%llu elems %llu locked "
+				"%d devs", zone->start, zone->end, zone->elems,
+				zone->locked);
+			for (j = 0; j < zone->ndevs; ++j) {
+				printk(KERN_CONT " %lld",
+					zone->devs[j]->devid);
+			}
+			if (device->reada_curr_zone == zone)
+				printk(KERN_CONT " curr off %llu",
+					device->reada_next - zone->start);
+			printk(KERN_CONT "\n");
+			index = (zone->end >> PAGE_CACHE_SHIFT) + 1;
+		}
+		cnt = 0;
+		index = 0;
+		while (all) {
+			struct reada_extent *re = NULL;
+
+			ret = radix_tree_gang_lookup(&device->reada_extents,
+						     (void **)&re, index, 1);
+			if (ret == 0)
+				break;
+			printk(KERN_DEBUG
+				"  re: logical %llu size %u empty %d for %lld",
+				re->logical, re->blocksize,
+				list_empty(&re->extctl), re->scheduled_for ?
+				re->scheduled_for->devid : -1);
+
+			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);
+				}
+			}
+			printk(KERN_CONT "\n");
+			index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
+			if (++cnt > 15)
+				break;
+		}
+	}
+
+	index = 0;
+	cnt = 0;
+	while (all) {
+		struct reada_extent *re = NULL;
+
+		ret = radix_tree_gang_lookup(&fs_info->reada_tree, (void **)&re,
+					     index, 1);
+		if (ret == 0)
+			break;
+		if (!re->scheduled_for) {
+			index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
+			continue;
+		}
+		printk(KERN_DEBUG
+			"re: logical %llu size %u list empty %d for %lld",
+			re->logical, re->blocksize, list_empty(&re->extctl),
+			re->scheduled_for ? re->scheduled_for->devid : -1);
+		for (i = 0; i < re->nzones; ++i) {
+			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);
+				}
+			}
+		}
+		printk(KERN_CONT "\n");
+		index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
+	}
+	spin_unlock(&fs_info->reada_lock);
+}
+#endif
+
+/*
+ * interface
+ */
+void *btrfs_reada_add(struct btrfs_root *root, struct btrfs_key *key_start,
+		      struct btrfs_key *key_end)
+{
+	struct reada_control *rc;
+	u64 start;
+	u64 generation;
+	int level;
+	struct extent_buffer *node;
+	static struct btrfs_key max_key = {
+		.objectid = (u64)-1,
+		.type = (u8)-1,
+		.offset = (u64)-1
+	};
+
+	rc = kzalloc(sizeof(*rc), GFP_NOFS);
+	if (!rc)
+		return ERR_PTR(-ENOMEM);
+
+	rc->root = root;
+	rc->key_start = *key_start;
+	rc->key_end = *key_end;
+	spin_lock_init(&rc->lock);
+	rc->elems = 0;
+	rc->read_total = 0;
+	init_waitqueue_head(&rc->wait);
+	kref_init(&rc->refcnt);
+	kref_get(&rc->refcnt); /* one ref for having elements */
+
+	node = btrfs_root_node(root);
+	start = node->start;
+	level = btrfs_header_level(node);
+	generation = btrfs_header_generation(node);
+	free_extent_buffer(node);
+
+	reada_add_block(rc, start, &max_key, level, generation);
+
+	reada_start_machine(root->fs_info);
+
+	return rc;
+}
+
+#ifdef DEBUG
+int btrfs_reada_wait(void *handle)
+{
+	struct reada_control *rc = handle;
+
+	spin_lock(&rc->lock);
+	while (rc->elems) {
+		spin_unlock(&rc->lock);
+		wait_event_timeout(rc->wait, rc->elems == 0, 5 * HZ);
+		dump_devs(rc->root->fs_info, rc->elems < 10 ? 1 : 0);
+		spin_lock(&rc->lock);
+	}
+	spin_unlock(&rc->lock);
+
+	dump_devs(rc->root->fs_info, rc->elems < 10 ? 1 : 0);
+
+	reada_control_put(rc);
+
+	return 0;
+}
+#else
+int btrfs_reada_wait(void *handle)
+{
+	struct reada_control *rc = handle;
+
+	spin_lock(&rc->lock);
+	while (rc->elems) {
+		spin_unlock(&rc->lock);
+		wait_event(rc->wait, rc->elems == 0);
+		spin_lock(&rc->lock);
+	}
+	spin_unlock(&rc->lock);
+
+	reada_control_put(rc);
+
+	return 0;
+}
+#endif
+
+void btrfs_reada_detach(void *handle)
+{
+	struct reada_control *rc = handle;
+
+	reada_control_put(rc);
+}
-- 
1.7.3.4

--
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

This adds the hooks needed for readahead. In the readpage_end_io_hook,
the extent state is checked for the EXTENT_READAHEAD flag. Only in this
case the readahead hook is called, to keep the impact on non-ra as low
as possible.
Additionally, a hook for a failed IO is added, otherwise readahead would
wait indefinitely for the extent to finish.

Changes for v2:
 - eliminate race condition

Signed-off-by: Arne Jansen <sensille@gmx.net>
---
 fs/btrfs/disk-io.c   |   37 +++++++++++++++++++++++++++++++++++++
 fs/btrfs/extent_io.c |    2 +-
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a1f85d6..353c253 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -574,11 +574,47 @@ static int btree_readpage_end_io_hook(struct page *page,
u64 start, u64 end,
 	end = min_t(u64, eb->len, PAGE_CACHE_SIZE);
 	end = eb->start + end - 1;
 err:
+	if (test_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags)) {
+		clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags);
+		btree_readahead_hook(root, eb, eb->start, ret);
+	}
+
 	free_extent_buffer(eb);
 out:
 	return ret;
 }
 
+static int btree_io_failed_hook(struct bio *failed_bio,
+			 struct page *page, u64 start, u64 end,
+			 struct extent_state *state)
+{
+	struct extent_io_tree *tree;
+	unsigned long len;
+	struct extent_buffer *eb;
+	struct btrfs_root *root = BTRFS_I(page->mapping->host)->root;
+
+	tree = &BTRFS_I(page->mapping->host)->io_tree;
+	if (page->private == EXTENT_PAGE_PRIVATE)
+		goto out;
+	if (!page->private)
+		goto out;
+
+	len = page->private >> 2;
+	WARN_ON(len == 0);
+
+	eb = alloc_extent_buffer(tree, start, len, page);
+	if (eb == NULL)
+		goto out;
+
+	if (test_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags)) {
+		clear_bit(EXTENT_BUFFER_READAHEAD, &eb->bflags);
+		btree_readahead_hook(root, eb, eb->start, -EIO);
+	}
+
+out:
+	return -EIO;	/* we fixed nothing */
+}
+
 static void end_workqueue_bio(struct bio *bio, int err)
 {
 	struct end_io_wq *end_io_wq = bio->bi_private;
@@ -3133,6 +3169,7 @@ static int btrfs_cleanup_transaction(struct btrfs_root
*root)
 static struct extent_io_ops btree_extent_io_ops = {
 	.write_cache_pages_lock_hook = btree_lock_page_hook,
 	.readpage_end_io_hook = btree_readpage_end_io_hook,
+	.readpage_io_failed_hook = btree_io_failed_hook,
 	.submit_bio_hook = btree_submit_bio_hook,
 	/* note we''re sharing with inode.c for the merge bio hook */
 	.merge_bio_hook = btrfs_merge_bio_hook,
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index b7f0b9b..19ca500 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1774,7 +1774,7 @@ static void end_bio_extent_readpage(struct bio *bio, int
err)
 		if (!uptodate && tree->ops &&
 		    tree->ops->readpage_io_failed_hook) {
 			ret = tree->ops->readpage_io_failed_hook(bio, page,
-							 start, end, NULL);
+							 start, end, state);
 			if (ret == 0) {
 				uptodate  					test_bit(BIO_UPTODATE, &bio->bi_flags);
-- 
1.7.3.4

--
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

This ioctl is added to trigger a readahead from user mode. It implements
a readahead using the new interface and also a traditional tree walk.
This way it''s possible to measure the two side by side.

Signed-off-by: Arne Jansen <sensille@gmx.net>
---
 fs/btrfs/ioctl.c |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/btrfs/ioctl.h |   16 +++++++++
 2 files changed, 106 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a3c4751..6e77c20 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2784,6 +2784,93 @@ static noinline long btrfs_ioctl_wait_sync(struct file
*file, void __user *argp)
 	return btrfs_wait_for_commit(root, transid);
 }
 
+static noinline long btrfs_ioctl_reada_test(struct btrfs_fs_info *fs_info,
+					    void __user *argp)
+{
+	struct btrfs_key start = { 0 };
+	struct btrfs_key end = {
+		.objectid = (u64)-1,
+		.type = (u8)-1,
+		.offset = (u64)-1
+	};
+	struct btrfs_ioctl_reada_args reada_args;
+	struct btrfs_key key;
+	struct btrfs_root *root = NULL;
+
+	if (!argp)
+		return -EINVAL;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(&reada_args,
+			   (struct btrfs_ioctl_reada_args __user *)argp,
+			   sizeof(reada_args)))
+		return -EFAULT;
+
+	start.objectid = reada_args.start_objectid;
+	start.type = reada_args.start_type;
+	start.offset = reada_args.start_offset;
+	end.objectid = reada_args.end_objectid;
+	end.type = reada_args.end_type;
+	end.offset = reada_args.end_offset;
+
+	key.objectid = reada_args.tree;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = (u64)-1;
+	root = btrfs_read_fs_root_no_name(fs_info, &key);
+	if (IS_ERR(root))
+		return -ENOENT;
+
+	if (!(reada_args.flags & BTRFS_READA_IOC_FLAGS_TRAD)) {
+		void *handle;
+
+		handle = btrfs_reada_add(root, &start, &end);
+		if (IS_ERR(handle))
+			return PTR_ERR(handle);
+
+		if (reada_args.flags & BTRFS_READA_IOC_FLAGS_WAIT)
+			btrfs_reada_wait(handle);
+		else
+			btrfs_reada_detach(handle);
+	} else {
+		struct btrfs_path *path;
+		struct extent_buffer *leaf;
+		int slot;
+		int ret;
+
+		/*
+		 * enumerate the tree the traditional way
+		 */
+		path = btrfs_alloc_path();
+		if (!path)
+			return -ENOMEM;
+		path->reada = 2;
+
+		ret = btrfs_search_slot(NULL, root, &start, path, 0, 0);
+		if (ret < 0)
+			goto out;
+
+		do {
+			leaf = path->nodes[0];
+			slot = path->slots[0];
+			btrfs_item_key_to_cpu(leaf, &key, slot);
+
+			if (key.objectid > end.objectid)
+				break;
+			if (key.objectid == end.objectid && key.type > end.type)
+				break;
+			if (key.objectid == end.objectid &&
+			    key.type == end.type && key.offset > end.offset)
+				break;
+		} while ((ret = btrfs_next_leaf(root, path)) == 0);
+out:
+		btrfs_free_path(path);
+		return ret >= 0 ? 0 : ret;
+	}
+	return 0;
+}
+
 static long btrfs_ioctl_scrub(struct btrfs_root *root, void __user *arg)
 {
 	int ret;
@@ -2836,8 +2923,7 @@ static long btrfs_ioctl_scrub_progress(struct btrfs_root
*root,
 	return ret;
 }
 
-long btrfs_ioctl(struct file *file, unsigned int
-		cmd, unsigned long arg)
+long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
 	void __user *argp = (void __user *)arg;
@@ -2908,7 +2994,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_scrub_cancel(root, argp);
 	case BTRFS_IOC_SCRUB_PROGRESS:
 		return btrfs_ioctl_scrub_progress(root, argp);
+	case BTRFS_IOC_READA_TEST:
+		return btrfs_ioctl_reada_test(root->fs_info, argp);
 	}
-
 	return -ENOTTY;
 }
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index ad1ea78..ee83259 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -193,6 +193,20 @@ struct btrfs_ioctl_space_args {
 	struct btrfs_ioctl_space_info spaces[0];
 };
 
+#define BTRFS_READA_IOC_FLAGS_WAIT	1
+#define BTRFS_READA_IOC_FLAGS_TRAD	2
+struct btrfs_ioctl_reada_args {
+	__u64 flags;
+	__u64 tree;
+	__u64 start_objectid;
+	__u8 start_type;
+	__u64 start_offset;
+	__u64 end_objectid;
+	__u8 end_type;
+	__u64 end_offset;
+	__u64 unused[100];
+};
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -248,4 +262,6 @@ struct btrfs_ioctl_space_args {
 				 struct btrfs_ioctl_dev_info_args)
 #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \
 			       struct btrfs_ioctl_fs_info_args)
+#define BTRFS_IOC_READA_TEST _IOW(BTRFS_IOCTL_MAGIC, 99, \
+				struct btrfs_ioctl_reada_args)
 #endif
-- 
1.7.3.4

--
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

Scrub uses a simple tree-enumeration to bring the relevant portions
of the extent- and csum-tree into the page cache before starting the
scrub-I/O. This is now replaced by using the new readahead-API.
During readahead the scrub is being accounted as paused, so it won''t
hold off transaction commits.

This change raises the average disk bandwith utilisation on my test
volume from 70% to 90%. On another volume, the time for a test run
went down from 89s to 43s.

Signed-off-by: Arne Jansen <sensille@gmx.net>
---
 fs/btrfs/scrub.c |  116 ++++++++++++++++++++++++------------------------------
 1 files changed, 52 insertions(+), 64 deletions(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index a8d03d5..ce5208c 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -29,15 +29,12 @@
  * any can be found.
  *
  * Future enhancements:
- *  - To enhance the performance, better read-ahead strategies for the
- *    extent-tree can be employed.
  *  - In case an unrepairable extent is encountered, track which files are
  *    affected and report them
  *  - In case of a read error on files with nodatasum, map the file and read
  *    the extent to trigger a writeback of the good copy
  *  - track and record media errors, throw out bad devices
  *  - add a mode to also read unallocated space
- *  - make the prefetch cancellable
  */
 
 struct scrub_bio;
@@ -741,13 +738,16 @@ static noinline_for_stack int scrub_stripe(struct
scrub_dev *sdev,
 	int slot;
 	int i;
 	u64 nstripes;
-	int start_stripe;
 	struct extent_buffer *l;
 	struct btrfs_key key;
 	u64 physical;
 	u64 logical;
 	u64 generation;
 	u64 mirror_num;
+	void *reada1;
+	void *reada2;
+	struct btrfs_key key_start;
+	struct btrfs_key key_end;
 
 	u64 increment = map->stripe_len;
 	u64 offset;
@@ -779,81 +779,67 @@ static noinline_for_stack int scrub_stripe(struct
scrub_dev *sdev,
 	if (!path)
 		return -ENOMEM;
 
-	path->reada = 2;
 	path->search_commit_root = 1;
 	path->skip_locking = 1;
 
 	/*
-	 * find all extents for each stripe and just read them to get
-	 * them into the page cache
-	 * FIXME: we can do better. build a more intelligent prefetching
+	 * trigger the readahead for extent tree csum tree and wait for
+	 * completion. During readahead, the scrub is officially paused
+	 * to not hold off transaction commits
 	 */
 	logical = base + offset;
-	physical = map->stripes[num].physical;
-	ret = 0;
-	for (i = 0; i < nstripes; ++i) {
-		key.objectid = logical;
-		key.type = BTRFS_EXTENT_ITEM_KEY;
-		key.offset = (u64)0;
-
-		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-		if (ret < 0)
-			goto out_noplug;
-
-		/*
-		 * we might miss half an extent here, but that doesn''t matter,
-		 * as it''s only the prefetch
-		 */
-		while (1) {
-			l = path->nodes[0];
-			slot = path->slots[0];
-			if (slot >= btrfs_header_nritems(l)) {
-				ret = btrfs_next_leaf(root, path);
-				if (ret == 0)
-					continue;
-				if (ret < 0)
-					goto out_noplug;
 
-				break;
-			}
-			btrfs_item_key_to_cpu(l, &key, slot);
+	wait_event(sdev->list_wait,
+		   atomic_read(&sdev->in_flight) == 0);
+	atomic_inc(&fs_info->scrubs_paused);
+	wake_up(&fs_info->scrub_pause_wait);
 
-			if (key.objectid >= logical + map->stripe_len)
-				break;
+	/* FIXME it might be better to start readahead at commit root */
+	key_start.objectid = logical;
+	key_start.type = BTRFS_EXTENT_ITEM_KEY;
+	key_start.offset = (u64)0;
+	key_end.objectid = base + offset + nstripes * increment;
+	key_end.type = BTRFS_EXTENT_ITEM_KEY;
+	key_end.offset = (u64)0;
+	reada1 = btrfs_reada_add(root, &key_start, &key_end);
+
+	key_start.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
+	key_start.type = BTRFS_EXTENT_CSUM_KEY;
+	key_start.offset = logical;
+	key_end.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
+	key_end.type = BTRFS_EXTENT_CSUM_KEY;
+	key_end.offset = base + offset + nstripes * increment;
+	reada2 = btrfs_reada_add(csum_root, &key_start, &key_end);
+
+	if (!IS_ERR(reada1))
+		btrfs_reada_wait(reada1);
+	if (!IS_ERR(reada2))
+		btrfs_reada_wait(reada2);
 
-			path->slots[0]++;
-		}
-		btrfs_release_path(path);
-		logical += increment;
-		physical += map->stripe_len;
-		cond_resched();
+	mutex_lock(&fs_info->scrub_lock);
+	while (atomic_read(&fs_info->scrub_pause_req)) {
+		mutex_unlock(&fs_info->scrub_lock);
+		wait_event(fs_info->scrub_pause_wait,
+		   atomic_read(&fs_info->scrub_pause_req) == 0);
+		mutex_lock(&fs_info->scrub_lock);
 	}
+	atomic_dec(&fs_info->scrubs_paused);
+	mutex_unlock(&fs_info->scrub_lock);
+	wake_up(&fs_info->scrub_pause_wait);
 
 	/*
 	 * collect all data csums for the stripe to avoid seeking during
 	 * the scrub. This might currently (crc32) end up to be about 1MB
 	 */
-	start_stripe = 0;
 	blk_start_plug(&plug);
-again:
-	logical = base + offset + start_stripe * increment;
-	for (i = start_stripe; i < nstripes; ++i) {
-		ret = btrfs_lookup_csums_range(csum_root, logical,
-					       logical + map->stripe_len - 1,
-					       &sdev->csum_list, 1);
-		if (ret)
-			goto out;
 
-		logical += increment;
-		cond_resched();
-	}
 	/*
 	 * now find all extents for each stripe and scrub them
 	 */
-	logical = base + offset + start_stripe * increment;
-	physical = map->stripes[num].physical + start_stripe * map->stripe_len;
+	logical = base + offset;
+	physical = map->stripes[num].physical;
 	ret = 0;
-	for (i = start_stripe; i < nstripes; ++i) {
+	for (i = 0; i < nstripes; ++i) {
 		/*
 		 * canceled?
 		 */
@@ -882,11 +868,14 @@ again:
 			atomic_dec(&fs_info->scrubs_paused);
 			mutex_unlock(&fs_info->scrub_lock);
 			wake_up(&fs_info->scrub_pause_wait);
-			scrub_free_csums(sdev);
-			start_stripe = i;
-			goto again;
 		}
 
+		ret = btrfs_lookup_csums_range(csum_root, logical,
+					       logical + map->stripe_len - 1,
+					       &sdev->csum_list, 1);
+		if (ret)
+			goto out;
+
 		key.objectid = logical;
 		key.type = BTRFS_EXTENT_ITEM_KEY;
 		key.offset = (u64)0;
@@ -982,7 +971,6 @@ next:
 
 out:
 	blk_finish_plug(&plug);
-out_noplug:
 	btrfs_free_path(path);
 	return ret < 0 ? ret : 0;
 }
-- 
1.7.3.4

--
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

On 06/29/2011 04:10 PM, Arne Jansen wrote:
> read_extent_buffer_pages currently has two modes, either trigger a read
> without waiting for anything, or wait for the I/O to finish. The former
> also bails when it''s unable to lock the page. This patch now adds
an
> additional parameter to allow it to block on page lock, but don''t
wait
> for completion.
> 
> Signed-off-by: Arne Jansen <sensille@gmx.net>
> ---
>  fs/btrfs/disk-io.c   |    4 ++--
>  fs/btrfs/extent_io.c |    6 +++---
>  fs/btrfs/extent_io.h |    3 ++-
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 1ac8db5d..cff882c 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -325,7 +325,7 @@ static int btree_read_extent_buffer_pages(struct
btrfs_root *root,
>  	clear_bit(EXTENT_BUFFER_CORRUPT, &eb->bflags);
>  	io_tree = &BTRFS_I(root->fs_info->btree_inode)->io_tree;
>  	while (1) {
> -		ret = read_extent_buffer_pages(io_tree, eb, start, 1,
> +		ret = read_extent_buffer_pages(io_tree, eb, start, 1, 1,
>  					       btree_get_extent, mirror_num);
>  		if (!ret &&
>  		    !verify_parent_transid(io_tree, eb, parent_transid))
> @@ -940,7 +940,7 @@ int readahead_tree_block(struct btrfs_root *root, u64
bytenr, u32 blocksize,
>  	if (!buf)
>  		return 0;
>  	read_extent_buffer_pages(&BTRFS_I(btree_inode)->io_tree,
> -				 buf, 0, 0, btree_get_extent, 0);
> +				 buf, 0, 0, 0, btree_get_extent, 0);
>  	free_extent_buffer(buf);
>  	return ret;
>  }
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index b181a94..b7f0b9b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3358,7 +3358,7 @@ int extent_buffer_uptodate(struct extent_io_tree
*tree,
>  
>  int read_extent_buffer_pages(struct extent_io_tree *tree,
>  			     struct extent_buffer *eb,
> -			     u64 start, int wait,
> +			     u64 start, int wait_lock, int wait_complete,
>  			     get_extent_t *get_extent, int mirror_num)
Don''t do this, it makes it confusing to figure out which one is which.
Add a flags parameter at the end so we can specify stuff like

WAIT_COMPLETE
WAIT_PAGE_LOCK

which is easier to read.  Thanks,

Josef
--
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

On 06/29/2011 04:10 PM, Arne Jansen wrote:
> Add a READAHEAD extent buffer flag.
> Add a function to trigger a read with this flag set.
> 
> Changes v2:
>  - use extent buffer flags instead of extent state flags
> 
> Signed-off-by: Arne Jansen <sensille@gmx.net>
> ---
>  fs/btrfs/disk-io.c   |   32 ++++++++++++++++++++++++++++++++
>  fs/btrfs/disk-io.h   |    2 ++
>  fs/btrfs/extent_io.h |    1 +
>  3 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index cff882c..20edd2d 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -945,6 +945,38 @@ int readahead_tree_block(struct btrfs_root *root, u64
bytenr, u32 blocksize,
>  	return ret;
>  }
>  
> +int reada_tree_block_flagged(struct btrfs_root *root, u64 bytenr, u32
blocksize,
> +			 int mirror_num, struct extent_buffer **eb)
> +{
> +	struct extent_buffer *buf = NULL;
> +	struct inode *btree_inode = root->fs_info->btree_inode;
> +	struct extent_io_tree *io_tree = &BTRFS_I(btree_inode)->io_tree;
> +	int ret;
> +
> +	buf = btrfs_find_create_tree_block(root, bytenr, blocksize);
> +	if (!buf)
> +		return 0;
> +
> +	set_bit(EXTENT_BUFFER_READAHEAD, &buf->bflags);
> +
> +	ret = read_extent_buffer_pages(io_tree, buf, 0, 0, 1, btree_get_extent,
> +				 mirror_num);
> +	if (ret) {
> +		free_extent_buffer(buf);
> +		return ret;
> +	}
> +
> +	if (test_bit(EXTENT_BUFFER_CORRUPT, &buf->bflags)) {
> +		*eb = buf;
> +		return -EIO;
Why are you returning buf here when if you get an error from
read_extent_buffer_pages you free it?  Instead free the extent buffer
and return EIO to be consistent.  Thanks,

Josef
--
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

On 06/29/2011 04:10 PM, Arne Jansen wrote:
> Add state information for readahead to btrfs_fs_info and btrfs_device
> 
> Changes v2:
>  - don''t wait in radix_trees
>  - add own set of workers for readahead
> 
You can add

Reviewed-by: Josef Bacik <josef@redhat.com>

Thanks,

Josef
--
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

On 06/29/2011 04:10 PM, Arne Jansen wrote:
> This is the implementation for the generic read ahead framework.
> 
> To trigger a readahead, btrfs_reada_add must be called. It will start
> a read ahead for the given range [start, end) on tree root. The returned
> handle can either be used to wait on the readahead to finish
> (btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).
> 
> The read ahead works as follows:
> On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
> reada_start_machine will then search for extents to prefetch and trigger
> some reads. When a read finishes for a node, all contained node/leaf
> pointers that lie in the given range will also be enqueued. The reads will
> be triggered in sequential order, thus giving a big win over a naive
> enumeration. It will also make use of multi-device layouts. Each disk
> will have its on read pointer and all disks will by utilized in parallel.
> Also will no two disks read both sides of a mirror simultaneously, as this
> would waste seeking capacity. Instead both disks will read different parts
> of the filesystem.
> Any number of readaheads can be started in parallel. The read order will be
> determined globally, i.e. 2 parallel readaheads will normally finish faster
> than the 2 started one after another.
> 
> Changes v2:
>  - protect root->node by transaction instead of node_lock
>  - fix missed branches:
>     The readahead had a too simple check to determine if a branch from
>     a node should be checked or not. It now also records the upper bound
>     of each node to see if the requested RA range lies within.
>  - use KERN_CONT to debug output, to avoid line breaks
>  - defer reada_start_machine to worker to avoid deadlock
> 
> Changes v3:
>  - protect root->node by rcu
> 
> Signed-off-by: Arne Jansen <sensille@gmx.net>
> 
> use rcu to protect root node
> ---
>  fs/btrfs/Makefile |    3 +-
>  fs/btrfs/ctree.h  |    8 +
>  fs/btrfs/reada.c  |  994
+++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1004 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
> index 9b72dcf..58302ca 100644
> --- a/fs/btrfs/Makefile
> +++ b/fs/btrfs/Makefile
> @@ -7,4 +7,5 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o
root-tree.o dir-item.o \
>  	   extent_map.o sysfs.o struct-funcs.o xattr.o ordered-data.o \
>  	   extent_io.o volumes.o async-thread.o ioctl.o locking.o orphan.o \
>  	   export.o tree-log.o acl.o free-space-cache.o zlib.o lzo.o \
> -	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o
> +	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
> +	   reada.o
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 51534bf..78519df 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2681,4 +2681,12 @@ int btrfs_scrub_cancel_devid(struct btrfs_root
*root, u64 devid);
>  int btrfs_scrub_progress(struct btrfs_root *root, u64 devid,
>  			 struct btrfs_scrub_progress *progress);
>  
> +/* reada.c */
> +void *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);
> +
>  #endif
> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
> new file mode 100644
> index 0000000..e99c65c
> --- /dev/null
> +++ b/fs/btrfs/reada.c
> @@ -0,0 +1,994 @@
> +/*
> + * Copyright (C) 2011 STRATO.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the
> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> + * Boston, MA 021110-1307, USA.
> + */
> +
> +#include <linux/sched.h>
> +#include <linux/pagemap.h>
> +#include <linux/writeback.h>
> +#include <linux/blkdev.h>
> +#include <linux/rbtree.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include "ctree.h"
> +#include "volumes.h"
> +#include "disk-io.h"
> +#include "transaction.h"
> +
> +#undef DEBUG
> +
> +/*
> + * This is the implementation for the generic read ahead framework.
> + *
> + * To trigger a readahead, btrfs_reada_add must be called. It will start
> + * a read ahead for the given range [start, end) on tree root. The
returned
> + * handle can either be used to wait on the readahead to finish
> + * (btrfs_reada_wait), or to send it to the background
(btrfs_reada_detach).
> + *
> + * The read ahead works as follows:
> + * On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
> + * reada_start_machine will then search for extents to prefetch and
trigger
> + * some reads. When a read finishes for a node, all contained node/leaf
> + * pointers that lie in the given range will also be enqueued. The reads
will
> + * be triggered in sequential order, thus giving a big win over a naive
> + * enumeration. It will also make use of multi-device layouts. Each disk
> + * will have its on read pointer and all disks will by utilized in
parallel.
> + * Also will no two disks read both sides of a mirror simultaneously, as
this
> + * would waste seeking capacity. Instead both disks will read different
parts
> + * of the filesystem.
> + * Any number of readaheads can be started in parallel. The read order
will be
> + * determined globally, i.e. 2 parallel readaheads will normally finish
faster
> + * than the 2 started one after another.
> + */
> +
> +#define MAX_MIRRORS 2
> +#define MAX_IN_FLIGHT 6
> +
> +struct reada_control {
> +	struct btrfs_root	*root;		/* tree to prefetch */
> +	struct btrfs_key	key_start;
> +	struct btrfs_key	key_end;	/* exclusive */
> +	spinlock_t		lock;
> +	u64			elems;
So instead of using a spinlock to guard elems, just use an atomic_t,
since thats the only use the lock has in this case.
> +	u64			read_total;
Doesn''t seem like this is used for anything.
> +	struct kref		refcnt;
> +	wait_queue_head_t	wait;
> +};
> +
> +struct reada_extctl {
> +	struct list_head	list;
> +	struct reada_control	*rc;
> +	u64			generation;
> +};
> +
This is completely useless, kill this struct and just put the generation
and the list into reada_control.
> +struct reada_extent {
> +	u64			logical;
> +	struct btrfs_key	top;
This isn''t needed.
> +	u32			blocksize;
> +	int			err;
> +	struct list_head	extctl;
> +	struct kref		refcnt;
> +	spinlock_t		lock;
> +	struct reada_zone	*zones[MAX_MIRRORS];
> +	int			nzones;
> +	struct btrfs_device	*scheduled_for;
> +};
> +
> +struct reada_zone {
> +	u64			start;
> +	u64			end;
> +	u64			elems;
> +	struct list_head	list;
> +	spinlock_t		lock;
> +	int			locked;
> +	struct btrfs_device	*device;
> +	struct btrfs_device	*devs[MAX_MIRRORS]; /* full list, incl self */
> +	int			ndevs;
> +	struct kref		refcnt;
> +};
> +
> +struct reada_machine_work {
> +	struct btrfs_work	work;
> +	struct btrfs_fs_info	*fs_info;
> +};
> +
> +static void reada_extent_put(struct btrfs_fs_info *, struct reada_extent
*);
> +static void reada_zone_put(struct reada_zone *);
> +static void reada_control_put(struct reada_control *rc);
> +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_key_cmp(struct btrfs_key *a, struct btrfs_key *b)
> +{
> +	if (a->objectid > b->objectid)
> +		return 1;
> +	if (a->objectid < b->objectid)
> +		return -1;
> +	if (a->type > b->type)
> +		return 1;
> +	if (a->type < b->type)
> +		return -1;
> +	if (a->offset > b->offset)
> +		return 1;
> +	if (a->offset < b->offset)
> +		return -1;
> +	return 0;
> +}
> +
Use btrfs_comp_cpu_keys() here instead of adding yet another key
comparing function.
> +static int reada_add_block(struct reada_control *rc, u64 logical,
> +			   struct btrfs_key *top, int level, u64 generation);
> +
> +/* 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)
> +{
> +	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)
> +		kref_get(&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
> +	 * don''t need the lock anymore
> +	 */
> +	list_replace_init(&re->extctl, &list);
> +	for_dev = re->scheduled_for;
> +	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;
> +	}
> +
> +	for (i = 0; i < nritems; i++) {
> +		struct reada_extctl *rec;
> +		u64 n_gen;
> +		struct btrfs_key key;
> +		struct btrfs_key next_key;
> +
> +		btrfs_node_key_to_cpu(eb, &key, i);
> +		if (i + 1 < nritems)
> +			btrfs_node_key_to_cpu(eb, &next_key, i + 1);
> +		else
> +			next_key = re->top;
Either declare top locally (which I still don''t like) or have some
check
below for i == nritems - 1.
> +		bytenr = btrfs_node_blockptr(eb, i);
> +		n_gen = btrfs_node_ptr_generation(eb, i);
> +
> +		list_for_each_entry(rec, &list, list) {
> +			struct reada_control *rc = rec->rc;
> +
> +			/*
> +			 * if the generation doesn''t match, just ignore this
> +			 * extctl. This will probably cut off a branch from
> +			 * prefetch. Alternatively one could start a new (sub-)
> +			 * prefetch for this branch, starting again from root.
> +			 * FIXME: move the generation check out of this loop
> +			 */
> +#ifdef DEBUG
> +			if (rec->generation != generation) {
> +				printk(KERN_DEBUG "generation mismatch for "
> +						"(%llu,%d,%llu) %llu != %llu\n",
> +				       key.objectid, key.type, key.offset,
> +				       rec->generation, generation);
> +			}
> +#endif
> +			if (rec->generation == generation &&
> +			    reada_key_cmp(&key, &rc->key_end) < 0 &&
> +			    reada_key_cmp(&next_key, &rc->key_start) > 0)
> +				reada_add_block(rc, bytenr, &next_key,
> +						level - 1, n_gen);
> +		}
> +	}
> +	/*
> +	 * free extctl records
> +	 */
> +	while (!list_empty(&list)) {
> +		struct reada_control *rc;
> +		struct reada_extctl *rec;
> +
> +		rec = list_first_entry(&list, struct reada_extctl, list);
> +		list_del(&rec->list);
> +		rc = rec->rc;
> +		kfree(rec);
> +
> +		kref_get(&rc->refcnt);
> +		spin_lock(&rc->lock);
> +		--rc->elems;
> +		if (rc->elems == 0) {
> +			reada_control_put(rc);
> +			wake_up(&rc->wait);
> +		}
> +		spin_unlock(&rc->lock);
> +		reada_control_put(rc);
> +
> +		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;
> +}
> +
> +/*
> + * 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 ret;
> +
> +	ret = __readahead_hook(root, eb, start, err);
> +
> +	reada_start_machine(root->fs_info);
> +
> +	return ret;
> +}
> +
> +static struct reada_zone *reada_find_zone(struct btrfs_fs_info *fs_info,
> +					  struct btrfs_device *dev, u64 logical,
> +					  struct btrfs_multi_bio *multi)
> +{
> +	int ret;
> +	int looped = 0;
> +	struct reada_zone *zone;
> +	struct btrfs_block_group_cache *cache = NULL;
> +	u64 start;
> +	u64 end;
> +	int i;
> +
> +again:
> +	zone = NULL;
> +	spin_lock(&fs_info->reada_lock);
> +	ret = radix_tree_gang_lookup(&dev->reada_zones, (void
**)&zone,
> +				     logical >> PAGE_CACHE_SHIFT, 1);
> +	if (ret == 1)
> +		kref_get(&zone->refcnt);
> +	spin_unlock(&fs_info->reada_lock);
> +
> +	if (ret == 1) {
> +		if (logical >= zone->start && logical < zone->end)
> +			return zone;
> +		spin_lock(&fs_info->reada_lock);
> +		reada_zone_put(zone);
> +		spin_unlock(&fs_info->reada_lock);
> +	}
> +
> +	if (looped)
> +		return NULL;
> +
> +	cache = btrfs_lookup_block_group(fs_info, logical);
> +	if (!cache)
> +		return NULL;
> +
> +	start = cache->key.objectid;
> +	end = start + cache->key.offset - 1;
> +	btrfs_put_block_group(cache);
> +
> +	zone = kzalloc(sizeof(*zone), GFP_NOFS);
> +	if (!zone)
> +		return NULL;
> +
> +	zone->start = start;
> +	zone->end = end;
> +	INIT_LIST_HEAD(&zone->list);
> +	spin_lock_init(&zone->lock);
> +	zone->locked = 0;
> +	kref_init(&zone->refcnt);
> +	zone->elems = 0;
> +	zone->device = dev; /* our device always sits at index 0 */
> +	for (i = 0; i < multi->num_stripes; ++i) {
> +		/* bounds have already been checked */
> +		zone->devs[i] = multi->stripes[i].dev;
> +	}
> +	zone->ndevs = multi->num_stripes;
> +
> +	spin_lock(&fs_info->reada_lock);
> +	ret = radix_tree_insert(&dev->reada_zones,
> +				(unsigned long)zone->end >> PAGE_CACHE_SHIFT,
> +				zone);
> +	spin_unlock(&fs_info->reada_lock);
> +
> +	if (ret) {
> +		kfree(zone);
> +		looped = 1;
> +		goto again;
> +	}
> +
> +	return zone;
> +}
> +
> +static struct reada_extent *reada_find_extent(struct btrfs_root *root,
> +					      u64 logical,
> +					      struct btrfs_key *top, int level)
> +{
> +	int ret;
> +	int looped = 0;
> +	struct reada_extent *re = NULL;
> +	struct btrfs_fs_info *fs_info = root->fs_info;
> +	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
> +	struct btrfs_multi_bio *multi = NULL;
> +	struct btrfs_device *dev;
> +	u32 blocksize;
> +	u64 length;
> +	int nzones = 0;
> +	int i;
> +	unsigned long index = logical >> PAGE_CACHE_SHIFT;
> +
> +again:
> +	spin_lock(&fs_info->reada_lock);
> +	re = radix_tree_lookup(&fs_info->reada_tree, index);
> +	if (re)
> +		kref_get(&re->refcnt);
> +	spin_unlock(&fs_info->reada_lock);
> +
> +	if (re || looped)
> +		return re;
> +
> +	re = kzalloc(sizeof(*re), GFP_NOFS);
> +	if (!re)
> +		return NULL;
> +
> +	blocksize = btrfs_level_size(root, level);
> +	re->logical = logical;
> +	re->blocksize = blocksize;
> +	re->top = *top;
> +	INIT_LIST_HEAD(&re->extctl);
> +	spin_lock_init(&re->lock);
> +	kref_init(&re->refcnt);
> +
> +	/*
> +	 * map block
> +	 */
> +	length = blocksize;
> +	ret = btrfs_map_block(map_tree, REQ_WRITE, logical, &length,
&multi, 0);
> +	if (ret || !multi || length < blocksize)
> +		goto error;
> +
> +	if (multi->num_stripes > MAX_MIRRORS) {
> +		printk(KERN_ERR "btrfs readahead: more than %d copies not "
> +				"supported", MAX_MIRRORS);
> +		goto error;
> +	}
> +
> +	for (nzones = 0; nzones < multi->num_stripes; ++nzones) {
> +		struct reada_zone *zone;
> +
> +		dev = multi->stripes[nzones].dev;
> +		zone = reada_find_zone(fs_info, dev, logical, multi);
> +		if (!zone)
> +			break;
> +
> +		re->zones[nzones] = zone;
> +		spin_lock(&zone->lock);
> +		if (!zone->elems)
> +			kref_get(&zone->refcnt);
> +		++zone->elems;
> +		spin_unlock(&zone->lock);
> +		spin_lock(&fs_info->reada_lock);
> +		reada_zone_put(zone);
> +		spin_unlock(&fs_info->reada_lock);
> +	}
> +	re->nzones = nzones;
> +	if (nzones == 0) {
> +		/* not a single zone found, error and out */
> +		goto error;
> +	}
> +
> +	/* insert extent in reada_tree + all per-device trees, all or nothing */
> +	spin_lock(&fs_info->reada_lock);
> +	ret = radix_tree_insert(&fs_info->reada_tree, index, re);
> +	if (ret) {
> +		spin_unlock(&fs_info->reada_lock);
> +		if (ret != -ENOMEM) {
> +			/* someone inserted the extent in the meantime */
> +			looped = 1;
> +		}
> +		goto error;
> +	}
> +	for (i = 0; i < nzones; ++i) {
> +		dev = multi->stripes[i].dev;
> +		ret = radix_tree_insert(&dev->reada_extents, index, re);
> +		if (ret) {
> +			while (--i >= 0) {
> +				dev = multi->stripes[i].dev;
> +				BUG_ON(dev == NULL);
> +				radix_tree_delete(&dev->reada_extents, index);
> +			}
> +			BUG_ON(fs_info == NULL);
> +			radix_tree_delete(&fs_info->reada_tree, index);
> +			spin_unlock(&fs_info->reada_lock);
> +			goto error;
> +		}
> +	}
> +	spin_unlock(&fs_info->reada_lock);
> +
> +	return re;
> +
> +error:
> +	while (nzones) {
> +		struct reada_zone *zone;
> +
> +		--nzones;
> +		zone = re->zones[nzones];
> +		kref_get(&zone->refcnt);
> +		spin_lock(&zone->lock);
> +		--zone->elems;
> +		if (zone->elems == 0) {
> +			/*
> +			 * no fs_info->reada_lock needed, as this can''t be
> +			 * the last ref
> +			 */
> +			reada_zone_put(zone);
> +		}
> +		spin_unlock(&zone->lock);
> +
> +		spin_lock(&fs_info->reada_lock);
> +		reada_zone_put(zone);
> +		spin_unlock(&fs_info->reada_lock);
> +	}
> +	kfree(re);
> +	if (looped)
> +		goto again;
> +	return NULL;
> +}
> +
> +static void reada_kref_dummy(struct kref *kr)
> +{
> +}
> +
> +static void reada_extent_put(struct btrfs_fs_info *fs_info,
> +			     struct reada_extent *re)
> +{
> +	int i;
> +	unsigned long index = re->logical >> PAGE_CACHE_SHIFT;
> +
> +	spin_lock(&fs_info->reada_lock);
> +	if (!kref_put(&re->refcnt, reada_kref_dummy)) {
> +		spin_unlock(&fs_info->reada_lock);
> +		return;
> +	}
> +
> +	radix_tree_delete(&fs_info->reada_tree, index);
> +	for (i = 0; i < re->nzones; ++i) {
> +		struct reada_zone *zone = re->zones[i];
> +
> +		radix_tree_delete(&zone->device->reada_extents, index);
> +	}
> +
> +	spin_unlock(&fs_info->reada_lock);
> +
> +	for (i = 0; i < re->nzones; ++i) {
> +		struct reada_zone *zone = re->zones[i];
> +
> +		kref_get(&zone->refcnt);
> +		spin_lock(&zone->lock);
> +		--zone->elems;
> +		if (zone->elems == 0) {
> +			/* no fs_info->reada_lock needed, as this can''t be
> +			 * the last ref */
> +			reada_zone_put(zone);
> +		}
> +		spin_unlock(&zone->lock);
> +
> +		spin_lock(&fs_info->reada_lock);
> +		reada_zone_put(zone);
> +		spin_unlock(&fs_info->reada_lock);
> +	}
> +	if (re->scheduled_for)
> +		atomic_dec(&re->scheduled_for->reada_in_flight);
> +
> +	kfree(re);
> +}
> +
> +/* call it with fs_info->reada_lock held */
> +static void reada_zone_put(struct reada_zone *zone)
> +{
> +	if (!kref_put(&zone->refcnt, reada_kref_dummy))
> +		return;
> +
> +	radix_tree_delete(&zone->device->reada_zones,
> +			  zone->end >> PAGE_CACHE_SHIFT);
> +
Instead of making the callers take the reada_lock, move it into this
function so that in the fast case we''re not taking an extra spin_lock.
Also if you are going to use the kfref stuff you might as well use the
release function stuff.
> +	kfree(zone);
> +
> +	return;
> +}
> +
> +static void reada_control_put(struct reada_control *rc)
> +{
> +	if (kref_put(&rc->refcnt, reada_kref_dummy)) {
> +		kfree(rc);
> +		return;
Don''t need the return here.
> +	}
> +}
> +
> +static int reada_add_block(struct reada_control *rc, u64 logical,
> +			   struct btrfs_key *top, int level, 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 */
> +	if (!re)
> +		return -1;
> +
> +	rec = kzalloc(sizeof(*rec), GFP_NOFS);
> +	if (!rec) {
> +		reada_extent_put(root->fs_info, re);
> +		return -1;
> +	}
> +
> +	rec->rc = rc;
> +	rec->generation = generation;
> +	spin_lock(&rc->lock);
> +	++rc->elems;
> +	spin_unlock(&rc->lock);
> +
> +	spin_lock(&re->lock);
> +	list_add_tail(&rec->list, &re->extctl);
> +	spin_unlock(&re->lock);
> +
> +	/* leave the ref on the extent */
> +
> +	return 0;
> +}
> +
> +/*
> + * called with fs_info->reada_lock held
> + */
> +static void reada_peer_zones_set_lock(struct reada_zone *zone, int lock)
> +{
> +	int i;
> +	unsigned long index = zone->end >> PAGE_CACHE_SHIFT;
> +
> +	for (i = 0; i < zone->ndevs; ++i) {
> +		struct reada_zone *peer;
> +		peer = radix_tree_lookup(&zone->devs[i]->reada_zones, index);
> +		if (peer && peer->device != zone->device)
> +			peer->locked = lock;
> +	}
> +}
> +
> +/*
> + * called with fs_info->reada_lock held
> + */
> +static int reada_pick_zone(struct btrfs_device *dev)
> +{
> +	struct reada_zone *top_zone = NULL;
> +	struct reada_zone *top_locked_zone = NULL;
> +	u64 top_elems = 0;
> +	u64 top_locked_elems = 0;
> +	unsigned long index = 0;
> +	int ret;
> +
> +	if (dev->reada_curr_zone) {
> +		reada_peer_zones_set_lock(dev->reada_curr_zone, 0);
> +		reada_zone_put(dev->reada_curr_zone);
> +		dev->reada_curr_zone = NULL;
> +	}
> +	/* pick the zone with the most elements */
> +	while (1) {
> +		struct reada_zone *zone;
> +
> +		ret = radix_tree_gang_lookup(&dev->reada_zones,
> +					     (void **)&zone, index, 1);
> +		if (ret == 0)
> +			break;
> +		index = (zone->end >> PAGE_CACHE_SHIFT) + 1;
> +		if (zone->locked) {
> +			if (zone->elems > top_locked_elems) {
> +				top_locked_elems = zone->elems;
> +				top_locked_zone = zone;
> +			}
> +		} else {
> +			if (zone->elems > top_elems) {
> +				top_elems = zone->elems;
> +				top_zone = zone;
> +			}
> +		}
> +	}
> +	if (top_zone)
> +		dev->reada_curr_zone = top_zone;
> +	else if (top_locked_zone)
> +		dev->reada_curr_zone = top_locked_zone;
> +	else
> +		return 0;
> +
> +	dev->reada_next = dev->reada_curr_zone->start;
> +	kref_get(&dev->reada_curr_zone->refcnt);
> +	reada_peer_zones_set_lock(dev->reada_curr_zone, 1);
> +
> +	return 1;
> +}
> +
> +static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
> +				   struct btrfs_device *dev)
> +{
> +	struct reada_extent *re = NULL;
> +	int mirror_num = 0;
> +	struct extent_buffer *eb = NULL;
> +	u64 logical;
> +	u32 blocksize;
> +	int ret;
> +	int i;
> +	int need_kick = 0;
> +
> +	spin_lock(&fs_info->reada_lock);
> +	if (dev->reada_curr_zone == NULL) {
> +		ret = reada_pick_zone(dev);
> +		if (!ret) {
> +			spin_unlock(&fs_info->reada_lock);
> +			return 0;
> +		}
> +	}
> +	/*
> +	 * FIXME currently we issue the reads one extent at a time. If we have
> +	 * a contiguous block of extents, we could also coagulate them or use
> +	 * plugging to speed things up
> +	 */
> +	ret = radix_tree_gang_lookup(&dev->reada_extents, (void
**)&re,
> +				     dev->reada_next >> PAGE_CACHE_SHIFT, 1);
> +	if (ret == 0 || re->logical >= dev->reada_curr_zone->end) {
> +		ret = reada_pick_zone(dev);
> +		if (!ret) {
> +			spin_unlock(&fs_info->reada_lock);
> +			return 0;
> +		}
> +		re = NULL;
> +		ret = radix_tree_gang_lookup(&dev->reada_extents, (void
**)&re,
> +					dev->reada_next >> PAGE_CACHE_SHIFT, 1);
> +	}
> +	if (ret == 0) {
> +		spin_unlock(&fs_info->reada_lock);
> +		return 0;
> +	}
> +	dev->reada_next = re->logical + re->blocksize;
> +	kref_get(&re->refcnt);
> +
> +	spin_unlock(&fs_info->reada_lock);
> +
> +	/*
> +	 * find mirror num
> +	 */
> +	for (i = 0; i < re->nzones; ++i) {
> +		if (re->zones[i]->device == dev) {
> +			mirror_num = i + 1;
> +			break;
> +		}
> +	}
> +	logical = re->logical;
> +	blocksize = re->blocksize;
> +
> +	spin_lock(&re->lock);
> +	if (re->scheduled_for == NULL) {
> +		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,
blocksize,
> +			 mirror_num, &eb);
> +	if (eb) {
> +		__readahead_hook(fs_info->extent_root, eb, eb->start, ret);
> +		free_extent_buffer(eb);
> +	}
> +
> +	return 1;
> +
> +}
> +
> +static void reada_start_machine_worker(struct btrfs_work *work)
> +{
> +	struct reada_machine_work *rmw;
> +	struct btrfs_fs_info *fs_info;
> +
> +	rmw = container_of(work, struct reada_machine_work, work);
> +	fs_info = rmw->fs_info;
> +
> +	kfree(rmw);
> +
> +	__reada_start_machine(fs_info);
> +}
> +
> +static void __reada_start_machine(struct btrfs_fs_info *fs_info)
> +{
> +	struct btrfs_device *device;
> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> +	u64 enqueued;
> +	u64 total = 0;
> +	int i;
> +
> +	do {
> +		enqueued = 0;
> +		list_for_each_entry(device, &fs_devices->devices, dev_list) {
> +			if (atomic_read(&device->reada_in_flight) <
> +			    MAX_IN_FLIGHT)
> +				enqueued += reada_start_machine_dev(fs_info,
> +								    device);
> +		}
> +		total += enqueued;
> +	} while (enqueued && total < 10000);
> +
What is this?  Are we doing this so that the worker stays alive so it
can continue to process new requests coming in?  If thats the case we
need to have a proper kthread that doesn''t exit until we unmount or
something, not this weirdness.
> +	if (enqueued == 0)
> +		return;
> +
> +	/*
> +	 * If everything is already in the cache, this is effectively single
> +	 * threaded. To a) not hold the caller for too long and b) to utilize
> +	 * more cores, we broke the loop above after 10000 iterations and now
> +	 * enqueue to workers to finish it. This will distribute the load to
> +	 * the cores.
> +	 * FIXME we might need our own workqueue here, with an idle threshold
> +	 * of one. Also these worker are relatively long-running.
> +	 */
> +	for (i = 0; i < 2; ++i)
> +		reada_start_machine(fs_info);
> +}
> +
> +static void reada_start_machine(struct btrfs_fs_info *fs_info)
> +{
> +	struct reada_machine_work *rmw;
> +
> +	rmw = kzalloc(sizeof(*rmw), GFP_NOFS);
> +	if (!rmw) {
> +		/* FIXME we cannot handle this properly right now */
> +		BUG();
Yes you can, everywhere that calls this can handle failures, so make
this return an int and have it return -ENOMEM if it fails.
> +	}
> +	rmw->work.func = reada_start_machine_worker;
> +	rmw->fs_info = fs_info;
> +
> +	btrfs_queue_worker(&fs_info->readahead_workers,
&rmw->work);
> +}
> +
> +#ifdef DEBUG
> +static void dump_devs(struct btrfs_fs_info *fs_info, int all)
> +{
> +	struct btrfs_device *device;
> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> +	unsigned long index;
> +	int ret;
> +	int i;
> +	int j;
> +	int cnt;
> +
> +	spin_lock(&fs_info->reada_lock);
> +	list_for_each_entry(device, &fs_devices->devices, dev_list) {
> +		printk(KERN_DEBUG "dev %lld has %d in flight\n",
device->devid,
> +			atomic_read(&device->reada_in_flight));
> +		index = 0;
> +		while (1) {
> +			struct reada_zone *zone;
> +			ret = radix_tree_gang_lookup(&device->reada_zones,
> +						     (void **)&zone, index, 1);
> +			if (ret == 0)
> +				break;
> +			printk(KERN_DEBUG "  zone %llu-%llu elems %llu locked "
> +				"%d devs", zone->start, zone->end, zone->elems,
> +				zone->locked);
> +			for (j = 0; j < zone->ndevs; ++j) {
> +				printk(KERN_CONT " %lld",
> +					zone->devs[j]->devid);
> +			}
> +			if (device->reada_curr_zone == zone)
> +				printk(KERN_CONT " curr off %llu",
> +					device->reada_next - zone->start);
> +			printk(KERN_CONT "\n");
> +			index = (zone->end >> PAGE_CACHE_SHIFT) + 1;
> +		}
> +		cnt = 0;
> +		index = 0;
> +		while (all) {
> +			struct reada_extent *re = NULL;
> +
> +			ret = radix_tree_gang_lookup(&device->reada_extents,
> +						     (void **)&re, index, 1);
> +			if (ret == 0)
> +				break;
> +			printk(KERN_DEBUG
> +				"  re: logical %llu size %u empty %d for %lld",
> +				re->logical, re->blocksize,
> +				list_empty(&re->extctl), re->scheduled_for ?
> +				re->scheduled_for->devid : -1);
> +
> +			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);
> +				}
> +			}
> +			printk(KERN_CONT "\n");
> +			index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
> +			if (++cnt > 15)
> +				break;
> +		}
> +	}
> +
> +	index = 0;
> +	cnt = 0;
> +	while (all) {
> +		struct reada_extent *re = NULL;
> +
> +		ret = radix_tree_gang_lookup(&fs_info->reada_tree, (void
**)&re,
> +					     index, 1);
> +		if (ret == 0)
> +			break;
> +		if (!re->scheduled_for) {
> +			index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
> +			continue;
> +		}
> +		printk(KERN_DEBUG
> +			"re: logical %llu size %u list empty %d for %lld",
> +			re->logical, re->blocksize, list_empty(&re->extctl),
> +			re->scheduled_for ? re->scheduled_for->devid : -1);
> +		for (i = 0; i < re->nzones; ++i) {
> +			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);
> +				}
> +			}
> +		}
> +		printk(KERN_CONT "\n");
> +		index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
> +	}
> +	spin_unlock(&fs_info->reada_lock);
> +}
> +#endif
> +
> +/*
> + * interface
> + */
> +void *btrfs_reada_add(struct btrfs_root *root, struct btrfs_key
*key_start,
> +		      struct btrfs_key *key_end)
> +{
Don''t do this.  Add struct reada_control to a header file so all the
typing gets checked properly by the compiler and we don''t end up with
weirdness.
> +	struct reada_control *rc;
> +	u64 start;
> +	u64 generation;
> +	int level;
> +	struct extent_buffer *node;
> +	static struct btrfs_key max_key = {
> +		.objectid = (u64)-1,
> +		.type = (u8)-1,
> +		.offset = (u64)-1
> +	};
> +
This isn''t needed at all, kill it.
> +	rc = kzalloc(sizeof(*rc), GFP_NOFS);
> +	if (!rc)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rc->root = root;
> +	rc->key_start = *key_start;
> +	rc->key_end = *key_end;
> +	spin_lock_init(&rc->lock);
> +	rc->elems = 0;
> +	rc->read_total = 0;
> +	init_waitqueue_head(&rc->wait);
> +	kref_init(&rc->refcnt);
> +	kref_get(&rc->refcnt); /* one ref for having elements */
> +
> +	node = btrfs_root_node(root);
> +	start = node->start;
> +	level = btrfs_header_level(node);
> +	generation = btrfs_header_generation(node);
> +	free_extent_buffer(node);
> +
> +	reada_add_block(rc, start, &max_key, level, generation);
> +
> +	reada_start_machine(root->fs_info);
> +
> +	return rc;
> +}
> +
> +#ifdef DEBUG
> +int btrfs_reada_wait(void *handle)
> +{
> +	struct reada_control *rc = handle;
> +
> +	spin_lock(&rc->lock);
> +	while (rc->elems) {
> +		spin_unlock(&rc->lock);
> +		wait_event_timeout(rc->wait, rc->elems == 0, 5 * HZ);
> +		dump_devs(rc->root->fs_info, rc->elems < 10 ? 1 : 0);
> +		spin_lock(&rc->lock);
> +	}
> +	spin_unlock(&rc->lock);
> +
> +	dump_devs(rc->root->fs_info, rc->elems < 10 ? 1 : 0);
> +
> +	reada_control_put(rc);
> +
> +	return 0;
> +}
> +#else
> +int btrfs_reada_wait(void *handle)
> +{
> +	struct reada_control *rc = handle;
> +
> +	spin_lock(&rc->lock);
> +	while (rc->elems) {
> +		spin_unlock(&rc->lock);
> +		wait_event(rc->wait, rc->elems == 0);
> +		spin_lock(&rc->lock);
> +	}
> +	spin_unlock(&rc->lock);
> +
> +	reada_control_put(rc);
> +
> +	return 0;
> +}
> +#endif
> +
> +void btrfs_reada_detach(void *handle)
> +{
> +	struct reada_control *rc = handle;
> +
> +	reada_control_put(rc);
> +}
Thats all I have time for on this one, but that should be enough to keep
you busy for a bit :).  I will look at it more tomorrow.  Thanks,

Josef
--
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

On 29.06.2011 23:49, Josef Bacik wrote:
> On 06/29/2011 04:10 PM, Arne Jansen wrote:
>> This is the implementation for the generic read ahead framework.
>>
>> To trigger a readahead, btrfs_reada_add must be called. It will start
>> a read ahead for the given range [start, end) on tree root. The
returned
>> handle can either be used to wait on the readahead to finish
>> (btrfs_reada_wait), or to send it to the background
(btrfs_reada_detach).
>>
>> The read ahead works as follows:
>> On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
>> reada_start_machine will then search for extents to prefetch and
trigger
>> some reads. When a read finishes for a node, all contained node/leaf
>> pointers that lie in the given range will also be enqueued. The reads
will
>> be triggered in sequential order, thus giving a big win over a naive
>> enumeration. It will also make use of multi-device layouts. Each disk
>> will have its on read pointer and all disks will by utilized in
parallel.
>> Also will no two disks read both sides of a mirror simultaneously, as
this
>> would waste seeking capacity. Instead both disks will read different
parts
>> of the filesystem.
>> Any number of readaheads can be started in parallel. The read order
will be
>> determined globally, i.e. 2 parallel readaheads will normally finish
faster
>> than the 2 started one after another.
>>
>> Changes v2:
>>  - protect root->node by transaction instead of node_lock
>>  - fix missed branches:
>>     The readahead had a too simple check to determine if a branch from
>>     a node should be checked or not. It now also records the upper
bound
>>     of each node to see if the requested RA range lies within.
>>  - use KERN_CONT to debug output, to avoid line breaks
>>  - defer reada_start_machine to worker to avoid deadlock
>>
>> Changes v3:
>>  - protect root->node by rcu
>>
>> Signed-off-by: Arne Jansen <sensille@gmx.net>
>>
>> use rcu to protect root node
>> ---
>>  fs/btrfs/Makefile |    3 +-
>>  fs/btrfs/ctree.h  |    8 +
>>  fs/btrfs/reada.c  |  994
+++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 1004 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
>> index 9b72dcf..58302ca 100644
>> --- a/fs/btrfs/Makefile
>> +++ b/fs/btrfs/Makefile
>> @@ -7,4 +7,5 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o
root-tree.o dir-item.o \
>>  	   extent_map.o sysfs.o struct-funcs.o xattr.o ordered-data.o \
>>  	   extent_io.o volumes.o async-thread.o ioctl.o locking.o orphan.o \
>>  	   export.o tree-log.o acl.o free-space-cache.o zlib.o lzo.o \
>> -	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o
>> +	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
>> +	   reada.o
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 51534bf..78519df 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -2681,4 +2681,12 @@ int btrfs_scrub_cancel_devid(struct btrfs_root
*root, u64 devid);
>>  int btrfs_scrub_progress(struct btrfs_root *root, u64 devid,
>>  			 struct btrfs_scrub_progress *progress);
>>  
>> +/* reada.c */
>> +void *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);
>> +
>>  #endif
>> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
>> new file mode 100644
>> index 0000000..e99c65c
>> --- /dev/null
>> +++ b/fs/btrfs/reada.c
>> @@ -0,0 +1,994 @@
>> +/*
>> + * Copyright (C) 2011 STRATO.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this program; if not, write to the
>> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>> + * Boston, MA 021110-1307, USA.
>> + */
>> +
>> +#include <linux/sched.h>
>> +#include <linux/pagemap.h>
>> +#include <linux/writeback.h>
>> +#include <linux/blkdev.h>
>> +#include <linux/rbtree.h>
>> +#include <linux/slab.h>
>> +#include <linux/workqueue.h>
>> +#include "ctree.h"
>> +#include "volumes.h"
>> +#include "disk-io.h"
>> +#include "transaction.h"
>> +
>> +#undef DEBUG
>> +
>> +/*
>> + * This is the implementation for the generic read ahead framework.
>> + *
>> + * To trigger a readahead, btrfs_reada_add must be called. It will
start
>> + * a read ahead for the given range [start, end) on tree root. The
returned
>> + * handle can either be used to wait on the readahead to finish
>> + * (btrfs_reada_wait), or to send it to the background
(btrfs_reada_detach).
>> + *
>> + * The read ahead works as follows:
>> + * On btrfs_reada_add, the root of the tree is inserted into a
radix_tree.
>> + * reada_start_machine will then search for extents to prefetch and
trigger
>> + * some reads. When a read finishes for a node, all contained
node/leaf
>> + * pointers that lie in the given range will also be enqueued. The
reads will
>> + * be triggered in sequential order, thus giving a big win over a
naive
>> + * enumeration. It will also make use of multi-device layouts. Each
disk
>> + * will have its on read pointer and all disks will by utilized in
parallel.
>> + * Also will no two disks read both sides of a mirror simultaneously,
as this
>> + * would waste seeking capacity. Instead both disks will read
different parts
>> + * of the filesystem.
>> + * Any number of readaheads can be started in parallel. The read order
will be
>> + * determined globally, i.e. 2 parallel readaheads will normally
finish faster
>> + * than the 2 started one after another.
>> + */
>> +
>> +#define MAX_MIRRORS 2
>> +#define MAX_IN_FLIGHT 6
>> +
>> +struct reada_control {
>> +	struct btrfs_root	*root;		/* tree to prefetch */
>> +	struct btrfs_key	key_start;
>> +	struct btrfs_key	key_end;	/* exclusive */
>> +	spinlock_t		lock;
>> +	u64			elems;
> 
> So instead of using a spinlock to guard elems, just use an atomic_t,
> since thats the only use the lock has in this case.
ok. My initial fear was that an atomic_t is not big enough, but it
seems I can rely on it being at least 32 bits, and readaheads this
large are stupid :)
> 
>> +	u64			read_total;
> 
> Doesn''t seem like this is used for anything.
ok.
> 
>> +	struct kref		refcnt;
>> +	wait_queue_head_t	wait;
>> +};
>> +
>> +struct reada_extctl {
>> +	struct list_head	list;
>> +	struct reada_control	*rc;
>> +	u64			generation;
>> +};
>> +
> 
> This is completely useless, kill this struct and just put the generation
> and the list into reada_control.
This struct is the link between reada_extent and reada_control.
In case more than one readahead is running, each reada_extent
can point to multiple reada_controls, so I need this link struct.
> 
>> +struct reada_extent {
>> +	u64			logical;
>> +	struct btrfs_key	top;
> 
> This isn''t needed.
Thinking 3 times more about it you might be right. The intent is
to determine the range of the last key in a node, to see if any
of the subscribed readaheads need to follow it. But for this purpose
it might be sufficient to assume (-1,-1,-1) as top.
Later on, I''d like to add chaining code to readahead, to trigger
more readaheads in other tree from the content of a leaf. In this
case, the range information for the key might be useful.
I can throw this out now and re-add it in case it is needed.
> 
>> +	u32			blocksize;
>> +	int			err;
>> +	struct list_head	extctl;
>> +	struct kref		refcnt;
>> +	spinlock_t		lock;
>> +	struct reada_zone	*zones[MAX_MIRRORS];
>> +	int			nzones;
>> +	struct btrfs_device	*scheduled_for;
>> +};
>> +
>> +struct reada_zone {
>> +	u64			start;
>> +	u64			end;
>> +	u64			elems;
>> +	struct list_head	list;
>> +	spinlock_t		lock;
>> +	int			locked;
>> +	struct btrfs_device	*device;
>> +	struct btrfs_device	*devs[MAX_MIRRORS]; /* full list, incl self */
>> +	int			ndevs;
>> +	struct kref		refcnt;
>> +};
>> +
>> +struct reada_machine_work {
>> +	struct btrfs_work	work;
>> +	struct btrfs_fs_info	*fs_info;
>> +};
>> +
>> +static void reada_extent_put(struct btrfs_fs_info *, struct
reada_extent *);
>> +static void reada_zone_put(struct reada_zone *);
>> +static void reada_control_put(struct reada_control *rc);
>> +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_key_cmp(struct btrfs_key *a, struct btrfs_key *b)
>> +{
>> +	if (a->objectid > b->objectid)
>> +		return 1;
>> +	if (a->objectid < b->objectid)
>> +		return -1;
>> +	if (a->type > b->type)
>> +		return 1;
>> +	if (a->type < b->type)
>> +		return -1;
>> +	if (a->offset > b->offset)
>> +		return 1;
>> +	if (a->offset < b->offset)
>> +		return -1;
>> +	return 0;
>> +}
>> +
> 
> Use btrfs_comp_cpu_keys() here instead of adding yet another key
> comparing function.
> 
>> +static int reada_add_block(struct reada_control *rc, u64 logical,
>> +			   struct btrfs_key *top, int level, u64 generation);
>> +
>> +/* 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)
>> +{
>> +	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)
>> +		kref_get(&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
>> +	 * don''t need the lock anymore
>> +	 */
>> +	list_replace_init(&re->extctl, &list);
>> +	for_dev = re->scheduled_for;
>> +	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;
>> +	}
>> +
>> +	for (i = 0; i < nritems; i++) {
>> +		struct reada_extctl *rec;
>> +		u64 n_gen;
>> +		struct btrfs_key key;
>> +		struct btrfs_key next_key;
>> +
>> +		btrfs_node_key_to_cpu(eb, &key, i);
>> +		if (i + 1 < nritems)
>> +			btrfs_node_key_to_cpu(eb, &next_key, i + 1);
>> +		else
>> +			next_key = re->top;
> 
> Either declare top locally (which I still don''t like) or have some
check
> below for i == nritems - 1.
I''d set next_key to (-1, -1, -1) instead.
> 
>> +		bytenr = btrfs_node_blockptr(eb, i);
>> +		n_gen = btrfs_node_ptr_generation(eb, i);
>> +
>> +		list_for_each_entry(rec, &list, list) {
>> +			struct reada_control *rc = rec->rc;
>> +
>> +			/*
>> +			 * if the generation doesn''t match, just ignore this
>> +			 * extctl. This will probably cut off a branch from
>> +			 * prefetch. Alternatively one could start a new (sub-)
>> +			 * prefetch for this branch, starting again from root.
>> +			 * FIXME: move the generation check out of this loop
>> +			 */
>> +#ifdef DEBUG
>> +			if (rec->generation != generation) {
>> +				printk(KERN_DEBUG "generation mismatch for "
>> +						"(%llu,%d,%llu) %llu != %llu\n",
>> +				       key.objectid, key.type, key.offset,
>> +				       rec->generation, generation);
>> +			}
>> +#endif
>> +			if (rec->generation == generation &&
>> +			    reada_key_cmp(&key, &rc->key_end) < 0 &&
>> +			    reada_key_cmp(&next_key, &rc->key_start) > 0)
>> +				reada_add_block(rc, bytenr, &next_key,
>> +						level - 1, n_gen);
>> +		}
>> +	}
>> +	/*
>> +	 * free extctl records
>> +	 */
>> +	while (!list_empty(&list)) {
>> +		struct reada_control *rc;
>> +		struct reada_extctl *rec;
>> +
>> +		rec = list_first_entry(&list, struct reada_extctl, list);
>> +		list_del(&rec->list);
>> +		rc = rec->rc;
>> +		kfree(rec);
>> +
>> +		kref_get(&rc->refcnt);
>> +		spin_lock(&rc->lock);
>> +		--rc->elems;
>> +		if (rc->elems == 0) {
>> +			reada_control_put(rc);
>> +			wake_up(&rc->wait);
>> +		}
>> +		spin_unlock(&rc->lock);
>> +		reada_control_put(rc);
>> +
>> +		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;
>> +}
>> +
>> +/*
>> + * 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 ret;
>> +
>> +	ret = __readahead_hook(root, eb, start, err);
>> +
>> +	reada_start_machine(root->fs_info);
>> +
>> +	return ret;
>> +}
>> +
>> +static struct reada_zone *reada_find_zone(struct btrfs_fs_info
*fs_info,
>> +					  struct btrfs_device *dev, u64 logical,
>> +					  struct btrfs_multi_bio *multi)
>> +{
>> +	int ret;
>> +	int looped = 0;
>> +	struct reada_zone *zone;
>> +	struct btrfs_block_group_cache *cache = NULL;
>> +	u64 start;
>> +	u64 end;
>> +	int i;
>> +
>> +again:
>> +	zone = NULL;
>> +	spin_lock(&fs_info->reada_lock);
>> +	ret = radix_tree_gang_lookup(&dev->reada_zones, (void
**)&zone,
>> +				     logical >> PAGE_CACHE_SHIFT, 1);
>> +	if (ret == 1)
>> +		kref_get(&zone->refcnt);
>> +	spin_unlock(&fs_info->reada_lock);
>> +
>> +	if (ret == 1) {
>> +		if (logical >= zone->start && logical <
zone->end)
>> +			return zone;
>> +		spin_lock(&fs_info->reada_lock);
>> +		reada_zone_put(zone);
>> +		spin_unlock(&fs_info->reada_lock);
>> +	}
>> +
>> +	if (looped)
>> +		return NULL;
>> +
>> +	cache = btrfs_lookup_block_group(fs_info, logical);
>> +	if (!cache)
>> +		return NULL;
>> +
>> +	start = cache->key.objectid;
>> +	end = start + cache->key.offset - 1;
>> +	btrfs_put_block_group(cache);
>> +
>> +	zone = kzalloc(sizeof(*zone), GFP_NOFS);
>> +	if (!zone)
>> +		return NULL;
>> +
>> +	zone->start = start;
>> +	zone->end = end;
>> +	INIT_LIST_HEAD(&zone->list);
>> +	spin_lock_init(&zone->lock);
>> +	zone->locked = 0;
>> +	kref_init(&zone->refcnt);
>> +	zone->elems = 0;
>> +	zone->device = dev; /* our device always sits at index 0 */
>> +	for (i = 0; i < multi->num_stripes; ++i) {
>> +		/* bounds have already been checked */
>> +		zone->devs[i] = multi->stripes[i].dev;
>> +	}
>> +	zone->ndevs = multi->num_stripes;
>> +
>> +	spin_lock(&fs_info->reada_lock);
>> +	ret = radix_tree_insert(&dev->reada_zones,
>> +				(unsigned long)zone->end >> PAGE_CACHE_SHIFT,
>> +				zone);
>> +	spin_unlock(&fs_info->reada_lock);
>> +
>> +	if (ret) {
>> +		kfree(zone);
>> +		looped = 1;
>> +		goto again;
>> +	}
>> +
>> +	return zone;
>> +}
>> +
>> +static struct reada_extent *reada_find_extent(struct btrfs_root *root,
>> +					      u64 logical,
>> +					      struct btrfs_key *top, int level)
>> +{
>> +	int ret;
>> +	int looped = 0;
>> +	struct reada_extent *re = NULL;
>> +	struct btrfs_fs_info *fs_info = root->fs_info;
>> +	struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
>> +	struct btrfs_multi_bio *multi = NULL;
>> +	struct btrfs_device *dev;
>> +	u32 blocksize;
>> +	u64 length;
>> +	int nzones = 0;
>> +	int i;
>> +	unsigned long index = logical >> PAGE_CACHE_SHIFT;
>> +
>> +again:
>> +	spin_lock(&fs_info->reada_lock);
>> +	re = radix_tree_lookup(&fs_info->reada_tree, index);
>> +	if (re)
>> +		kref_get(&re->refcnt);
>> +	spin_unlock(&fs_info->reada_lock);
>> +
>> +	if (re || looped)
>> +		return re;
>> +
>> +	re = kzalloc(sizeof(*re), GFP_NOFS);
>> +	if (!re)
>> +		return NULL;
>> +
>> +	blocksize = btrfs_level_size(root, level);
>> +	re->logical = logical;
>> +	re->blocksize = blocksize;
>> +	re->top = *top;
>> +	INIT_LIST_HEAD(&re->extctl);
>> +	spin_lock_init(&re->lock);
>> +	kref_init(&re->refcnt);
>> +
>> +	/*
>> +	 * map block
>> +	 */
>> +	length = blocksize;
>> +	ret = btrfs_map_block(map_tree, REQ_WRITE, logical, &length,
&multi, 0);
>> +	if (ret || !multi || length < blocksize)
>> +		goto error;
>> +
>> +	if (multi->num_stripes > MAX_MIRRORS) {
>> +		printk(KERN_ERR "btrfs readahead: more than %d copies not
"
>> +				"supported", MAX_MIRRORS);
>> +		goto error;
>> +	}
>> +
>> +	for (nzones = 0; nzones < multi->num_stripes; ++nzones) {
>> +		struct reada_zone *zone;
>> +
>> +		dev = multi->stripes[nzones].dev;
>> +		zone = reada_find_zone(fs_info, dev, logical, multi);
>> +		if (!zone)
>> +			break;
>> +
>> +		re->zones[nzones] = zone;
>> +		spin_lock(&zone->lock);
>> +		if (!zone->elems)
>> +			kref_get(&zone->refcnt);
>> +		++zone->elems;
>> +		spin_unlock(&zone->lock);
>> +		spin_lock(&fs_info->reada_lock);
>> +		reada_zone_put(zone);
>> +		spin_unlock(&fs_info->reada_lock);
>> +	}
>> +	re->nzones = nzones;
>> +	if (nzones == 0) {
>> +		/* not a single zone found, error and out */
>> +		goto error;
>> +	}
>> +
>> +	/* insert extent in reada_tree + all per-device trees, all or nothing
*/
>> +	spin_lock(&fs_info->reada_lock);
>> +	ret = radix_tree_insert(&fs_info->reada_tree, index, re);
>> +	if (ret) {
>> +		spin_unlock(&fs_info->reada_lock);
>> +		if (ret != -ENOMEM) {
>> +			/* someone inserted the extent in the meantime */
>> +			looped = 1;
>> +		}
>> +		goto error;
>> +	}
>> +	for (i = 0; i < nzones; ++i) {
>> +		dev = multi->stripes[i].dev;
>> +		ret = radix_tree_insert(&dev->reada_extents, index, re);
>> +		if (ret) {
>> +			while (--i >= 0) {
>> +				dev = multi->stripes[i].dev;
>> +				BUG_ON(dev == NULL);
>> +				radix_tree_delete(&dev->reada_extents, index);
>> +			}
>> +			BUG_ON(fs_info == NULL);
>> +			radix_tree_delete(&fs_info->reada_tree, index);
>> +			spin_unlock(&fs_info->reada_lock);
>> +			goto error;
>> +		}
>> +	}
>> +	spin_unlock(&fs_info->reada_lock);
>> +
>> +	return re;
>> +
>> +error:
>> +	while (nzones) {
>> +		struct reada_zone *zone;
>> +
>> +		--nzones;
>> +		zone = re->zones[nzones];
>> +		kref_get(&zone->refcnt);
>> +		spin_lock(&zone->lock);
>> +		--zone->elems;
>> +		if (zone->elems == 0) {
>> +			/*
>> +			 * no fs_info->reada_lock needed, as this can''t be
>> +			 * the last ref
>> +			 */
>> +			reada_zone_put(zone);
>> +		}
>> +		spin_unlock(&zone->lock);
>> +
>> +		spin_lock(&fs_info->reada_lock);
>> +		reada_zone_put(zone);
>> +		spin_unlock(&fs_info->reada_lock);
>> +	}
>> +	kfree(re);
>> +	if (looped)
>> +		goto again;
>> +	return NULL;
>> +}
>> +
>> +static void reada_kref_dummy(struct kref *kr)
>> +{
>> +}
>> +
>> +static void reada_extent_put(struct btrfs_fs_info *fs_info,
>> +			     struct reada_extent *re)
>> +{
>> +	int i;
>> +	unsigned long index = re->logical >> PAGE_CACHE_SHIFT;
>> +
>> +	spin_lock(&fs_info->reada_lock);
>> +	if (!kref_put(&re->refcnt, reada_kref_dummy)) {
>> +		spin_unlock(&fs_info->reada_lock);
>> +		return;
>> +	}
>> +
>> +	radix_tree_delete(&fs_info->reada_tree, index);
>> +	for (i = 0; i < re->nzones; ++i) {
>> +		struct reada_zone *zone = re->zones[i];
>> +
>> +		radix_tree_delete(&zone->device->reada_extents, index);
>> +	}
>> +
>> +	spin_unlock(&fs_info->reada_lock);
>> +
>> +	for (i = 0; i < re->nzones; ++i) {
>> +		struct reada_zone *zone = re->zones[i];
>> +
>> +		kref_get(&zone->refcnt);
>> +		spin_lock(&zone->lock);
>> +		--zone->elems;
>> +		if (zone->elems == 0) {
>> +			/* no fs_info->reada_lock needed, as this can''t be
>> +			 * the last ref */
>> +			reada_zone_put(zone);
>> +		}
>> +		spin_unlock(&zone->lock);
>> +
>> +		spin_lock(&fs_info->reada_lock);
>> +		reada_zone_put(zone);
>> +		spin_unlock(&fs_info->reada_lock);
>> +	}
>> +	if (re->scheduled_for)
>> +		atomic_dec(&re->scheduled_for->reada_in_flight);
>> +
>> +	kfree(re);
>> +}
>> +
>> +/* call it with fs_info->reada_lock held */
>> +static void reada_zone_put(struct reada_zone *zone)
>> +{
>> +	if (!kref_put(&zone->refcnt, reada_kref_dummy))
>> +		return;
>> +
>> +	radix_tree_delete(&zone->device->reada_zones,
>> +			  zone->end >> PAGE_CACHE_SHIFT);
>> +
> 
> Instead of making the callers take the reada_lock, move it into this
> function so that in the fast case we''re not taking an extra
spin_lock.
I had to move this out, mainly because reada_start_machine_dev needs to
hold the lock for several operation, one of which might call zone_put.
Maybe I can defer the zone_put until afterwards.
I''d like to wait with these kind of optimizations until there are some
uses of readahead in more time critical paths, or at least until we have
settled there will be such uses ;)
> Also if you are going to use the kfref stuff you might as well use the
> release function stuff.
Right, in the case I can. I didn''t do it for symmetrie with the other
cases, but changed it now.
> 
>> +	kfree(zone);
>> +
>> +	return;
>> +}
>> +
>> +static void reada_control_put(struct reada_control *rc)
>> +{
>> +	if (kref_put(&rc->refcnt, reada_kref_dummy)) {
>> +		kfree(rc);
>> +		return;
> 
> Don''t need the return here.
I killed the whole function and built reada_control_release instead.
> 
>> +	}
>> +}
>> +
>> +static int reada_add_block(struct reada_control *rc, u64 logical,
>> +			   struct btrfs_key *top, int level, 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
*/
>> +	if (!re)
>> +		return -1;
>> +
>> +	rec = kzalloc(sizeof(*rec), GFP_NOFS);
>> +	if (!rec) {
>> +		reada_extent_put(root->fs_info, re);
>> +		return -1;
>> +	}
>> +
>> +	rec->rc = rc;
>> +	rec->generation = generation;
>> +	spin_lock(&rc->lock);
>> +	++rc->elems;
>> +	spin_unlock(&rc->lock);
>> +
>> +	spin_lock(&re->lock);
>> +	list_add_tail(&rec->list, &re->extctl);
>> +	spin_unlock(&re->lock);
>> +
>> +	/* leave the ref on the extent */
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * called with fs_info->reada_lock held
>> + */
>> +static void reada_peer_zones_set_lock(struct reada_zone *zone, int
lock)
>> +{
>> +	int i;
>> +	unsigned long index = zone->end >> PAGE_CACHE_SHIFT;
>> +
>> +	for (i = 0; i < zone->ndevs; ++i) {
>> +		struct reada_zone *peer;
>> +		peer = radix_tree_lookup(&zone->devs[i]->reada_zones,
index);
>> +		if (peer && peer->device != zone->device)
>> +			peer->locked = lock;
>> +	}
>> +}
>> +
>> +/*
>> + * called with fs_info->reada_lock held
>> + */
>> +static int reada_pick_zone(struct btrfs_device *dev)
>> +{
>> +	struct reada_zone *top_zone = NULL;
>> +	struct reada_zone *top_locked_zone = NULL;
>> +	u64 top_elems = 0;
>> +	u64 top_locked_elems = 0;
>> +	unsigned long index = 0;
>> +	int ret;
>> +
>> +	if (dev->reada_curr_zone) {
>> +		reada_peer_zones_set_lock(dev->reada_curr_zone, 0);
>> +		reada_zone_put(dev->reada_curr_zone);
>> +		dev->reada_curr_zone = NULL;
>> +	}
>> +	/* pick the zone with the most elements */
>> +	while (1) {
>> +		struct reada_zone *zone;
>> +
>> +		ret = radix_tree_gang_lookup(&dev->reada_zones,
>> +					     (void **)&zone, index, 1);
>> +		if (ret == 0)
>> +			break;
>> +		index = (zone->end >> PAGE_CACHE_SHIFT) + 1;
>> +		if (zone->locked) {
>> +			if (zone->elems > top_locked_elems) {
>> +				top_locked_elems = zone->elems;
>> +				top_locked_zone = zone;
>> +			}
>> +		} else {
>> +			if (zone->elems > top_elems) {
>> +				top_elems = zone->elems;
>> +				top_zone = zone;
>> +			}
>> +		}
>> +	}
>> +	if (top_zone)
>> +		dev->reada_curr_zone = top_zone;
>> +	else if (top_locked_zone)
>> +		dev->reada_curr_zone = top_locked_zone;
>> +	else
>> +		return 0;
>> +
>> +	dev->reada_next = dev->reada_curr_zone->start;
>> +	kref_get(&dev->reada_curr_zone->refcnt);
>> +	reada_peer_zones_set_lock(dev->reada_curr_zone, 1);
>> +
>> +	return 1;
>> +}
>> +
>> +static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
>> +				   struct btrfs_device *dev)
>> +{
>> +	struct reada_extent *re = NULL;
>> +	int mirror_num = 0;
>> +	struct extent_buffer *eb = NULL;
>> +	u64 logical;
>> +	u32 blocksize;
>> +	int ret;
>> +	int i;
>> +	int need_kick = 0;
>> +
>> +	spin_lock(&fs_info->reada_lock);
>> +	if (dev->reada_curr_zone == NULL) {
>> +		ret = reada_pick_zone(dev);
>> +		if (!ret) {
>> +			spin_unlock(&fs_info->reada_lock);
>> +			return 0;
>> +		}
>> +	}
>> +	/*
>> +	 * FIXME currently we issue the reads one extent at a time. If we
have
>> +	 * a contiguous block of extents, we could also coagulate them or use
>> +	 * plugging to speed things up
>> +	 */
>> +	ret = radix_tree_gang_lookup(&dev->reada_extents, (void
**)&re,
>> +				     dev->reada_next >> PAGE_CACHE_SHIFT, 1);
>> +	if (ret == 0 || re->logical >= dev->reada_curr_zone->end)
{
>> +		ret = reada_pick_zone(dev);
>> +		if (!ret) {
>> +			spin_unlock(&fs_info->reada_lock);
>> +			return 0;
>> +		}
>> +		re = NULL;
>> +		ret = radix_tree_gang_lookup(&dev->reada_extents, (void
**)&re,
>> +					dev->reada_next >> PAGE_CACHE_SHIFT, 1);
>> +	}
>> +	if (ret == 0) {
>> +		spin_unlock(&fs_info->reada_lock);
>> +		return 0;
>> +	}
>> +	dev->reada_next = re->logical + re->blocksize;
>> +	kref_get(&re->refcnt);
>> +
>> +	spin_unlock(&fs_info->reada_lock);
>> +
>> +	/*
>> +	 * find mirror num
>> +	 */
>> +	for (i = 0; i < re->nzones; ++i) {
>> +		if (re->zones[i]->device == dev) {
>> +			mirror_num = i + 1;
>> +			break;
>> +		}
>> +	}
>> +	logical = re->logical;
>> +	blocksize = re->blocksize;
>> +
>> +	spin_lock(&re->lock);
>> +	if (re->scheduled_for == NULL) {
>> +		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,
blocksize,
>> +			 mirror_num, &eb);
>> +	if (eb) {
>> +		__readahead_hook(fs_info->extent_root, eb, eb->start, ret);
>> +		free_extent_buffer(eb);
>> +	}
>> +
>> +	return 1;
>> +
>> +}
>> +
>> +static void reada_start_machine_worker(struct btrfs_work *work)
>> +{
>> +	struct reada_machine_work *rmw;
>> +	struct btrfs_fs_info *fs_info;
>> +
>> +	rmw = container_of(work, struct reada_machine_work, work);
>> +	fs_info = rmw->fs_info;
>> +
>> +	kfree(rmw);
>> +
>> +	__reada_start_machine(fs_info);
>> +}
>> +
>> +static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>> +{
>> +	struct btrfs_device *device;
>> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>> +	u64 enqueued;
>> +	u64 total = 0;
>> +	int i;
>> +
>> +	do {
>> +		enqueued = 0;
>> +		list_for_each_entry(device, &fs_devices->devices, dev_list) {
>> +			if (atomic_read(&device->reada_in_flight) <
>> +			    MAX_IN_FLIGHT)
>> +				enqueued += reada_start_machine_dev(fs_info,
>> +								    device);
>> +		}
>> +		total += enqueued;
>> +	} while (enqueued && total < 10000);
>> +
> 
> What is this?  Are we doing this so that the worker stays alive so it
> can continue to process new requests coming in?  If thats the case we
> need to have a proper kthread that doesn''t exit until we unmount
or
> something, not this weirdness.
Hopefully the comment below explains what the intention is. Maybe I
should move it up to answer the question before it arises :) A kthread
is not enough, as I want parallelism here.

I''ll kill the FIXME as it is done already.
> 
>> +	if (enqueued == 0)
>> +		return;
>> +
>> +	/*
>> +	 * If everything is already in the cache, this is effectively single
>> +	 * threaded. To a) not hold the caller for too long and b) to utilize
>> +	 * more cores, we broke the loop above after 10000 iterations and now
>> +	 * enqueue to workers to finish it. This will distribute the load to
>> +	 * the cores.
>> +	 * FIXME we might need our own workqueue here, with an idle threshold
>> +	 * of one. Also these worker are relatively long-running.
>> +	 */
>> +	for (i = 0; i < 2; ++i)
>> +		reada_start_machine(fs_info);
>> +}
>> +
>> +static void reada_start_machine(struct btrfs_fs_info *fs_info)
>> +{
>> +	struct reada_machine_work *rmw;
>> +
>> +	rmw = kzalloc(sizeof(*rmw), GFP_NOFS);
>> +	if (!rmw) {
>> +		/* FIXME we cannot handle this properly right now */
>> +		BUG();
> 
> Yes you can, everywhere that calls this can handle failures, so make
> this return an int and have it return -ENOMEM if it fails.
Just passing up the error isn''t enough. We also need to signal the
error
to all waiters and clean up all data structures. Maybe it''s easier to
just keep a small cache of these struct, maybe #CPUs, so we can never
fail here.
> 
>> +	}
>> +	rmw->work.func = reada_start_machine_worker;
>> +	rmw->fs_info = fs_info;
>> +
>> +	btrfs_queue_worker(&fs_info->readahead_workers,
&rmw->work);
>> +}
>> +
>> +#ifdef DEBUG
>> +static void dump_devs(struct btrfs_fs_info *fs_info, int all)
>> +{
>> +	struct btrfs_device *device;
>> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>> +	unsigned long index;
>> +	int ret;
>> +	int i;
>> +	int j;
>> +	int cnt;
>> +
>> +	spin_lock(&fs_info->reada_lock);
>> +	list_for_each_entry(device, &fs_devices->devices, dev_list) {
>> +		printk(KERN_DEBUG "dev %lld has %d in flight\n",
device->devid,
>> +			atomic_read(&device->reada_in_flight));
>> +		index = 0;
>> +		while (1) {
>> +			struct reada_zone *zone;
>> +			ret = radix_tree_gang_lookup(&device->reada_zones,
>> +						     (void **)&zone, index, 1);
>> +			if (ret == 0)
>> +				break;
>> +			printk(KERN_DEBUG "  zone %llu-%llu elems %llu locked "
>> +				"%d devs", zone->start, zone->end, zone->elems,
>> +				zone->locked);
>> +			for (j = 0; j < zone->ndevs; ++j) {
>> +				printk(KERN_CONT " %lld",
>> +					zone->devs[j]->devid);
>> +			}
>> +			if (device->reada_curr_zone == zone)
>> +				printk(KERN_CONT " curr off %llu",
>> +					device->reada_next - zone->start);
>> +			printk(KERN_CONT "\n");
>> +			index = (zone->end >> PAGE_CACHE_SHIFT) + 1;
>> +		}
>> +		cnt = 0;
>> +		index = 0;
>> +		while (all) {
>> +			struct reada_extent *re = NULL;
>> +
>> +			ret = radix_tree_gang_lookup(&device->reada_extents,
>> +						     (void **)&re, index, 1);
>> +			if (ret == 0)
>> +				break;
>> +			printk(KERN_DEBUG
>> +				"  re: logical %llu size %u empty %d for %lld",
>> +				re->logical, re->blocksize,
>> +				list_empty(&re->extctl), re->scheduled_for ?
>> +				re->scheduled_for->devid : -1);
>> +
>> +			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);
>> +				}
>> +			}
>> +			printk(KERN_CONT "\n");
>> +			index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
>> +			if (++cnt > 15)
>> +				break;
>> +		}
>> +	}
>> +
>> +	index = 0;
>> +	cnt = 0;
>> +	while (all) {
>> +		struct reada_extent *re = NULL;
>> +
>> +		ret = radix_tree_gang_lookup(&fs_info->reada_tree, (void
**)&re,
>> +					     index, 1);
>> +		if (ret == 0)
>> +			break;
>> +		if (!re->scheduled_for) {
>> +			index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
>> +			continue;
>> +		}
>> +		printk(KERN_DEBUG
>> +			"re: logical %llu size %u list empty %d for %lld",
>> +			re->logical, re->blocksize, list_empty(&re->extctl),
>> +			re->scheduled_for ? re->scheduled_for->devid : -1);
>> +		for (i = 0; i < re->nzones; ++i) {
>> +			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);
>> +				}
>> +			}
>> +		}
>> +		printk(KERN_CONT "\n");
>> +		index = (re->logical >> PAGE_CACHE_SHIFT) + 1;
>> +	}
>> +	spin_unlock(&fs_info->reada_lock);
>> +}
>> +#endif
>> +
>> +/*
>> + * interface
>> + */
>> +void *btrfs_reada_add(struct btrfs_root *root, struct btrfs_key
*key_start,
>> +		      struct btrfs_key *key_end)
>> +{
> 
> Don''t do this.  Add struct reada_control to a header file so all
the
> typing gets checked properly by the compiler and we don''t end up
with
> weirdness.
> 
ok.
>> +	struct reada_control *rc;
>> +	u64 start;
>> +	u64 generation;
>> +	int level;
>> +	struct extent_buffer *node;
>> +	static struct btrfs_key max_key = {
>> +		.objectid = (u64)-1,
>> +		.type = (u8)-1,
>> +		.offset = (u64)-1
>> +	};
>> +
> 
> This isn''t needed at all, kill it.
> 
>> +	rc = kzalloc(sizeof(*rc), GFP_NOFS);
>> +	if (!rc)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	rc->root = root;
>> +	rc->key_start = *key_start;
>> +	rc->key_end = *key_end;
>> +	spin_lock_init(&rc->lock);
>> +	rc->elems = 0;
>> +	rc->read_total = 0;
>> +	init_waitqueue_head(&rc->wait);
>> +	kref_init(&rc->refcnt);
>> +	kref_get(&rc->refcnt); /* one ref for having elements */
>> +
>> +	node = btrfs_root_node(root);
>> +	start = node->start;
>> +	level = btrfs_header_level(node);
>> +	generation = btrfs_header_generation(node);
>> +	free_extent_buffer(node);
>> +
>> +	reada_add_block(rc, start, &max_key, level, generation);
>> +
>> +	reada_start_machine(root->fs_info);
>> +
>> +	return rc;
>> +}
>> +
>> +#ifdef DEBUG
>> +int btrfs_reada_wait(void *handle)
>> +{
>> +	struct reada_control *rc = handle;
>> +
>> +	spin_lock(&rc->lock);
>> +	while (rc->elems) {
>> +		spin_unlock(&rc->lock);
>> +		wait_event_timeout(rc->wait, rc->elems == 0, 5 * HZ);
>> +		dump_devs(rc->root->fs_info, rc->elems < 10 ? 1 : 0);
>> +		spin_lock(&rc->lock);
>> +	}
>> +	spin_unlock(&rc->lock);
>> +
>> +	dump_devs(rc->root->fs_info, rc->elems < 10 ? 1 : 0);
>> +
>> +	reada_control_put(rc);
>> +
>> +	return 0;
>> +}
>> +#else
>> +int btrfs_reada_wait(void *handle)
>> +{
>> +	struct reada_control *rc = handle;
>> +
>> +	spin_lock(&rc->lock);
>> +	while (rc->elems) {
>> +		spin_unlock(&rc->lock);
>> +		wait_event(rc->wait, rc->elems == 0);
>> +		spin_lock(&rc->lock);
>> +	}
>> +	spin_unlock(&rc->lock);
>> +
>> +	reada_control_put(rc);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +void btrfs_reada_detach(void *handle)
>> +{
>> +	struct reada_control *rc = handle;
>> +
>> +	reada_control_put(rc);
>> +}
> 
> Thats all I have time for on this one, but that should be enough to keep
> you busy for a bit :).  I will look at it more tomorrow.  Thanks,
ok, I prepared the changes, but wait a bit before sending it out.

Thanks,
Arne
> 
> Josef
--
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

On 06/30/2011 03:37 AM, Arne Jansen wrote:
> On 29.06.2011 23:49, Josef Bacik wrote:
>> On 06/29/2011 04:10 PM, Arne Jansen wrote:
>
>>
>>> +	struct kref		refcnt;
>>> +	wait_queue_head_t	wait;
>>> +};
>>> +
>>> +struct reada_extctl {
>>> +	struct list_head	list;
>>> +	struct reada_control	*rc;
>>> +	u64			generation;
>>> +};
>>> +
>>
>> This is completely useless, kill this struct and just put the
generation
>> and the list into reada_control.
>
> This struct is the link between reada_extent and reada_control.
> In case more than one readahead is running, each reada_extent
> can point to multiple reada_controls, so I need this link struct.
>
And this is where I get confused.  Why would we need multiple 
reada_control''s for the same extent?  Can''t we just say
"Oh hey there''s
already a reada control outstanding for this extent, take a ref on the 
control and wait for that"?

>>> +
>>> +/* call it with fs_info->reada_lock held */
>>> +static void reada_zone_put(struct reada_zone *zone)
>>> +{
>>> +	if (!kref_put(&zone->refcnt, reada_kref_dummy))
>>> +		return;
>>> +
>>> +	radix_tree_delete(&zone->device->reada_zones,
>>> +			  zone->end>>  PAGE_CACHE_SHIFT);
>>> +
>>
>> Instead of making the callers take the reada_lock, move it into this
>> function so that in the fast case we''re not taking an extra
spin_lock.
>
> I had to move this out, mainly because reada_start_machine_dev needs to
> hold the lock for several operation, one of which might call zone_put.
> Maybe I can defer the zone_put until afterwards.
> I''d like to wait with these kind of optimizations until there are
some
> uses of readahead in more time critical paths, or at least until we have
> settled there will be such uses ;)
>
>> Also if you are going to use the kfref stuff you might as well use the
>> release function stuff.
>
> Right, in the case I can. I didn''t do it for symmetrie with the
other
> cases, but changed it now.
>
>>
>>> +	kfree(zone);
>>> +
>>> +	return;
>>> +}
>>> +
>>> +static void reada_control_put(struct reada_control *rc)
>>> +{
>>> +	if (kref_put(&rc->refcnt, reada_kref_dummy)) {
>>> +		kfree(rc);
>>> +		return;
>>
>> Don''t need the return here.
>
> I killed the whole function and built reada_control_release instead.
>
>>
>>> +	}
>>> +}
>>> +
>>> +static int reada_add_block(struct reada_control *rc, u64 logical,
>>> +			   struct btrfs_key *top, int level, 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 */
>>> +	if (!re)
>>> +		return -1;
>>> +
>>> +	rec = kzalloc(sizeof(*rec), GFP_NOFS);
>>> +	if (!rec) {
>>> +		reada_extent_put(root->fs_info, re);
>>> +		return -1;
>>> +	}
>>> +
>>> +	rec->rc = rc;
>>> +	rec->generation = generation;
>>> +	spin_lock(&rc->lock);
>>> +	++rc->elems;
>>> +	spin_unlock(&rc->lock);
>>> +
>>> +	spin_lock(&re->lock);
>>> +	list_add_tail(&rec->list,&re->extctl);
>>> +	spin_unlock(&re->lock);
>>> +
>>> +	/* leave the ref on the extent */
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/*
>>> + * called with fs_info->reada_lock held
>>> + */
>>> +static void reada_peer_zones_set_lock(struct reada_zone *zone, int
lock)
>>> +{
>>> +	int i;
>>> +	unsigned long index = zone->end>>  PAGE_CACHE_SHIFT;
>>> +
>>> +	for (i = 0; i<  zone->ndevs; ++i) {
>>> +		struct reada_zone *peer;
>>> +		peer = radix_tree_lookup(&zone->devs[i]->reada_zones,
index);
>>> +		if (peer&&  peer->device != zone->device)
>>> +			peer->locked = lock;
>>> +	}
>>> +}
>>> +
>>> +/*
>>> + * called with fs_info->reada_lock held
>>> + */
>>> +static int reada_pick_zone(struct btrfs_device *dev)
>>> +{
>>> +	struct reada_zone *top_zone = NULL;
>>> +	struct reada_zone *top_locked_zone = NULL;
>>> +	u64 top_elems = 0;
>>> +	u64 top_locked_elems = 0;
>>> +	unsigned long index = 0;
>>> +	int ret;
>>> +
>>> +	if (dev->reada_curr_zone) {
>>> +		reada_peer_zones_set_lock(dev->reada_curr_zone, 0);
>>> +		reada_zone_put(dev->reada_curr_zone);
>>> +		dev->reada_curr_zone = NULL;
>>> +	}
>>> +	/* pick the zone with the most elements */
>>> +	while (1) {
>>> +		struct reada_zone *zone;
>>> +
>>> +		ret = radix_tree_gang_lookup(&dev->reada_zones,
>>> +					     (void **)&zone, index, 1);
>>> +		if (ret == 0)
>>> +			break;
>>> +		index = (zone->end>>  PAGE_CACHE_SHIFT) + 1;
>>> +		if (zone->locked) {
>>> +			if (zone->elems>  top_locked_elems) {
>>> +				top_locked_elems = zone->elems;
>>> +				top_locked_zone = zone;
>>> +			}
>>> +		} else {
>>> +			if (zone->elems>  top_elems) {
>>> +				top_elems = zone->elems;
>>> +				top_zone = zone;
>>> +			}
>>> +		}
>>> +	}
>>> +	if (top_zone)
>>> +		dev->reada_curr_zone = top_zone;
>>> +	else if (top_locked_zone)
>>> +		dev->reada_curr_zone = top_locked_zone;
>>> +	else
>>> +		return 0;
>>> +
>>> +	dev->reada_next = dev->reada_curr_zone->start;
>>> +	kref_get(&dev->reada_curr_zone->refcnt);
>>> +	reada_peer_zones_set_lock(dev->reada_curr_zone, 1);
>>> +
>>> +	return 1;
>>> +}
>>> +
>>> +static int reada_start_machine_dev(struct btrfs_fs_info *fs_info,
>>> +				   struct btrfs_device *dev)
>>> +{
>>> +	struct reada_extent *re = NULL;
>>> +	int mirror_num = 0;
>>> +	struct extent_buffer *eb = NULL;
>>> +	u64 logical;
>>> +	u32 blocksize;
>>> +	int ret;
>>> +	int i;
>>> +	int need_kick = 0;
>>> +
>>> +	spin_lock(&fs_info->reada_lock);
>>> +	if (dev->reada_curr_zone == NULL) {
>>> +		ret = reada_pick_zone(dev);
>>> +		if (!ret) {
>>> +			spin_unlock(&fs_info->reada_lock);
>>> +			return 0;
>>> +		}
>>> +	}
>>> +	/*
>>> +	 * FIXME currently we issue the reads one extent at a time. If we
have
>>> +	 * a contiguous block of extents, we could also coagulate them or
use
>>> +	 * plugging to speed things up
>>> +	 */
>>> +	ret = radix_tree_gang_lookup(&dev->reada_extents, (void
**)&re,
>>> +				     dev->reada_next>>  PAGE_CACHE_SHIFT, 1);
>>> +	if (ret == 0 || re->logical>=
dev->reada_curr_zone->end) {
>>> +		ret = reada_pick_zone(dev);
>>> +		if (!ret) {
>>> +			spin_unlock(&fs_info->reada_lock);
>>> +			return 0;
>>> +		}
>>> +		re = NULL;
>>> +		ret = radix_tree_gang_lookup(&dev->reada_extents, (void
**)&re,
>>> +					dev->reada_next>>  PAGE_CACHE_SHIFT, 1);
>>> +	}
>>> +	if (ret == 0) {
>>> +		spin_unlock(&fs_info->reada_lock);
>>> +		return 0;
>>> +	}
>>> +	dev->reada_next = re->logical + re->blocksize;
>>> +	kref_get(&re->refcnt);
>>> +
>>> +	spin_unlock(&fs_info->reada_lock);
>>> +
>>> +	/*
>>> +	 * find mirror num
>>> +	 */
>>> +	for (i = 0; i<  re->nzones; ++i) {
>>> +		if (re->zones[i]->device == dev) {
>>> +			mirror_num = i + 1;
>>> +			break;
>>> +		}
>>> +	}
>>> +	logical = re->logical;
>>> +	blocksize = re->blocksize;
>>> +
>>> +	spin_lock(&re->lock);
>>> +	if (re->scheduled_for == NULL) {
>>> +		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,
blocksize,
>>> +			 mirror_num,&eb);
>>> +	if (eb) {
>>> +		__readahead_hook(fs_info->extent_root, eb, eb->start,
ret);
>>> +		free_extent_buffer(eb);
>>> +	}
>>> +
>>> +	return 1;
>>> +
>>> +}
>>> +
>>> +static void reada_start_machine_worker(struct btrfs_work *work)
>>> +{
>>> +	struct reada_machine_work *rmw;
>>> +	struct btrfs_fs_info *fs_info;
>>> +
>>> +	rmw = container_of(work, struct reada_machine_work, work);
>>> +	fs_info = rmw->fs_info;
>>> +
>>> +	kfree(rmw);
>>> +
>>> +	__reada_start_machine(fs_info);
>>> +}
>>> +
>>> +static void __reada_start_machine(struct btrfs_fs_info *fs_info)
>>> +{
>>> +	struct btrfs_device *device;
>>> +	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>>> +	u64 enqueued;
>>> +	u64 total = 0;
>>> +	int i;
>>> +
>>> +	do {
>>> +		enqueued = 0;
>>> +		list_for_each_entry(device,&fs_devices->devices,
dev_list) {
>>> +			if (atomic_read(&device->reada_in_flight)<
>>> +			    MAX_IN_FLIGHT)
>>> +				enqueued += reada_start_machine_dev(fs_info,
>>> +								    device);
>>> +		}
>>> +		total += enqueued;
>>> +	} while (enqueued&&  total<  10000);
>>> +
>>
>> What is this?  Are we doing this so that the worker stays alive so it
>> can continue to process new requests coming in?  If thats the case we
>> need to have a proper kthread that doesn''t exit until we
unmount or
>> something, not this weirdness.
>
> Hopefully the comment below explains what the intention is. Maybe I
> should move it up to answer the question before it arises :) A kthread
> is not enough, as I want parallelism here.
>
> I''ll kill the FIXME as it is done already.
>
Yeah the comment wasn''t and still isn''t clear to me.  You are
using the
workers here, which means we already have a thread per cpu running here, 
so we don''t need to do things to artificially keep one of them running,
we just give it work to do and the worker threads will dispatch a thread 
to do the work, we have built in parallelism, so this is just confusing 
and unnecessary.
>>
>>> +	if (enqueued == 0)
>>> +		return;
>>> +
>>> +	/*
>>> +	 * If everything is already in the cache, this is effectively
single
>>> +	 * threaded. To a) not hold the caller for too long and b) to
utilize
>>> +	 * more cores, we broke the loop above after 10000 iterations and
now
>>> +	 * enqueue to workers to finish it. This will distribute the load
to
>>> +	 * the cores.
>>> +	 * FIXME we might need our own workqueue here, with an idle
threshold
>>> +	 * of one. Also these worker are relatively long-running.
>>> +	 */
>>> +	for (i = 0; i<  2; ++i)
>>> +		reada_start_machine(fs_info);
>>> +}
>>> +
>>> +static void reada_start_machine(struct btrfs_fs_info *fs_info)
>>> +{
>>> +	struct reada_machine_work *rmw;
>>> +
>>> +	rmw = kzalloc(sizeof(*rmw), GFP_NOFS);
>>> +	if (!rmw) {
>>> +		/* FIXME we cannot handle this properly right now */
>>> +		BUG();
>>
>> Yes you can, everywhere that calls this can handle failures, so make
>> this return an int and have it return -ENOMEM if it fails.
>
> Just passing up the error isn''t enough. We also need to signal the
error
> to all waiters and clean up all data structures. Maybe it''s easier
to
> just keep a small cache of these struct, maybe #CPUs, so we can never
> fail here.
Well this is just used to add the current readahead work right?  We can 
fail here with no problems, it just means the person wanting to add the 
readahead work failed, there is no reason to stop any of the other 
workers who may already have work going on.  Thanks,

Josef
--
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

On 30.06.2011 14:49, Josef Bacik wrote:
> On 06/30/2011 03:37 AM, Arne Jansen wrote:
>> On 29.06.2011 23:49, Josef Bacik wrote:
>>> On 06/29/2011 04:10 PM, Arne Jansen wrote:
> 
>>
>>>
>>>> +    struct kref        refcnt;
>>>> +    wait_queue_head_t    wait;
>>>> +};
>>>> +
>>>> +struct reada_extctl {
>>>> +    struct list_head    list;
>>>> +    struct reada_control    *rc;
>>>> +    u64            generation;
>>>> +};
>>>> +
>>>
>>> This is completely useless, kill this struct and just put the
generation
>>> and the list into reada_control.
>>
>> This struct is the link between reada_extent and reada_control.
>> In case more than one readahead is running, each reada_extent
>> can point to multiple reada_controls, so I need this link struct.
>>
> 
> And this is where I get confused.  Why would we need multiple
> reada_control''s for the same extent?  Can''t we just say
"Oh hey there''s
> already a reada control outstanding for this extent, take a ref on the
> control and wait for that"?
Each readahead request allocates a single reada_control, which basically
records the range and the tree to preload.
For each reqested extent a reada_extent is allocated, but only one, even
if there are more than one readaheads waiting for it. When the extent
arrives, we have to check the extent (e.g. an intermediate node) against
all waiting reada_controls to see which links we have to follow and
which not. For this, we have to attach links to each reada_control to
the extent. As the list differs from extent to extent, I build each
list with the help of these stub structs.
> 
> 
[snip]
>>>> +
>>>> +static void __reada_start_machine(struct btrfs_fs_info
*fs_info)
>>>> +{
>>>> +    struct btrfs_device *device;
>>>> +    struct btrfs_fs_devices *fs_devices =
fs_info->fs_devices;
>>>> +    u64 enqueued;
>>>> +    u64 total = 0;
>>>> +    int i;
>>>> +
>>>> +    do {
>>>> +        enqueued = 0;
>>>> +       
list_for_each_entry(device,&fs_devices->devices, dev_list) {
>>>> +            if
(atomic_read(&device->reada_in_flight)<
>>>> +                MAX_IN_FLIGHT)
>>>> +                enqueued += reada_start_machine_dev(fs_info,
>>>> +                                    device);
>>>> +        }
>>>> +        total += enqueued;
>>>> +    } while (enqueued&&  total<  10000);
>>>> +
>>>
>>> What is this?  Are we doing this so that the worker stays alive so
it
>>> can continue to process new requests coming in?  If thats the case
we
>>> need to have a proper kthread that doesn''t exit until we
unmount or
>>> something, not this weirdness.
>>
>> Hopefully the comment below explains what the intention is. Maybe I
>> should move it up to answer the question before it arises :) A kthread
>> is not enough, as I want parallelism here.
>>
>> I''ll kill the FIXME as it is done already.
>>
> 
> Yeah the comment wasn''t and still isn''t clear to me.  You
are using the
> workers here, which means we already have a thread per cpu running here,
> so we don''t need to do things to artificially keep one of them
running,
> we just give it work to do and the worker threads will dispatch a thread
> to do the work, we have built in parallelism, so this is just confusing
> and unnecessary.
We only have builtin parallelism in case we actually read from disk. In
case everything is already in RAM, the loop is broken in smaller parts,
and 2 new workers are kicked off every 10000.
In the case where most of the tree has to be read from disk, this case
will never be hit.
> 
>>>
>>>> +    if (enqueued == 0)
>>>> +        return;
>>>> +
>>>> +    /*
>>>> +     * If everything is already in the cache, this is
effectively
>>>> single
>>>> +     * threaded. To a) not hold the caller for too long and b)
to
>>>> utilize
>>>> +     * more cores, we broke the loop above after 10000
iterations
>>>> and now
>>>> +     * enqueue to workers to finish it. This will distribute
the
>>>> load to
>>>> +     * the cores.
>>>> +     * FIXME we might need our own workqueue here, with an
idle
>>>> threshold
>>>> +     * of one. Also these worker are relatively long-running.
>>>> +     */
>>>> +    for (i = 0; i<  2; ++i)
>>>> +        reada_start_machine(fs_info);
>>>> +}
>>>> +
>>>> +static void reada_start_machine(struct btrfs_fs_info *fs_info)
>>>> +{
>>>> +    struct reada_machine_work *rmw;
>>>> +
>>>> +    rmw = kzalloc(sizeof(*rmw), GFP_NOFS);
>>>> +    if (!rmw) {
>>>> +        /* FIXME we cannot handle this properly right now */
>>>> +        BUG();
>>>
>>> Yes you can, everywhere that calls this can handle failures, so
make
>>> this return an int and have it return -ENOMEM if it fails.
>>
>> Just passing up the error isn''t enough. We also need to signal
the error
>> to all waiters and clean up all data structures. Maybe it''s
easier to
>> just keep a small cache of these struct, maybe #CPUs, so we can never
>> fail here.
> 
> Well this is just used to add the current readahead work right?  We can
> fail here with no problems, it just means the person wanting to add the
> readahead work failed, there is no reason to stop any of the other
> workers who may already have work going on.  Thanks,
No, this is to trigger more of the enqueued requests. If we don''t
trigger them, they will hang forever, and with them the requester of
the readahead. This code is already called in worker context.
The only real problem arises when the this is the last running worker
and no I/O requests are outstanding. In this case ignoring the failure
would just keep the machine hanging forever. So in theory we would only
need to keep one spare struct to keep things running.

-Arne
> 
> Josef
--
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