Re: [PATCH v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-07 Thread Mark Fasheh 
On Tue, Jun 07, 2016 at 08:42:46AM +0800, Qu Wenruo wrote:
> 
> 
> At 06/07/2016 03:54 AM, Mark Fasheh wrote:
> >On Sat, Jun 04, 2016 at 06:26:39PM +0800, Qu Wenruo wrote:
> >>
> >>
> >>On 06/03/2016 10:27 PM, Josef Bacik wrote:
> >>>On 06/01/2016 09:12 PM, Qu Wenruo wrote:
> 
> 
> At 06/02/2016 06:08 AM, Mark Fasheh wrote:
> >On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:
> >>Core implement for inband de-duplication.
> >>It reuse the async_cow_start() facility to do the calculate dedupe
> >>hash.
> >>And use dedupe hash to do inband de-duplication at extent level.
> >>
> >>The work flow is as below:
> >>1) Run delalloc range for an inode
> >>2) Calculate hash for the delalloc range at the unit of dedupe_bs
> >>3) For hash match(duplicated) case, just increase source extent ref
> >>  and insert file extent.
> >>  For hash mismatch case, go through the normal cow_file_range()
> >>  fallback, and add hash into dedupe_tree.
> >>  Compress for hash miss case is not supported yet.
> >>
> >>Current implement restore all dedupe hash in memory rb-tree, with LRU
> >>behavior to control the limit.
> >>
> >>Signed-off-by: Wang Xiaoguang 
> >>Signed-off-by: Qu Wenruo 
> >>---
> >>fs/btrfs/extent-tree.c |  18 
> >>fs/btrfs/inode.c   | 235
> >>++---
> >>fs/btrfs/relocation.c  |  16 
> >>3 files changed, 236 insertions(+), 33 deletions(-)
> >>
> >>diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> >>index 53e1297..dabd721 100644
> >>--- a/fs/btrfs/extent-tree.c
> >>+++ b/fs/btrfs/extent-tree.c
> >>@@ -37,6 +37,7 @@
> >>#include "math.h"
> >>#include "sysfs.h"
> >>#include "qgroup.h"
> >>+#include "dedupe.h"
> >>
> >>#undef SCRAMBLE_DELAYED_REFS
> >>
> >>@@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct
> >>btrfs_trans_handle *trans,
> >>
> >>if (btrfs_delayed_ref_is_head(node)) {
> >>struct btrfs_delayed_ref_head *head;
> >>+struct btrfs_fs_info *fs_info = root->fs_info;
> >>+
> >>/*
> >> * we've hit the end of the chain and we were supposed
> >> * to insert this extent into the tree.  But, it got
> >>@@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct
> >>btrfs_trans_handle *trans,
> >>btrfs_pin_extent(root, node->bytenr,
> >> node->num_bytes, 1);
> >>if (head->is_data) {
> >>+/*
> >>+ * If insert_reserved is given, it means
> >>+ * a new extent is revered, then deleted
> >>+ * in one tran, and inc/dec get merged to 0.
> >>+ *
> >>+ * In this case, we need to remove its dedup
> >>+ * hash.
> >>+ */
> >>+btrfs_dedupe_del(trans, fs_info, node->bytenr);
> >>ret = btrfs_del_csums(trans, root,
> >>  node->bytenr,
> >>  node->num_bytes);
> >>@@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct
> >>btrfs_trans_handle *trans,
> >>btrfs_release_path(path);
> >>
> >>if (is_data) {
> >>+ret = btrfs_dedupe_del(trans, info, bytenr);
> >>+if (ret < 0) {
> >>+btrfs_abort_transaction(trans, extent_root,
> >>+ret);
> >
> >I don't see why an error here should lead to a readonly fs.
> >   --Mark
> >
> 
> Because such deletion error can lead to corruption.
> 
> For example, extent A is already in hash pool.
> And when freeing extent A, we need to delete its hash, of course.
> 
> But if such deletion fails, which means the hash is still in the pool,
> even the extent A no longer exists in extent tree.
> >>>
> >>>Except if we're in in-memory mode only it doesn't matter, so don't abort
> >>>if we're in in-memory mode.  Thanks,
> >>>
> >>>Josef
> >>>
> >>
> >>If we can't ensure a hash is delete along with the extent, we will
> >>screw up the whole fs, as new write can points to non-exist extent.
> >>
> >>Although you're right with in-memory mode here, we won't abort
> >>trans, as inmem_del_hash() won't return error code. It will always
> >>return 0.
> >
> >Until a third party comes along and changes it to return an error code and
> >neither you or I are there to remind them to fix this check (or have simply
> >forgotten).
> >
> >
> >>So still, no need to change anyway.
> >
> >Personally I'd call this 'defensive coding' and do a check for in-memory
> >only before our abort_trans().  This would have no effect on our running
> >code but avoids the problem

Re: [PATCH v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-06 Thread Qu Wenruo 



At 06/07/2016 03:54 AM, Mark Fasheh wrote:

On Sat, Jun 04, 2016 at 06:26:39PM +0800, Qu Wenruo wrote:



On 06/03/2016 10:27 PM, Josef Bacik wrote:

On 06/01/2016 09:12 PM, Qu Wenruo wrote:



At 06/02/2016 06:08 AM, Mark Fasheh wrote:

On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:

Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe
hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
  and insert file extent.
  For hash mismatch case, go through the normal cow_file_range()
  fallback, and add hash into dedupe_tree.
  Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
fs/btrfs/extent-tree.c |  18 
fs/btrfs/inode.c   | 235
++---
fs/btrfs/relocation.c  |  16 
3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -37,6 +37,7 @@
#include "math.h"
#include "sysfs.h"
#include "qgroup.h"
+#include "dedupe.h"

#undef SCRAMBLE_DELAYED_REFS

@@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct
btrfs_trans_handle *trans,

if (btrfs_delayed_ref_is_head(node)) {
struct btrfs_delayed_ref_head *head;
+struct btrfs_fs_info *fs_info = root->fs_info;
+
/*
 * we've hit the end of the chain and we were supposed
 * to insert this extent into the tree.  But, it got
@@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct
btrfs_trans_handle *trans,
btrfs_pin_extent(root, node->bytenr,
 node->num_bytes, 1);
if (head->is_data) {
+/*
+ * If insert_reserved is given, it means
+ * a new extent is revered, then deleted
+ * in one tran, and inc/dec get merged to 0.
+ *
+ * In this case, we need to remove its dedup
+ * hash.
+ */
+btrfs_dedupe_del(trans, fs_info, node->bytenr);
ret = btrfs_del_csums(trans, root,
  node->bytenr,
  node->num_bytes);
@@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct
btrfs_trans_handle *trans,
btrfs_release_path(path);

if (is_data) {
+ret = btrfs_dedupe_del(trans, info, bytenr);
+if (ret < 0) {
+btrfs_abort_transaction(trans, extent_root,
+ret);


I don't see why an error here should lead to a readonly fs.
   --Mark



Because such deletion error can lead to corruption.

For example, extent A is already in hash pool.
And when freeing extent A, we need to delete its hash, of course.

But if such deletion fails, which means the hash is still in the pool,
even the extent A no longer exists in extent tree.


Except if we're in in-memory mode only it doesn't matter, so don't abort
if we're in in-memory mode.  Thanks,

Josef



If we can't ensure a hash is delete along with the extent, we will
screw up the whole fs, as new write can points to non-exist extent.

Although you're right with in-memory mode here, we won't abort
trans, as inmem_del_hash() won't return error code. It will always
return 0.


Until a third party comes along and changes it to return an error code and
neither you or I are there to remind them to fix this check (or have simply
forgotten).



So still, no need to change anyway.


Personally I'd call this 'defensive coding' and do a check for in-memory
only before our abort_trans().  This would have no effect on our running
code but avoids the problem I stated above.  Alternatively, you could
clearly comment the exception. I don't like leaving it as-is for the reason
I stated above.

Thanks,
--Mark



The whole 'defensive coding' is here just because the V10 patchset comes 
with full function, including 2 backends and other things later.


A lot of code like this is here because we know what will be added later.
So this is true that it looks ridiculous until one knows there is an 
on-disk backend to be added.


I'm OK to move the check to btrfs_dedupe_del(), but this makes me 
curious about the correct coding style for adding new function.



If we have a clear view of the future functions , should we leave such 
interfaces for them?

Or add them when adding the new functions?

And what level of integration should be done inside btrfs codes?
Should any caller of an exported btrfs

Re: [PATCH v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-06 Thread Mark Fasheh 
On Sat, Jun 04, 2016 at 06:26:39PM +0800, Qu Wenruo wrote:
> 
> 
> On 06/03/2016 10:27 PM, Josef Bacik wrote:
> >On 06/01/2016 09:12 PM, Qu Wenruo wrote:
> >>
> >>
> >>At 06/02/2016 06:08 AM, Mark Fasheh wrote:
> >>>On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:
> Core implement for inband de-duplication.
> It reuse the async_cow_start() facility to do the calculate dedupe
> hash.
> And use dedupe hash to do inband de-duplication at extent level.
> 
> The work flow is as below:
> 1) Run delalloc range for an inode
> 2) Calculate hash for the delalloc range at the unit of dedupe_bs
> 3) For hash match(duplicated) case, just increase source extent ref
>    and insert file extent.
>    For hash mismatch case, go through the normal cow_file_range()
>    fallback, and add hash into dedupe_tree.
>    Compress for hash miss case is not supported yet.
> 
> Current implement restore all dedupe hash in memory rb-tree, with LRU
> behavior to control the limit.
> 
> Signed-off-by: Wang Xiaoguang 
> Signed-off-by: Qu Wenruo 
> ---
>  fs/btrfs/extent-tree.c |  18 
>  fs/btrfs/inode.c   | 235
> ++---
>  fs/btrfs/relocation.c  |  16 
>  3 files changed, 236 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 53e1297..dabd721 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -37,6 +37,7 @@
>  #include "math.h"
>  #include "sysfs.h"
>  #include "qgroup.h"
> +#include "dedupe.h"
> 
>  #undef SCRAMBLE_DELAYED_REFS
> 
> @@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct
> btrfs_trans_handle *trans,
> 
>  if (btrfs_delayed_ref_is_head(node)) {
>  struct btrfs_delayed_ref_head *head;
> +struct btrfs_fs_info *fs_info = root->fs_info;
> +
>  /*
>   * we've hit the end of the chain and we were supposed
>   * to insert this extent into the tree.  But, it got
> @@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct
> btrfs_trans_handle *trans,
>  btrfs_pin_extent(root, node->bytenr,
>   node->num_bytes, 1);
>  if (head->is_data) {
> +/*
> + * If insert_reserved is given, it means
> + * a new extent is revered, then deleted
> + * in one tran, and inc/dec get merged to 0.
> + *
> + * In this case, we need to remove its dedup
> + * hash.
> + */
> +btrfs_dedupe_del(trans, fs_info, node->bytenr);
>  ret = btrfs_del_csums(trans, root,
>    node->bytenr,
>    node->num_bytes);
> @@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct
> btrfs_trans_handle *trans,
>  btrfs_release_path(path);
> 
>  if (is_data) {
> +ret = btrfs_dedupe_del(trans, info, bytenr);
> +if (ret < 0) {
> +btrfs_abort_transaction(trans, extent_root,
> +ret);
> >>>
> >>>I don't see why an error here should lead to a readonly fs.
> >>>--Mark
> >>>
> >>
> >>Because such deletion error can lead to corruption.
> >>
> >>For example, extent A is already in hash pool.
> >>And when freeing extent A, we need to delete its hash, of course.
> >>
> >>But if such deletion fails, which means the hash is still in the pool,
> >>even the extent A no longer exists in extent tree.
> >
> >Except if we're in in-memory mode only it doesn't matter, so don't abort
> >if we're in in-memory mode.  Thanks,
> >
> >Josef
> >
> 
> If we can't ensure a hash is delete along with the extent, we will
> screw up the whole fs, as new write can points to non-exist extent.
> 
> Although you're right with in-memory mode here, we won't abort
> trans, as inmem_del_hash() won't return error code. It will always
> return 0.

Until a third party comes along and changes it to return an error code and
neither you or I are there to remind them to fix this check (or have simply
forgotten).


> So still, no need to change anyway.

Personally I'd call this 'defensive coding' and do a check for in-memory
only before our abort_trans().  This would have no effect on our running
code but avoids the problem I stated above.  Alternatively, you could
clearly comment the exception. I don't like leaving it as-is for the reason 
I stated above.

Thanks,
--Mark

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

Re: [PATCH v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-04 Thread Qu Wenruo 



On 06/03/2016 10:43 PM, Josef Bacik wrote:

On 04/01/2016 02:35 AM, Qu Wenruo wrote:

Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
   and insert file extent.
   For hash mismatch case, go through the normal cow_file_range()
   fallback, and add hash into dedupe_tree.
   Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/extent-tree.c |  18 
 fs/btrfs/inode.c   | 235
++---
 fs/btrfs/relocation.c  |  16 
 3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c






@@ -1076,6 +1135,68 @@ out_unlock:
 goto out;
 }

+static int hash_file_ranges(struct inode *inode, u64 start, u64 end,
+struct async_cow *async_cow, int *num_added)
+{
+struct btrfs_root *root = BTRFS_I(inode)->root;
+struct btrfs_fs_info *fs_info = root->fs_info;
+struct btrfs_dedupe_info *dedupe_info = fs_info->dedupe_info;
+struct page *locked_page = async_cow->locked_page;
+u16 hash_algo;
+u64 actual_end;
+u64 isize = i_size_read(inode);
+u64 dedupe_bs;
+u64 cur_offset = start;
+int ret = 0;
+
+actual_end = min_t(u64, isize, end + 1);
+/* If dedupe is not enabled, don't split extent into dedupe_bs */
+if (fs_info->dedupe_enabled && dedupe_info) {
+dedupe_bs = dedupe_info->blocksize;
+hash_algo = dedupe_info->hash_type;
+} else {
+dedupe_bs = SZ_128M;
+/* Just dummy, to avoid access NULL pointer */
+hash_algo = BTRFS_DEDUPE_HASH_SHA256;
+}
+
+while (cur_offset < end) {
+struct btrfs_dedupe_hash *hash = NULL;
+u64 len;
+
+len = min(end + 1 - cur_offset, dedupe_bs);
+if (len < dedupe_bs)
+goto next;
+
+hash = btrfs_dedupe_alloc_hash(hash_algo);
+if (!hash) {
+ret = -ENOMEM;
+goto out;
+}
+ret = btrfs_dedupe_calc_hash(fs_info, inode, cur_offset, hash);
+if (ret < 0)
+goto out;
+
+ret = btrfs_dedupe_search(fs_info, inode, cur_offset, hash);
+if (ret < 0)
+goto out;


You leak hash in both of these cases.  Also if btrfs_dedup_search




+if (ret < 0)
+goto out_qgroup;
+
+/*
+ * Hash hit won't create a new data extent, so its reserved quota
+ * space won't be freed by new delayed_ref_head.
+ * Need to free it here.
+ */
+if (btrfs_dedupe_hash_hit(hash))
+btrfs_qgroup_free_data(inode, file_pos, ram_bytes);
+
+/* Add missed hash into dedupe tree */
+if (hash && hash->bytenr == 0) {
+hash->bytenr = ins.objectid;
+hash->num_bytes = ins.offset;
+ret = btrfs_dedupe_add(trans, root->fs_info, hash);


I don't want to flip read only if we fail this in the in-memory mode.
Thanks,

Josef


Right, unlike btrfs_dedupe_del() case, if we fail to insert hash, 
nothing wrong will happen.

We would just slightly reduce the dedupe rate.

I'm OK to skip dedupe_add() error.

Thanks,
Qu

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

--
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 v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-04 Thread Qu Wenruo 



On 06/03/2016 10:27 PM, Josef Bacik wrote:

On 06/01/2016 09:12 PM, Qu Wenruo wrote:



At 06/02/2016 06:08 AM, Mark Fasheh wrote:

On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:

Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe
hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
   and insert file extent.
   For hash mismatch case, go through the normal cow_file_range()
   fallback, and add hash into dedupe_tree.
   Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/extent-tree.c |  18 
 fs/btrfs/inode.c   | 235
++---
 fs/btrfs/relocation.c  |  16 
 3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -37,6 +37,7 @@
 #include "math.h"
 #include "sysfs.h"
 #include "qgroup.h"
+#include "dedupe.h"

 #undef SCRAMBLE_DELAYED_REFS

@@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct
btrfs_trans_handle *trans,

 if (btrfs_delayed_ref_is_head(node)) {
 struct btrfs_delayed_ref_head *head;
+struct btrfs_fs_info *fs_info = root->fs_info;
+
 /*
  * we've hit the end of the chain and we were supposed
  * to insert this extent into the tree.  But, it got
@@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct
btrfs_trans_handle *trans,
 btrfs_pin_extent(root, node->bytenr,
  node->num_bytes, 1);
 if (head->is_data) {
+/*
+ * If insert_reserved is given, it means
+ * a new extent is revered, then deleted
+ * in one tran, and inc/dec get merged to 0.
+ *
+ * In this case, we need to remove its dedup
+ * hash.
+ */
+btrfs_dedupe_del(trans, fs_info, node->bytenr);
 ret = btrfs_del_csums(trans, root,
   node->bytenr,
   node->num_bytes);
@@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct
btrfs_trans_handle *trans,
 btrfs_release_path(path);

 if (is_data) {
+ret = btrfs_dedupe_del(trans, info, bytenr);
+if (ret < 0) {
+btrfs_abort_transaction(trans, extent_root,
+ret);


I don't see why an error here should lead to a readonly fs.
--Mark



Because such deletion error can lead to corruption.

For example, extent A is already in hash pool.
And when freeing extent A, we need to delete its hash, of course.

But if such deletion fails, which means the hash is still in the pool,
even the extent A no longer exists in extent tree.


Except if we're in in-memory mode only it doesn't matter, so don't abort
if we're in in-memory mode.  Thanks,

Josef



If we can't ensure a hash is delete along with the extent, we will screw 
up the whole fs, as new write can points to non-exist extent.


Although you're right with in-memory mode here, we won't abort trans, as 
inmem_del_hash() won't return error code. It will always return 0.


So still, no need to change anyway.

Thanks,
Qu


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

--
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 v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-03 Thread Josef Bacik 

On 04/01/2016 02:35 AM, Qu Wenruo wrote:

Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
   and insert file extent.
   For hash mismatch case, go through the normal cow_file_range()
   fallback, and add hash into dedupe_tree.
   Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/extent-tree.c |  18 
 fs/btrfs/inode.c   | 235 ++---
 fs/btrfs/relocation.c  |  16 
 3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c






@@ -1076,6 +1135,68 @@ out_unlock:
goto out;
 }

+static int hash_file_ranges(struct inode *inode, u64 start, u64 end,
+   struct async_cow *async_cow, int *num_added)
+{
+   struct btrfs_root *root = BTRFS_I(inode)->root;
+   struct btrfs_fs_info *fs_info = root->fs_info;
+   struct btrfs_dedupe_info *dedupe_info = fs_info->dedupe_info;
+   struct page *locked_page = async_cow->locked_page;
+   u16 hash_algo;
+   u64 actual_end;
+   u64 isize = i_size_read(inode);
+   u64 dedupe_bs;
+   u64 cur_offset = start;
+   int ret = 0;
+
+   actual_end = min_t(u64, isize, end + 1);
+   /* If dedupe is not enabled, don't split extent into dedupe_bs */
+   if (fs_info->dedupe_enabled && dedupe_info) {
+   dedupe_bs = dedupe_info->blocksize;
+   hash_algo = dedupe_info->hash_type;
+   } else {
+   dedupe_bs = SZ_128M;
+   /* Just dummy, to avoid access NULL pointer */
+   hash_algo = BTRFS_DEDUPE_HASH_SHA256;
+   }
+
+   while (cur_offset < end) {
+   struct btrfs_dedupe_hash *hash = NULL;
+   u64 len;
+
+   len = min(end + 1 - cur_offset, dedupe_bs);
+   if (len < dedupe_bs)
+   goto next;
+
+   hash = btrfs_dedupe_alloc_hash(hash_algo);
+   if (!hash) {
+   ret = -ENOMEM;
+   goto out;
+   }
+   ret = btrfs_dedupe_calc_hash(fs_info, inode, cur_offset, hash);
+   if (ret < 0)
+   goto out;
+
+   ret = btrfs_dedupe_search(fs_info, inode, cur_offset, hash);
+   if (ret < 0)
+   goto out;


You leak hash in both of these cases.  Also if btrfs_dedup_search




+   if (ret < 0)
+   goto out_qgroup;
+
+   /*
+* Hash hit won't create a new data extent, so its reserved quota
+* space won't be freed by new delayed_ref_head.
+* Need to free it here.
+*/
+   if (btrfs_dedupe_hash_hit(hash))
+   btrfs_qgroup_free_data(inode, file_pos, ram_bytes);
+
+   /* Add missed hash into dedupe tree */
+   if (hash && hash->bytenr == 0) {
+   hash->bytenr = ins.objectid;
+   hash->num_bytes = ins.offset;
+   ret = btrfs_dedupe_add(trans, root->fs_info, hash);


I don't want to flip read only if we fail this in the in-memory mode. 
Thanks,


Josef
--
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 v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-03 Thread Josef Bacik 

On 06/01/2016 09:12 PM, Qu Wenruo wrote:



At 06/02/2016 06:08 AM, Mark Fasheh wrote:

On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:

Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
   and insert file extent.
   For hash mismatch case, go through the normal cow_file_range()
   fallback, and add hash into dedupe_tree.
   Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/extent-tree.c |  18 
 fs/btrfs/inode.c   | 235
++---
 fs/btrfs/relocation.c  |  16 
 3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -37,6 +37,7 @@
 #include "math.h"
 #include "sysfs.h"
 #include "qgroup.h"
+#include "dedupe.h"

 #undef SCRAMBLE_DELAYED_REFS

@@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct
btrfs_trans_handle *trans,

 if (btrfs_delayed_ref_is_head(node)) {
 struct btrfs_delayed_ref_head *head;
+struct btrfs_fs_info *fs_info = root->fs_info;
+
 /*
  * we've hit the end of the chain and we were supposed
  * to insert this extent into the tree.  But, it got
@@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct
btrfs_trans_handle *trans,
 btrfs_pin_extent(root, node->bytenr,
  node->num_bytes, 1);
 if (head->is_data) {
+/*
+ * If insert_reserved is given, it means
+ * a new extent is revered, then deleted
+ * in one tran, and inc/dec get merged to 0.
+ *
+ * In this case, we need to remove its dedup
+ * hash.
+ */
+btrfs_dedupe_del(trans, fs_info, node->bytenr);
 ret = btrfs_del_csums(trans, root,
   node->bytenr,
   node->num_bytes);
@@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct
btrfs_trans_handle *trans,
 btrfs_release_path(path);

 if (is_data) {
+ret = btrfs_dedupe_del(trans, info, bytenr);
+if (ret < 0) {
+btrfs_abort_transaction(trans, extent_root,
+ret);


I don't see why an error here should lead to a readonly fs.
--Mark



Because such deletion error can lead to corruption.

For example, extent A is already in hash pool.
And when freeing extent A, we need to delete its hash, of course.

But if such deletion fails, which means the hash is still in the pool,
even the extent A no longer exists in extent tree.


Except if we're in in-memory mode only it doesn't matter, so don't abort 
if we're in in-memory mode.  Thanks,


Josef

--
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 v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-01 Thread Qu Wenruo 



At 06/02/2016 06:08 AM, Mark Fasheh wrote:

On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:

Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
   and insert file extent.
   For hash mismatch case, go through the normal cow_file_range()
   fallback, and add hash into dedupe_tree.
   Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/extent-tree.c |  18 
 fs/btrfs/inode.c   | 235 ++---
 fs/btrfs/relocation.c  |  16 
 3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -37,6 +37,7 @@
 #include "math.h"
 #include "sysfs.h"
 #include "qgroup.h"
+#include "dedupe.h"

 #undef SCRAMBLE_DELAYED_REFS

@@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct btrfs_trans_handle 
*trans,

if (btrfs_delayed_ref_is_head(node)) {
struct btrfs_delayed_ref_head *head;
+   struct btrfs_fs_info *fs_info = root->fs_info;
+
/*
 * we've hit the end of the chain and we were supposed
 * to insert this extent into the tree.  But, it got
@@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct btrfs_trans_handle 
*trans,
btrfs_pin_extent(root, node->bytenr,
 node->num_bytes, 1);
if (head->is_data) {
+   /*
+* If insert_reserved is given, it means
+* a new extent is revered, then deleted
+* in one tran, and inc/dec get merged to 0.
+*
+* In this case, we need to remove its dedup
+* hash.
+*/
+   btrfs_dedupe_del(trans, fs_info, node->bytenr);
ret = btrfs_del_csums(trans, root,
  node->bytenr,
  node->num_bytes);
@@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
btrfs_release_path(path);

if (is_data) {
+   ret = btrfs_dedupe_del(trans, info, bytenr);
+   if (ret < 0) {
+   btrfs_abort_transaction(trans, extent_root,
+   ret);


I don't see why an error here should lead to a readonly fs.
--Mark



Because such deletion error can lead to corruption.

For example, extent A is already in hash pool.
And when freeing extent A, we need to delete its hash, of course.

But if such deletion fails, which means the hash is still in the pool, 
even the extent A no longer exists in extent tree.


If we don't abort trans here, next dedupe write may points to the 
non-exist extent A, and cause corruption.


Thanks,
Qu

--
Mark Fasheh





--
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 v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-06-01 Thread Mark Fasheh 
On Fri, Apr 01, 2016 at 02:35:00PM +0800, Qu Wenruo wrote:
> Core implement for inband de-duplication.
> It reuse the async_cow_start() facility to do the calculate dedupe hash.
> And use dedupe hash to do inband de-duplication at extent level.
> 
> The work flow is as below:
> 1) Run delalloc range for an inode
> 2) Calculate hash for the delalloc range at the unit of dedupe_bs
> 3) For hash match(duplicated) case, just increase source extent ref
>and insert file extent.
>For hash mismatch case, go through the normal cow_file_range()
>fallback, and add hash into dedupe_tree.
>Compress for hash miss case is not supported yet.
> 
> Current implement restore all dedupe hash in memory rb-tree, with LRU
> behavior to control the limit.
> 
> Signed-off-by: Wang Xiaoguang 
> Signed-off-by: Qu Wenruo 
> ---
>  fs/btrfs/extent-tree.c |  18 
>  fs/btrfs/inode.c   | 235 
> ++---
>  fs/btrfs/relocation.c  |  16 
>  3 files changed, 236 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 53e1297..dabd721 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -37,6 +37,7 @@
>  #include "math.h"
>  #include "sysfs.h"
>  #include "qgroup.h"
> +#include "dedupe.h"
>  
>  #undef SCRAMBLE_DELAYED_REFS
>  
> @@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct 
> btrfs_trans_handle *trans,
>  
>   if (btrfs_delayed_ref_is_head(node)) {
>   struct btrfs_delayed_ref_head *head;
> + struct btrfs_fs_info *fs_info = root->fs_info;
> +
>   /*
>* we've hit the end of the chain and we were supposed
>* to insert this extent into the tree.  But, it got
> @@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct 
> btrfs_trans_handle *trans,
>   btrfs_pin_extent(root, node->bytenr,
>node->num_bytes, 1);
>   if (head->is_data) {
> + /*
> +  * If insert_reserved is given, it means
> +  * a new extent is revered, then deleted
> +  * in one tran, and inc/dec get merged to 0.
> +  *
> +  * In this case, we need to remove its dedup
> +  * hash.
> +  */
> + btrfs_dedupe_del(trans, fs_info, node->bytenr);
>   ret = btrfs_del_csums(trans, root,
> node->bytenr,
> node->num_bytes);
> @@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct 
> btrfs_trans_handle *trans,
>   btrfs_release_path(path);
>  
>   if (is_data) {
> + ret = btrfs_dedupe_del(trans, info, bytenr);
> + if (ret < 0) {
> + btrfs_abort_transaction(trans, extent_root,
> + ret);

I don't see why an error here should lead to a readonly fs.
--Mark

--
Mark Fasheh
--
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 v10 09/21] btrfs: dedupe: Inband in-memory only de-duplication implement

2016-04-01 Thread Qu Wenruo 
Core implement for inband de-duplication.
It reuse the async_cow_start() facility to do the calculate dedupe hash.
And use dedupe hash to do inband de-duplication at extent level.

The work flow is as below:
1) Run delalloc range for an inode
2) Calculate hash for the delalloc range at the unit of dedupe_bs
3) For hash match(duplicated) case, just increase source extent ref
   and insert file extent.
   For hash mismatch case, go through the normal cow_file_range()
   fallback, and add hash into dedupe_tree.
   Compress for hash miss case is not supported yet.

Current implement restore all dedupe hash in memory rb-tree, with LRU
behavior to control the limit.

Signed-off-by: Wang Xiaoguang 
Signed-off-by: Qu Wenruo 
---
 fs/btrfs/extent-tree.c |  18 
 fs/btrfs/inode.c   | 235 ++---
 fs/btrfs/relocation.c  |  16 
 3 files changed, 236 insertions(+), 33 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 53e1297..dabd721 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -37,6 +37,7 @@
 #include "math.h"
 #include "sysfs.h"
 #include "qgroup.h"
+#include "dedupe.h"
 
 #undef SCRAMBLE_DELAYED_REFS
 
@@ -2399,6 +2400,8 @@ static int run_one_delayed_ref(struct btrfs_trans_handle 
*trans,
 
if (btrfs_delayed_ref_is_head(node)) {
struct btrfs_delayed_ref_head *head;
+   struct btrfs_fs_info *fs_info = root->fs_info;
+
/*
 * we've hit the end of the chain and we were supposed
 * to insert this extent into the tree.  But, it got
@@ -2413,6 +2416,15 @@ static int run_one_delayed_ref(struct btrfs_trans_handle 
*trans,
btrfs_pin_extent(root, node->bytenr,
 node->num_bytes, 1);
if (head->is_data) {
+   /*
+* If insert_reserved is given, it means
+* a new extent is revered, then deleted
+* in one tran, and inc/dec get merged to 0.
+*
+* In this case, we need to remove its dedup
+* hash.
+*/
+   btrfs_dedupe_del(trans, fs_info, node->bytenr);
ret = btrfs_del_csums(trans, root,
  node->bytenr,
  node->num_bytes);
@@ -6713,6 +6725,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle 
*trans,
btrfs_release_path(path);
 
if (is_data) {
+   ret = btrfs_dedupe_del(trans, info, bytenr);
+   if (ret < 0) {
+   btrfs_abort_transaction(trans, extent_root,
+   ret);
+   goto out;
+   }
ret = btrfs_del_csums(trans, root, bytenr, num_bytes);
if (ret) {
btrfs_abort_transaction(trans, extent_root, 
ret);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 41a5688..96790d0 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -60,6 +60,7 @@
 #include "hash.h"
 #include "props.h"
 #include "qgroup.h"
+#include "dedupe.h"
 
 struct btrfs_iget_args {
struct btrfs_key *location;
@@ -106,7 +107,8 @@ static int btrfs_finish_ordered_io(struct 
btrfs_ordered_extent *ordered_extent);
 static noinline int cow_file_range(struct inode *inode,
   struct page *locked_page,
   u64 start, u64 end, int *page_started,
-  unsigned long *nr_written, int unlock);
+  unsigned long *nr_written, int unlock,
+  struct btrfs_dedupe_hash *hash);
 static struct extent_map *create_pinned_em(struct inode *inode, u64 start,
   u64 len, u64 orig_start,
   u64 block_start, u64 block_len,
@@ -335,6 +337,7 @@ struct async_extent {
struct page **pages;
unsigned long nr_pages;
int compress_type;
+   struct btrfs_dedupe_hash *hash;
struct list_head list;
 };
 
@@ -353,7 +356,8 @@ static noinline int add_async_extent(struct async_cow *cow,
 u64 compressed_size,
 struct page **pages,
 unsigned long nr_pages,
-int compress_type)
+int compress_type,
+struct