[PATCH 0/5] More vmalloc cleanups

[PATCH 0/5] More vmalloc cleanups

[David Sterba]

All vmalloc/vzalloc calls should be addressed, in this or the currently pending
patches.

David Sterba (5):
  btrfs: replace opencoded kvzalloc with the helper
  btrfs: send: use kvmalloc in iterate_dir_item
  btrfs: scrub: add memalloc_nofs protection around init_ipath
  btrfs: use GFP_KERNEL in init_ipath
  btrfs: adjust includes after vmalloc removal

 fs/btrfs/backref.c         | 10 +++++-----
 fs/btrfs/check-integrity.c | 11 ++++-------
 fs/btrfs/ctree.c           |  2 +-
 fs/btrfs/ioctl.c           |  4 ++--
 fs/btrfs/raid56.c          | 11 ++++-------
 fs/btrfs/scrub.c           |  9 +++++++++
 fs/btrfs/send.c            | 11 ++++-------
 7 files changed, 29 insertions(+), 29 deletions(-)

-- 
2.12.0

[PATCH 1/5] btrfs: replace opencoded kvzalloc with the helper

[David Sterba]

The logic of kmalloc and vmalloc fallback is open coded in
several places, we can now use the existing helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/check-integrity.c | 11 ++++-------
 fs/btrfs/raid56.c          | 11 ++++-------
 fs/btrfs/send.c            |  9 +++------
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 6cabc8acee2a..5f8006e4de9d 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -94,7 +94,7 @@
 #include <linux/mutex.h>
 #include <linux/genhd.h>
 #include <linux/blkdev.h>
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include <linux/string.h>
 #include "ctree.h"
 #include "disk-io.h"
@@ -2920,13 +2920,10 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info,
 		       fs_info->sectorsize, PAGE_SIZE);
 		return -1;
 	}
-	state = kzalloc(sizeof(*state), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	state = kvzalloc(sizeof(*state), GFP_KERNEL);
 	if (!state) {
-		state = vzalloc(sizeof(*state));
-		if (!state) {
-			pr_info("btrfs check-integrity: vzalloc() failed!\n");
-			return -1;
-		}
+		pr_info("btrfs check-integrity: allocation failed!\n");
+		return -1;
 	}
 
 	if (!btrfsic_is_initialized) {
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index d8ea0eb76325..d68af3c61b49 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -31,7 +31,7 @@
 #include <linux/hash.h>
 #include <linux/list_sort.h>
 #include <linux/raid/xor.h>
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include <asm/div64.h>
 #include "ctree.h"
 #include "extent_map.h"
@@ -218,12 +218,9 @@ int btrfs_alloc_stripe_hash_table(struct btrfs_fs_info *info)
 	 * of a failing mount.
 	 */
 	table_size = sizeof(*table) + sizeof(*h) * num_entries;
-	table = kzalloc(table_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
-	if (!table) {
-		table = vzalloc(table_size);
-		if (!table)
-			return -ENOMEM;
-	}
+	table = kvzalloc(table_size, GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
 
 	spin_lock_init(&table->cache_lock);
 	INIT_LIST_HEAD(&table->stripe_cache);
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index e8185c83f667..924b1d941b53 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -6389,13 +6389,10 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
 
 	alloc_size = sizeof(struct clone_root) * (arg->clone_sources_count + 1);
 
-	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN);
+	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL);
 	if (!sctx->clone_roots) {
-		sctx->clone_roots = vzalloc(alloc_size);
-		if (!sctx->clone_roots) {
-			ret = -ENOMEM;
-			goto out;
-		}
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	alloc_size = arg->clone_sources_count * sizeof(*arg->clone_sources);
-- 
2.12.0

Re: [PATCH 1/5] btrfs: replace opencoded kvzalloc with the helper

[Anand Jain]

> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index e8185c83f667..924b1d941b53 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -6389,13 +6389,10 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
>
>  	alloc_size = sizeof(struct clone_root) * (arg->clone_sources_count + 1);
>
> -	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN);
> +	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL);


  Should be kvzalloc ?

Thanks, Anand


>  	if (!sctx->clone_roots) {
> -		sctx->clone_roots = vzalloc(alloc_size);
> -		if (!sctx->clone_roots) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> +		ret = -ENOMEM;
> +		goto out;
>  	}
>
>  	alloc_size = arg->clone_sources_count * sizeof(*arg->clone_sources);
>

Re: [PATCH 1/5] btrfs: replace opencoded kvzalloc with the helper

[David Sterba]

On Thu, Jun 01, 2017 at 04:04:47PM +0800, Anand Jain wrote:
> 
> 
> 
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index e8185c83f667..924b1d941b53 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -6389,13 +6389,10 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
> >
> >  	alloc_size = sizeof(struct clone_root) * (arg->clone_sources_count + 1);
> >
> > -	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN);
> > +	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL);
> 
> 
>   Should be kvzalloc ?

