Re: [PATCH] Btrfs: implement support for fallocate collapse range

2014-07-03 Thread Filipe David Manana
On Thu, Jul 3, 2014 at 5:46 AM, Chandan Rajendra
chan...@linux.vnet.ibm.com wrote:
 On Monday 23 Jun 2014 11:25:47 Filipe David Borba Manana wrote:
 diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
 index aeab453..8f1a371 100644
 --- a/fs/btrfs/ctree.c
 +++ b/fs/btrfs/ctree.c
 @@ -2825,12 +2825,12 @@ cow_done:
* It is safe to drop the lock on our parent before we
* go through the expensive btree search on b.
*
 -  * If we're inserting or deleting (ins_len != 0), then we might
 -  * be changing slot zero, which may require changing the 
 parent.
 -  * So, we can't drop the lock until after we know which slot
 -  * we're operating on.
 +  * If we're inserting, deleting or updating a key (cow != 0),
 +  * then we might be changing slot zero, which may require
 +  * changing the parent. So, we can't drop the lock until after
 +  * we know which slot we're operating on.
*/
 - if (!ins_len  !p-keep_locks) {
 + if (!cow  !p-keep_locks) {
   int u = level + 1;

 For the key update case (i.e. (ins_len == 0) and (cow == 1)), maybe
 we could optimize by having a variable to hold the return value of
 should_cow_block(), i.e. it keeps track of whether the current
 metadata block was COWed. If the variable indicates that the block was
 *not* COWed, then we could release the lock on the parent block since
 the update operation (even on slot 0) isn't going to change the
 corresponding entry in the parent block.

Hi Chandan,

Just because a node wasn't cowed it doesn't mean it isn't going to be
updated. Further updating the key works bottom-up - we don't know
during btrfs_search_slot() if our caller intends to update a key in
the leaf or just update the item, so we need to return a path here
with all nodes with slot == 0 having a write lock on them. I'm
actually for now basically just undoing this
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=eb653de15987612444b6cde3b0e67b1edd94625f

Anyway right now I have a functional issue to tackle first.

Thanks.


 Thanks,
 chandan




-- 
Filipe David Manana,

Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men.
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: implement support for fallocate collapse range

2014-07-02 Thread Chandan Rajendra
On Monday 23 Jun 2014 11:25:47 Filipe David Borba Manana wrote:
 diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
 index aeab453..8f1a371 100644
 --- a/fs/btrfs/ctree.c
 +++ b/fs/btrfs/ctree.c
 @@ -2825,12 +2825,12 @@ cow_done:
* It is safe to drop the lock on our parent before we
* go through the expensive btree search on b.
*
 -  * If we're inserting or deleting (ins_len != 0), then we might
 -  * be changing slot zero, which may require changing the parent.
 -  * So, we can't drop the lock until after we know which slot
 -  * we're operating on.
 +  * If we're inserting, deleting or updating a key (cow != 0),
 +  * then we might be changing slot zero, which may require
 +  * changing the parent. So, we can't drop the lock until after
 +  * we know which slot we're operating on.
*/
 - if (!ins_len  !p-keep_locks) {
 + if (!cow  !p-keep_locks) {
   int u = level + 1;

For the key update case (i.e. (ins_len == 0) and (cow == 1)), maybe
we could optimize by having a variable to hold the return value of
should_cow_block(), i.e. it keeps track of whether the current
metadata block was COWed. If the variable indicates that the block was
*not* COWed, then we could release the lock on the parent block since
the update operation (even on slot 0) isn't going to change the
corresponding entry in the parent block.

Thanks,
chandan

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Btrfs: implement support for fallocate collapse range

2014-06-23 Thread Filipe David Borba Manana
This implements fallocate's FALLOC_FL_COLLAPSE_RANGE operation for
BTRFS. This fallocate operation was introduced in the linux kernel
version 3.15.

Existing tests in xfstests already test this operation explicitly
and implicitly via fsstress.

Signed-off-by: Filipe David Borba Manana fdman...@gmail.com
---
 fs/btrfs/ctree.c   |  42 -
 fs/btrfs/ctree.h   |   2 +
 fs/btrfs/extent-tree.c |  30 +--
 fs/btrfs/file.c| 486 +
 4 files changed, 453 insertions(+), 107 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index aeab453..8f1a371 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2825,12 +2825,12 @@ cow_done:
 * It is safe to drop the lock on our parent before we
 * go through the expensive btree search on b.
 *
-* If we're inserting or deleting (ins_len != 0), then we might
-* be changing slot zero, which may require changing the parent.
-* So, we can't drop the lock until after we know which slot
-* we're operating on.
+* If we're inserting, deleting or updating a key (cow != 0),
+* then we might be changing slot zero, which may require
+* changing the parent. So, we can't drop the lock until after
+* we know which slot we're operating on.
 */
-   if (!ins_len  !p-keep_locks) {
+   if (!cow  !p-keep_locks) {
int u = level + 1;
 
if (u  BTRFS_MAX_LEVEL  p-locks[u]) {
@@ -2865,7 +2865,7 @@ cow_done:
 * which means we must have a write lock
 * on the parent
 */
-   if (slot == 0  ins_len 
+   if (slot == 0  cow 
write_lock_level  level + 1) {
write_lock_level = level + 1;
btrfs_release_path(p);
@@ -5660,6 +5660,36 @@ next:
 }
 
 /*
+ * This differs from btrfs_find_next_key in that it ignores leaf/node
+ * generations and it doesn't unlock and re-lock nodes/leaves nor does
+ * any subsequent searches (calls to btrfs_search_slot), preserving the
+ * locks in the given path.
+ *
+ * Returns 0 if a next key exists, 1 otherwise.
+ */
+int btrfs_find_next_current_key(struct btrfs_path *path, int level,
+   struct btrfs_key *key)
+
+{
+   for (; level  BTRFS_MAX_LEVEL; level++) {
+   if (!path-nodes[level])
+   break;
+   if (path-slots[level] + 1 =
+   btrfs_header_nritems(path-nodes[level]))
+   continue;
+   if (level == 0)
+   btrfs_item_key_to_cpu(path-nodes[level], key,
+ path-slots[level] + 1);
+   else
+   btrfs_node_key_to_cpu(path-nodes[level], key,
+ path-slots[level] + 1);
+   return 0;
+   }
+   return 1;
+}
+
+
+/*
  * search the tree again to find a leaf with greater keys
  * returns 0 if it found something or 1 if there are no greater leaves.
  * returns  0 on io errors.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b7e2c1c..166a35f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3446,6 +3446,8 @@ struct extent_buffer *btrfs_lock_root_node(struct 
btrfs_root *root);
 int btrfs_find_next_key(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key *key, int lowest_level,
u64 min_trans);
+int btrfs_find_next_current_key(struct btrfs_path *path, int level,
+   struct btrfs_key *key);
 int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
 struct btrfs_path *path,
 u64 min_trans);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index fafb3e5..a6d0ec7 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -100,8 +100,6 @@ static int alloc_reserved_tree_block(struct 
btrfs_trans_handle *trans,
 static int do_chunk_alloc(struct btrfs_trans_handle *trans,
  struct btrfs_root *extent_root, u64 flags,
  int force);
-static int find_next_key(struct btrfs_path *path, int level,
-struct btrfs_key *key);
 static void dump_space_info(struct btrfs_space_info *info, u64 bytes,
int dump_block_groups);
 static int btrfs_update_reserved_bytes(struct btrfs_block_group_cache *cache,
@@ -440,7 +438,7 @@ next:
if (path-slots[0]  nritems) {
btrfs_item_key_to_cpu(leaf, key, path-slots[0]);
} else {
-   ret = find_next_key(path,