Re: [RFC PATCH] btrfs: Remove V0 extent support

2018-06-21 Thread Chris Mason




On 21 Jun 2018, at 5:22, David Sterba wrote:


On Thu, Jun 21, 2018 at 09:45:00AM +0300, Nikolay Borisov wrote:

The v0 compat code was introduced in commit 5d4f98a28c7d
("Btrfs: Mixed back reference  (FORWARD ROLLING FORMAT CHANGE)") 9
years ago, which was merged in 2.6.31. This means that the code is
there to support filesystems which are _VERY_ old and if you are 
using
btrfs on such an old kernel, you have much bigger problems. This 
coupled
with the fact that no one is likely testing/maintining this code 
likely

means it has bugs lurking. All things considered I think 43 kernel
releases later it's high time this remnant of the past got removed.


Ack. Given the age of the format and nearly zero chance of such
filesystem to be still in use it should be ok to remove it directly 
and

not add some sort of warning when such filesystem is found.


Agreed, it seems very unlikely to me that we have happy v0 users.



This patch removes all code wrapped in #ifdefs but leaves the BUG_ONs 
in case

we have a v0 with no support intact as a sort of safety-net.


I think there should be some verbose way to report what happened and
possibly do proper error handling and go to read-only eventually.
The type of checks for v0 do not seem to be something that could be 
done

at mount time.


Yeah, with the code we have right now, it's really obvious the BUG()s 
are for V0 extents.  With all the ifdefs removed its much less clear.  
I'd swap them out for a transaction abort with a message about the v0 
extents.


Also, there's at least one ASSERT() left over in a #else, which I would 
also turn into a more explicit abort or at least comment.


-chris
--
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: [RFC PATCH] btrfs: Remove V0 extent support

2018-06-21 Thread David Sterba
On Thu, Jun 21, 2018 at 09:45:00AM +0300, Nikolay Borisov wrote:
> The v0 compat code was introduced in commit 5d4f98a28c7d
> ("Btrfs: Mixed back reference  (FORWARD ROLLING FORMAT CHANGE)") 9
> years ago, which was merged in 2.6.31. This means that the code is
> there to support filesystems which are _VERY_ old and if you are using
> btrfs on such an old kernel, you have much bigger problems. This coupled
> with the fact that no one is likely testing/maintining this code likely
> means it has bugs lurking. All things considered I think 43 kernel
> releases later it's high time this remnant of the past got removed.

Ack. Given the age of the format and nearly zero chance of such
filesystem to be still in use it should be ok to remove it directly and
not add some sort of warning when such filesystem is found.

> This patch removes all code wrapped in #ifdefs but leaves the BUG_ONs in case
> we have a v0 with no support intact as a sort of safety-net. 

I think there should be some verbose way to report what happened and
possibly do proper error handling and go to read-only eventually.
The type of checks for v0 do not seem to be something that could be done
at mount time.
--
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


[RFC PATCH] btrfs: Remove V0 extent support

2018-06-21 Thread Nikolay Borisov
The v0 compat code was introduced in commit 5d4f98a28c7d
("Btrfs: Mixed back reference  (FORWARD ROLLING FORMAT CHANGE)") 9
years ago, which was merged in 2.6.31. This means that the code is
there to support filesystems which are _VERY_ old and if you are using
btrfs on such an old kernel, you have much bigger problems. This coupled
with the fact that no one is likely testing/maintining this code likely
means it has bugs lurking. All things considered I think 43 kernel
releases later it's high time this remnant of the past got removed.

This patch removes all code wrapped in #ifdefs but leaves the BUG_ONs in case
we have a v0 with no support intact as a sort of safety-net. 

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/ctree.c   |   6 +-
 fs/btrfs/ctree.h   |   2 -
 fs/btrfs/extent-tree.c | 209 +
 fs/btrfs/print-tree.c  |  30 +--
 fs/btrfs/relocation.c  | 151 +--
 5 files changed, 4 insertions(+), 394 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6879697520d5..d1273ec2e0d5 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -888,11 +888,7 @@ int btrfs_block_can_be_shared(struct btrfs_root *root,
 btrfs_root_last_snapshot(>root_item) ||
 btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC)))
return 1;
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-   if (test_bit(BTRFS_ROOT_REF_COWS, >state) &&
-   btrfs_header_backref_rev(buf) < BTRFS_MIXED_BACKREF_REV)
-   return 1;
-#endif
+
return 0;
 }
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e671a1fcbbec..bc52bf7ac572 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -55,8 +55,6 @@ struct btrfs_ordered_sum;
 
 #define BTRFS_OLDEST_GENERATION0ULL
 
-#define BTRFS_COMPAT_EXTENT_TREE_V0
-
 /*
  * the max metadata block size.  This limit is somewhat artificial,
  * but the memmove costs go through the roof for larger blocks.
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 0f7e797236dc..4129831523a2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -870,17 +870,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle 
*trans,
num_refs = btrfs_extent_refs(leaf, ei);
extent_flags = btrfs_extent_flags(leaf, ei);
} else {
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-   struct btrfs_extent_item_v0 *ei0;
-   BUG_ON(item_size != sizeof(*ei0));
-   ei0 = btrfs_item_ptr(leaf, path->slots[0],
-struct btrfs_extent_item_v0);
-   num_refs = btrfs_extent_refs_v0(leaf, ei0);
-   /* FIXME: this isn't correct for data */
-   extent_flags = BTRFS_BLOCK_FLAG_FULL_BACKREF;
-#else
BUG();
-#endif
}
BUG_ON(num_refs == 0);
} else {
@@ -1039,89 +1029,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle 
*trans,
  * tree block info structure.
  */
 
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-static int convert_extent_item_v0(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info,
- struct btrfs_path *path,
- u64 owner, u32 extra_size)
-{
-   struct btrfs_root *root = fs_info->extent_root;
-   struct btrfs_extent_item *item;
-   struct btrfs_extent_item_v0 *ei0;
-   struct btrfs_extent_ref_v0 *ref0;
-   struct btrfs_tree_block_info *bi;
-   struct extent_buffer *leaf;
-   struct btrfs_key key;
-   struct btrfs_key found_key;
-   u32 new_size = sizeof(*item);
-   u64 refs;
-   int ret;
-
-   leaf = path->nodes[0];
-   BUG_ON(btrfs_item_size_nr(leaf, path->slots[0]) != sizeof(*ei0));
-
-   btrfs_item_key_to_cpu(leaf, , path->slots[0]);
-   ei0 = btrfs_item_ptr(leaf, path->slots[0],
-struct btrfs_extent_item_v0);
-   refs = btrfs_extent_refs_v0(leaf, ei0);
-
-   if (owner == (u64)-1) {
-   while (1) {
-   if (path->slots[0] >= btrfs_header_nritems(leaf)) {
-   ret = btrfs_next_leaf(root, path);
-   if (ret < 0)
-   return ret;
-   BUG_ON(ret > 0); /* Corruption */
-   leaf = path->nodes[0];
-   }
-   btrfs_item_key_to_cpu(leaf, _key,
- path->slots[0]);
-   BUG_ON(key.objectid != found_key.objectid);
-   if (found_key.type != BTRFS_EXTENT_REF_V0_KEY) {
-   path->slots[0]++;
-   continue;
-   }
-