Of course, I'll fix it. Thanks.

[PATCH 2/5] btrfs: send: use kvmalloc in iterate_dir_item

[David Sterba]

We use a growing buffer for xattrs larger than a page size, at some
point vmalloc is unconditionally used for larger buffers. We can still
try to avoid it using the kvmalloc helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 924b1d941b53..7416b17c0eac 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1083,7 +1083,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
 				buf = tmp;
 			}
 			if (!buf) {
-				buf = vmalloc(buf_len);
+				buf = kvmalloc(buf_len, GFP_KERNEL);
 				if (!buf) {
 					ret = -ENOMEM;
 					goto out;
-- 
2.12.0

Re: [PATCH 2/5] btrfs: send: use kvmalloc in iterate_dir_item

[Anand Jain]

On 06/01/17 01:49, David Sterba wrote:
> We use a growing buffer for xattrs larger than a page size, at some
> point vmalloc is unconditionally used for larger buffers. We can still
> try to avoid it using the kvmalloc helper.

  Reviewed-by: Anand Jain <anand.jain@oracle.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/send.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 924b1d941b53..7416b17c0eac 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -1083,7 +1083,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
>  				buf = tmp;
>  			}
>  			if (!buf) {
> -				buf = vmalloc(buf_len);
> +				buf = kvmalloc(buf_len, GFP_KERNEL);
>  				if (!buf) {
>  					ret = -ENOMEM;
>  					goto out;
>

[PATCH 3/5] btrfs: scrub: add memalloc_nofs protection around init_ipath

[David Sterba]

init_ipath is called from a safe ioctl context and from scrub when
printing an error.  The protection is added for three reasons:

* init_data_container calls vmalloc and this does not work as expected
  in the GFP_NOFS context, so this silently does GFP_KERNEL and might
  deadlock in some cases
* keep the context constraint of GFP_NOFS, used by scrub
* we want to use GFP_KERNEL unconditionally inside init_ipath or its
  callees

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/scrub.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index e99be644b19f..096e503e3ddc 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -18,6 +18,7 @@
 
 #include <linux/blkdev.h>
 #include <linux/ratelimit.h>
+#include <linux/sched/mm.h>
 #include "ctree.h"
 #include "volumes.h"
 #include "disk-io.h"
@@ -733,6 +734,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
 	u32 nlink;
 	int ret;
 	int i;
+	unsigned nofs_flag;
 	struct extent_buffer *eb;
 	struct btrfs_inode_item *inode_item;
 	struct scrub_warning *swarn = warn_ctx;
@@ -771,7 +773,14 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
 	nlink = btrfs_inode_nlink(eb, inode_item);
 	btrfs_release_path(swarn->path);
 
+	/*
+	 * init_path might indirectly call vmalloc, or use GFP_KERNEL. Scrub
+	 * uses GFP_NOFS in this context, so we keep it consistent but it does
+	 * not seem to be strictly necessary.
+	 */
+	nofs_flag = memalloc_nofs_save();
 	ipath = init_ipath(4096, local_root, swarn->path);
+	memalloc_nofs_restore(nofs_flag);
 	if (IS_ERR(ipath)) {
 		ret = PTR_ERR(ipath);
 		ipath = NULL;
-- 
2.12.0

Re: [PATCH 3/5] btrfs: scrub: add memalloc_nofs protection around init_ipath

[Anand Jain]

On 06/01/17 01:49, David Sterba wrote:
> init_ipath is called from a safe ioctl context and from scrub when
> printing an error.  The protection is added for three reasons:
>
> * init_data_container calls vmalloc and this does not work as expected
>   in the GFP_NOFS context, so this silently does GFP_KERNEL and might
>   deadlock in some cases
> * keep the context constraint of GFP_NOFS, used by scrub
> * we want to use GFP_KERNEL unconditionally inside init_ipath or its
>   callees

  Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand


> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/scrub.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index e99be644b19f..096e503e3ddc 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -18,6 +18,7 @@
>
>  #include <linux/blkdev.h>
>  #include <linux/ratelimit.h>
> +#include <linux/sched/mm.h>
>  #include "ctree.h"
>  #include "volumes.h"
>  #include "disk-io.h"
> @@ -733,6 +734,7 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
>  	u32 nlink;
>  	int ret;
>  	int i;
> +	unsigned nofs_flag;
>  	struct extent_buffer *eb;
>  	struct btrfs_inode_item *inode_item;
>  	struct scrub_warning *swarn = warn_ctx;
> @@ -771,7 +773,14 @@ static int scrub_print_warning_inode(u64 inum, u64 offset, u64 root,
>  	nlink = btrfs_inode_nlink(eb, inode_item);
>  	btrfs_release_path(swarn->path);
>
> +	/*
> +	 * init_path might indirectly call vmalloc, or use GFP_KERNEL. Scrub
> +	 * uses GFP_NOFS in this context, so we keep it consistent but it does
> +	 * not seem to be strictly necessary.
> +	 */
> +	nofs_flag = memalloc_nofs_save();
>  	ipath = init_ipath(4096, local_root, swarn->path);
> +	memalloc_nofs_restore(nofs_flag);
>  	if (IS_ERR(ipath)) {
>  		ret = PTR_ERR(ipath);
>  		ipath = NULL;
>

[PATCH 4/5] btrfs: use GFP_KERNEL in init_ipath

[David Sterba]

Now that init_ipath is called either from a safe context or with
memalloc_nofs protection, we can switch to GFP_KERNEL allocations in
init_path and init_data_container.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/backref.c | 10 +++++-----
 fs/btrfs/ioctl.c   |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 24865da63d8f..f723c11bb763 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -16,7 +16,7 @@
  * Boston, MA 021110-1307, USA.
  */
 
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include <linux/rbtree.h>
 #include "ctree.h"
 #include "disk-io.h"
@@ -2305,7 +2305,7 @@ struct btrfs_data_container *init_data_container(u32 total_bytes)
 	size_t alloc_bytes;
 
 	alloc_bytes = max_t(size_t, total_bytes, sizeof(*data));
-	data = vmalloc(alloc_bytes);
+	data = kvmalloc(alloc_bytes, GFP_KERNEL);
 	if (!data)
 		return ERR_PTR(-ENOMEM);
 
@@ -2339,9 +2339,9 @@ struct inode_fs_paths *init_ipath(s32 total_bytes, struct btrfs_root *fs_root,
 	if (IS_ERR(fspath))
 		return (void *)fspath;
 
-	ifp = kmalloc(sizeof(*ifp), GFP_NOFS);
+	ifp = kmalloc(sizeof(*ifp), GFP_KERNEL);
 	if (!ifp) {
-		vfree(fspath);
+		kvfree(fspath);
 		return ERR_PTR(-ENOMEM);
 	}
 
@@ -2356,6 +2356,6 @@ void free_ipath(struct inode_fs_paths *ipath)
 {
 	if (!ipath)
 		return;
-	vfree(ipath->fspath);
+	kvfree(ipath->fspath);
 	kfree(ipath);
 }
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c9cdea8061bc..e4116f9248c2 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -37,7 +37,7 @@
 #include <linux/bit_spinlock.h>
 #include <linux/security.h>
 #include <linux/xattr.h>
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/blkdev.h>
 #include <linux/uuid.h>
@@ -4588,7 +4588,7 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_fs_info *fs_info,
 
 out:
 	btrfs_free_path(path);
-	vfree(inodes);
+	kvfree(inodes);
 	kfree(loi);
 
 	return ret;
-- 
2.12.0

Re: [PATCH 4/5] btrfs: use GFP_KERNEL in init_ipath

[Anand Jain]

On 06/01/17 01:49, David Sterba wrote:
> Now that init_ipath is called either from a safe context or with
> memalloc_nofs protection, we can switch to GFP_KERNEL allocations in
> init_path and init_data_container.

  Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand


> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/backref.c | 10 +++++-----
>  fs/btrfs/ioctl.c   |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 24865da63d8f..f723c11bb763 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -16,7 +16,7 @@
>   * Boston, MA 021110-1307, USA.
>   */
>
> -#include <linux/vmalloc.h>
> +#include <linux/mm.h>
>  #include <linux/rbtree.h>
>  #include "ctree.h"
>  #include "disk-io.h"
> @@ -2305,7 +2305,7 @@ struct btrfs_data_container *init_data_container(u32 total_bytes)
>  	size_t alloc_bytes;
>
>  	alloc_bytes = max_t(size_t, total_bytes, sizeof(*data));
> -	data = vmalloc(alloc_bytes);
> +	data = kvmalloc(alloc_bytes, GFP_KERNEL);
>  	if (!data)
>  		return ERR_PTR(-ENOMEM);
>
> @@ -2339,9 +2339,9 @@ struct inode_fs_paths *init_ipath(s32 total_bytes, struct btrfs_root *fs_root,
>  	if (IS_ERR(fspath))
>  		return (void *)fspath;
>
> -	ifp = kmalloc(sizeof(*ifp), GFP_NOFS);
> +	ifp = kmalloc(sizeof(*ifp), GFP_KERNEL);
>  	if (!ifp) {
> -		vfree(fspath);
> +		kvfree(fspath);
>  		return ERR_PTR(-ENOMEM);
>  	}
>
> @@ -2356,6 +2356,6 @@ void free_ipath(struct inode_fs_paths *ipath)
>  {
>  	if (!ipath)
>  		return;
> -	vfree(ipath->fspath);
> +	kvfree(ipath->fspath);
>  	kfree(ipath);
>  }
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index c9cdea8061bc..e4116f9248c2 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -37,7 +37,7 @@
>  #include <linux/bit_spinlock.h>
>  #include <linux/security.h>
>  #include <linux/xattr.h>
> -#include <linux/vmalloc.h>
> +#include <linux/mm.h>
>  #include <linux/slab.h>
>  #include <linux/blkdev.h>
>  #include <linux/uuid.h>
> @@ -4588,7 +4588,7 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_fs_info *fs_info,
>
>  out:
>  	btrfs_free_path(path);
> -	vfree(inodes);
> +	kvfree(inodes);
>  	kfree(loi);
>
>  	return ret;
>

[PATCH 5/5] btrfs: adjust includes after vmalloc removal

[David Sterba]

As we don't use vmalloc/vzalloc/vfree directly in ctree.c, we can now
use the proper header that defines kvmalloc.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6e1b02dd72d3..3f4daa9d6e2c 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -19,7 +19,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/rbtree.h>
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
-- 
2.12.0