[v10,04/21] btrfs: dedupe: Introduce function to remove hash from in-memory tree
diff mbox

Message ID 1459492512-31435-5-git-send-email-quwenruo@cn.fujitsu.com
State New
Headers show

Commit Message

Qu Wenruo April 1, 2016, 6:34 a.m. UTC
From: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>

Introduce static function inmem_del() to remove hash from in-memory
dedupe tree.
And implement btrfs_dedupe_del() and btrfs_dedup_destroy() interfaces.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
---
 fs/btrfs/dedupe.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

Comments

Mark Fasheh June 1, 2016, 7:40 p.m. UTC | #1
On Fri, Apr 01, 2016 at 02:34:55PM +0800, Qu Wenruo wrote:
> From: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
> 
> Introduce static function inmem_del() to remove hash from in-memory
> dedupe tree.
> And implement btrfs_dedupe_del() and btrfs_dedup_destroy() interfaces.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
> ---
>  fs/btrfs/dedupe.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 
> diff --git a/fs/btrfs/dedupe.c b/fs/btrfs/dedupe.c
> index 4e8455e..a229ded 100644
> --- a/fs/btrfs/dedupe.c
> +++ b/fs/btrfs/dedupe.c
> @@ -303,3 +303,108 @@ int btrfs_dedupe_add(struct btrfs_trans_handle *trans,
>  		return inmem_add(dedupe_info, hash);
>  	return -EINVAL;
>  }
> +
> +static struct inmem_hash *
> +inmem_search_bytenr(struct btrfs_dedupe_info *dedupe_info, u64 bytenr)
> +{
> +	struct rb_node **p = &dedupe_info->bytenr_root.rb_node;
> +	struct rb_node *parent = NULL;
> +	struct inmem_hash *entry = NULL;
> +
> +	while (*p) {
> +		parent = *p;
> +		entry = rb_entry(parent, struct inmem_hash, bytenr_node);
> +
> +		if (bytenr < entry->bytenr)
> +			p = &(*p)->rb_left;
> +		else if (bytenr > entry->bytenr)
> +			p = &(*p)->rb_right;
> +		else
> +			return entry;
> +	}
> +
> +	return NULL;
> +}
> +
> +/* Delete a hash from in-memory dedupe tree */
> +static int inmem_del(struct btrfs_dedupe_info *dedupe_info, u64 bytenr)
> +{
> +	struct inmem_hash *hash;
> +
> +	mutex_lock(&dedupe_info->lock);
> +	hash = inmem_search_bytenr(dedupe_info, bytenr);
> +	if (!hash) {
> +		mutex_unlock(&dedupe_info->lock);
> +		return 0;
> +	}
> +
> +	__inmem_del(dedupe_info, hash);
> +	mutex_unlock(&dedupe_info->lock);
> +	return 0;
> +}
> +
> +/* Remove a dedupe hash from dedupe tree */
> +int btrfs_dedupe_del(struct btrfs_trans_handle *trans,
> +		     struct btrfs_fs_info *fs_info, u64 bytenr)
> +{
> +	struct btrfs_dedupe_info *dedupe_info = fs_info->dedupe_info;
> +
> +	if (!fs_info->dedupe_enabled)
> +		return 0;
> +
> +	if (WARN_ON(dedupe_info == NULL))
> +		return -EINVAL;
> +
> +	if (dedupe_info->backend == BTRFS_DEDUPE_BACKEND_INMEMORY)
> +		return inmem_del(dedupe_info, bytenr);
> +	return -EINVAL;
> +}
> +
> +static void inmem_destroy(struct btrfs_dedupe_info *dedupe_info)
> +{
> +	struct inmem_hash *entry, *tmp;
> +
> +	mutex_lock(&dedupe_info->lock);
> +	list_for_each_entry_safe(entry, tmp, &dedupe_info->lru_list, lru_list)
> +		__inmem_del(dedupe_info, entry);
> +	mutex_unlock(&dedupe_info->lock);
> +}
> +
> +int btrfs_dedupe_disable(struct btrfs_fs_info *fs_info)
> +{
> +	struct btrfs_dedupe_info *dedupe_info;
> +	int ret;
> +
> +	/* Here we don't want to increase refs of dedupe_info */
> +	fs_info->dedupe_enabled = 0;

Can this clear of fs_info->dedupe_enabled race with another thread in write?
I don't see any locking (but perhaps that comes in a later patch).
	--Mark

--
Mark Fasheh
--
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
Qu Wenruo June 2, 2016, 1:01 a.m. UTC | #2
At 06/02/2016 03:40 AM, Mark Fasheh wrote:
> On Fri, Apr 01, 2016 at 02:34:55PM +0800, Qu Wenruo wrote:
>> From: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
>>
>> Introduce static function inmem_del() to remove hash from in-memory
>> dedupe tree.
>> And implement btrfs_dedupe_del() and btrfs_dedup_destroy() interfaces.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> Signed-off-by: Wang Xiaoguang <wangxg.fnst@cn.fujitsu.com>
>> ---
>>  fs/btrfs/dedupe.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 105 insertions(+)
>>
>> diff --git a/fs/btrfs/dedupe.c b/fs/btrfs/dedupe.c
>> index 4e8455e..a229ded 100644
>> --- a/fs/btrfs/dedupe.c
>> +++ b/fs/btrfs/dedupe.c
>> @@ -303,3 +303,108 @@ int btrfs_dedupe_add(struct btrfs_trans_handle *trans,
>>  		return inmem_add(dedupe_info, hash);
>>  	return -EINVAL;
>>  }
>> +
>> +static struct inmem_hash *
>> +inmem_search_bytenr(struct btrfs_dedupe_info *dedupe_info, u64 bytenr)
>> +{
>> +	struct rb_node **p = &dedupe_info->bytenr_root.rb_node;
>> +	struct rb_node *parent = NULL;
>> +	struct inmem_hash *entry = NULL;
>> +
>> +	while (*p) {
>> +		parent = *p;
>> +		entry = rb_entry(parent, struct inmem_hash, bytenr_node);
>> +
>> +		if (bytenr < entry->bytenr)
>> +			p = &(*p)->rb_left;
>> +		else if (bytenr > entry->bytenr)
>> +			p = &(*p)->rb_right;
>> +		else
>> +			return entry;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +/* Delete a hash from in-memory dedupe tree */
>> +static int inmem_del(struct btrfs_dedupe_info *dedupe_info, u64 bytenr)
>> +{
>> +	struct inmem_hash *hash;
>> +
>> +	mutex_lock(&dedupe_info->lock);
>> +	hash = inmem_search_bytenr(dedupe_info, bytenr);
>> +	if (!hash) {
>> +		mutex_unlock(&dedupe_info->lock);
>> +		return 0;
>> +	}
>> +
>> +	__inmem_del(dedupe_info, hash);
>> +	mutex_unlock(&dedupe_info->lock);
>> +	return 0;
>> +}
>> +
>> +/* Remove a dedupe hash from dedupe tree */
>> +int btrfs_dedupe_del(struct btrfs_trans_handle *trans,
>> +		     struct btrfs_fs_info *fs_info, u64 bytenr)
>> +{
>> +	struct btrfs_dedupe_info *dedupe_info = fs_info->dedupe_info;
>> +
>> +	if (!fs_info->dedupe_enabled)
>> +		return 0;
>> +
>> +	if (WARN_ON(dedupe_info == NULL))
>> +		return -EINVAL;
>> +
>> +	if (dedupe_info->backend == BTRFS_DEDUPE_BACKEND_INMEMORY)
>> +		return inmem_del(dedupe_info, bytenr);
>> +	return -EINVAL;
>> +}
>> +
>> +static void inmem_destroy(struct btrfs_dedupe_info *dedupe_info)
>> +{
>> +	struct inmem_hash *entry, *tmp;
>> +
>> +	mutex_lock(&dedupe_info->lock);
>> +	list_for_each_entry_safe(entry, tmp, &dedupe_info->lru_list, lru_list)
>> +		__inmem_del(dedupe_info, entry);
>> +	mutex_unlock(&dedupe_info->lock);
>> +}
>> +
>> +int btrfs_dedupe_disable(struct btrfs_fs_info *fs_info)
>> +{
>> +	struct btrfs_dedupe_info *dedupe_info;
>> +	int ret;
>> +
>> +	/* Here we don't want to increase refs of dedupe_info */
>> +	fs_info->dedupe_enabled = 0;
>
> Can this clear of fs_info->dedupe_enabled race with another thread in write?
> I don't see any locking (but perhaps that comes in a later patch).
> 	--Mark

Here we use sync fs to ensure no dedupe write will happen later.
(No need to ensure current transaction will go through dedupe)

Different buffered writer may get different dedupe_enabled flag, but 
whether we do dedupe is not determined by the flag at buffered write 
time.(For this V10 patch)

Instead it's determined by the flag when we run delalloc range.
And we use sync_fs() to write all dirty pages, ensuring no dedupe write 
will happen later, then freeing dedupe_info.

So it's should be fine, and test cases like btrfs/200(not mainlined) 
will test it.


Although the behavior may change a little in V11, to handle ENOSPC problem.

In next version, whether doing dedupe is determined at buffered write 
time, and restore the result to io_tree, to handle metadata reservation 
better.
And still use sync_fs() to ensure no other race.

Thanks,
Qu

>
> --
> Mark Fasheh
>
>


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

Patch
diff mbox

diff --git a/fs/btrfs/dedupe.c b/fs/btrfs/dedupe.c
index 4e8455e..a229ded 100644
--- a/fs/btrfs/dedupe.c
+++ b/fs/btrfs/dedupe.c
@@ -303,3 +303,108 @@  int btrfs_dedupe_add(struct btrfs_trans_handle *trans,
 		return inmem_add(dedupe_info, hash);
 	return -EINVAL;
 }
+
+static struct inmem_hash *
+inmem_search_bytenr(struct btrfs_dedupe_info *dedupe_info, u64 bytenr)
+{
+	struct rb_node **p = &dedupe_info->bytenr_root.rb_node;
+	struct rb_node *parent = NULL;
+	struct inmem_hash *entry = NULL;
+
+	while (*p) {
+		parent = *p;
+		entry = rb_entry(parent, struct inmem_hash, bytenr_node);
+
+		if (bytenr < entry->bytenr)
+			p = &(*p)->rb_left;
+		else if (bytenr > entry->bytenr)
+			p = &(*p)->rb_right;
+		else
+			return entry;
+	}
+
+	return NULL;
+}
+
+/* Delete a hash from in-memory dedupe tree */
+static int inmem_del(struct btrfs_dedupe_info *dedupe_info, u64 bytenr)
+{
+	struct inmem_hash *hash;
+
+	mutex_lock(&dedupe_info->lock);
+	hash = inmem_search_bytenr(dedupe_info, bytenr);
+	if (!hash) {
+		mutex_unlock(&dedupe_info->lock);
+		return 0;
+	}
+
+	__inmem_del(dedupe_info, hash);
+	mutex_unlock(&dedupe_info->lock);
+	return 0;
+}
+
+/* Remove a dedupe hash from dedupe tree */
+int btrfs_dedupe_del(struct btrfs_trans_handle *trans,
+		     struct btrfs_fs_info *fs_info, u64 bytenr)
+{
+	struct btrfs_dedupe_info *dedupe_info = fs_info->dedupe_info;
+
+	if (!fs_info->dedupe_enabled)
+		return 0;
+
+	if (WARN_ON(dedupe_info == NULL))
+		return -EINVAL;
+
+	if (dedupe_info->backend == BTRFS_DEDUPE_BACKEND_INMEMORY)
+		return inmem_del(dedupe_info, bytenr);
+	return -EINVAL;
+}
+
+static void inmem_destroy(struct btrfs_dedupe_info *dedupe_info)
+{
+	struct inmem_hash *entry, *tmp;
+
+	mutex_lock(&dedupe_info->lock);
+	list_for_each_entry_safe(entry, tmp, &dedupe_info->lru_list, lru_list)
+		__inmem_del(dedupe_info, entry);
+	mutex_unlock(&dedupe_info->lock);
+}
+
+int btrfs_dedupe_disable(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_dedupe_info *dedupe_info;
+	int ret;
+
+	/* Here we don't want to increase refs of dedupe_info */
+	fs_info->dedupe_enabled = 0;
+
+	dedupe_info = fs_info->dedupe_info;
+
+	if (!dedupe_info)
+		return 0;
+
+	/* Don't allow disable status change in RO mount */
+	if (fs_info->sb->s_flags & MS_RDONLY)
+		return -EROFS;
+
+	/*
+	 * Wait for all unfinished write to complete dedupe routine
+	 * As disable operation is not a frequent operation, we are
+	 * OK to use heavy but safe sync_filesystem().
+	 */
+	down_read(&fs_info->sb->s_umount);
+	ret = sync_filesystem(fs_info->sb);
+	up_read(&fs_info->sb->s_umount);
+	if (ret < 0)
+		return ret;
+
+	fs_info->dedupe_info = NULL;
+
+	/* now we are OK to clean up everything */
+	if (dedupe_info->backend == BTRFS_DEDUPE_BACKEND_INMEMORY)
+		inmem_destroy(dedupe_info);
+
+	crypto_free_shash(dedupe_info->dedupe_driver);
+	kfree(dedupe_info);
+	return 0;
+}