[PATCH] btrfs: fallback to vmalloc in btrfs_compare_tree

[PATCH] btrfs: fallback to vmalloc in btrfs_compare_tree

[David Sterba]

The allocation of node could fail if the memory is too fragmented for a
given node size, practically observed with 64k.

http://article.gmane.org/gmane.comp.file-systems.btrfs/54689

Reported-by: Jean-Denis Girard <jd.girard@sysnux.pf>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 77592931ab4f..ec7928a27aaa 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -19,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/rbtree.h>
+#include <linux/vmalloc.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -5361,10 +5362,13 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
 		goto out;
 	}
 
-	tmp_buf = kmalloc(left_root->nodesize, GFP_KERNEL);
+	tmp_buf = kmalloc(left_root->nodesize, GFP_KERNEL | __GFP_NOWARN);
 	if (!tmp_buf) {
-		ret = -ENOMEM;
-		goto out;
+		tmp_buf = vmalloc(left_root->nodesize);
+		if (!tmp_buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
 	}
 
 	left_path->search_commit_root = 1;
@@ -5565,7 +5569,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
 out:
 	btrfs_free_path(left_path);
 	btrfs_free_path(right_path);
-	kfree(tmp_buf);
+	kvfree(tmp_buf);
 	return ret;
 }
 
-- 
2.7.1

Re: [PATCH] btrfs: fallback to vmalloc in btrfs_compare_tree

[Liu Bo]

On Wed, Mar 30, 2016 at 04:05:43PM +0200, David Sterba wrote:
> The allocation of node could fail if the memory is too fragmented for a
> given node size, practically observed with 64k.

It's not a critical path.  Why not use vmalloc directly?

Thanks,

-liubo

> 
> http://article.gmane.org/gmane.comp.file-systems.btrfs/54689
> 
> Reported-by: Jean-Denis Girard <jd.girard@sysnux.pf>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 77592931ab4f..ec7928a27aaa 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -19,6 +19,7 @@
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/rbtree.h>
> +#include <linux/vmalloc.h>
>  #include "ctree.h"
>  #include "disk-io.h"
>  #include "transaction.h"
> @@ -5361,10 +5362,13 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
>  		goto out;
>  	}
>  
> -	tmp_buf = kmalloc(left_root->nodesize, GFP_KERNEL);
> +	tmp_buf = kmalloc(left_root->nodesize, GFP_KERNEL | __GFP_NOWARN);
>  	if (!tmp_buf) {
> -		ret = -ENOMEM;
> -		goto out;
> +		tmp_buf = vmalloc(left_root->nodesize);
> +		if (!tmp_buf) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  	}
>  
>  	left_path->search_commit_root = 1;
> @@ -5565,7 +5569,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
>  out:
>  	btrfs_free_path(left_path);
>  	btrfs_free_path(right_path);
> -	kfree(tmp_buf);
> +	kvfree(tmp_buf);
>  	return ret;
>  }
>  
> -- 
> 2.7.1
> 
> --
> 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

Re: [PATCH] btrfs: fallback to vmalloc in btrfs_compare_tree

[David Sterba]

On Wed, Mar 30, 2016 at 10:10:45AM -0700, Liu Bo wrote:
> On Wed, Mar 30, 2016 at 04:05:43PM +0200, David Sterba wrote:
> > The allocation of node could fail if the memory is too fragmented for a
> > given node size, practically observed with 64k.
> 
> It's not a critical path.  Why not use vmalloc directly?

We should try to avoid vmalloc if possible.

Re: [PATCH] btrfs: fallback to vmalloc in btrfs_compare_tree

[Jean-Denis Girard]

Hi David,

Le 30/03/2016 04:05, David Sterba a écrit :
> The allocation of node could fail if the memory is too fragmented for a
> given node size, practically observed with 64k.
> 
> http://article.gmane.org/gmane.comp.file-systems.btrfs/54689
> 
> Reported-by: Jean-Denis Girard <jd.girard@sysnux.pf>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 77592931ab4f..ec7928a27aaa 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -19,6 +19,7 @@
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/rbtree.h>
> +#include <linux/vmalloc.h>
>  #include "ctree.h"
>  #include "disk-io.h"
>  #include "transaction.h"
> @@ -5361,10 +5362,13 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
>  		goto out;
>  	}
>  
> -	tmp_buf = kmalloc(left_root->nodesize, GFP_KERNEL);
> +	tmp_buf = kmalloc(left_root->nodesize, GFP_KERNEL | __GFP_NOWARN);
>  	if (!tmp_buf) {
> -		ret = -ENOMEM;
> -		goto out;
> +		tmp_buf = vmalloc(left_root->nodesize);
> +		if (!tmp_buf) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  	}
>  
>  	left_path->search_commit_root = 1;
> @@ -5565,7 +5569,7 @@ int btrfs_compare_trees(struct btrfs_root *left_root,
>  out:
>  	btrfs_free_path(left_path);
>  	btrfs_free_path(right_path);
> -	kfree(tmp_buf);
> +	kvfree(tmp_buf);
>  	return ret;
>  }
>  
> 

I adapted / applied the patch for kernel-4.4.6, rebooted and now the
backup completes without error, thanks a lot!

Tested by:  Jean-Denis Girard <jd.girard@sysnux.pf>


Thanks,
-- 
Jean-Denis Girard

SysNux                Systèmes   Linux   en   Polynésie   française
http://www.sysnux.pf/ Tél: +689 40.50.10.40 / GSM: +689 87.79.75.27