* Re: [PATCH] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 0:25 [PATCH] Btrfs: send, lower mem requirements for processing xattrs Filipe Manana
@ 2014-08-10 23:52 ` Josef Bacik
2014-08-11 0:10 ` Filipe David Manana
2014-08-20 14:28 ` David Sterba
2014-08-11 1:22 ` [PATCH v2] " Filipe Manana
` (3 subsequent siblings)
4 siblings, 2 replies; 10+ messages in thread
From: Josef Bacik @ 2014-08-10 23:52 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs@vger.kernel.org, lists@colorremedies.com
Sigh I can only top post from my phone. Instead of using contig_bug just use is_vmalloc_addr. Thanks,
Josef
Filipe Manana <fdmanana@suse.com> wrote:
Maximum xattr size can be up to nearly the leaf size. For an fs with a
leaf size larger than the page size, using kmalloc requires allocating
multiple pages that are contiguous, which might not be possible if
there's heavy memory fragmentation. Therefore fallback to vmalloc if
we fail to allocate with kmalloc. Also start with a smaller buffer size,
since xattr values typically are smaller than a page.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/send.c | 41 +++++++++++++++++++++++++++++++++--------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 3c63b29..215064d 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -997,6 +997,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
struct btrfs_key di_key;
char *buf = NULL;
int buf_len;
+ bool contig_buf;
u32 name_len;
u32 data_len;
u32 cur;
@@ -1006,11 +1007,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
int num;
u8 type;
- if (found_key->type == BTRFS_XATTR_ITEM_KEY)
- buf_len = BTRFS_MAX_XATTR_SIZE(root);
- else
- buf_len = PATH_MAX;
-
+ /*
+ * Start with a small buffer (1 page). If later we end up needing more
+ * space, which can happen for xattrs on a fs with a leaf size > 4Kb,
+ * attempt to increase the buffer. Typically xattr values are small.
+ */
+ buf_len = PATH_MAX;
+ contig_buf = true;
buf = kmalloc(buf_len, GFP_NOFS);
if (!buf) {
ret = -ENOMEM;
@@ -1037,7 +1040,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
ret = -ENAMETOOLONG;
goto out;
}
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
ret = -E2BIG;
goto out;
}
@@ -1045,12 +1048,31 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
/*
* Path too long
*/
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > PATH_MAX) {
ret = -ENAMETOOLONG;
goto out;
}
}
+ if (name_len + data_len > buf_len) {
+ if (contig_buf)
+ kfree(buf);
+ else
+ vfree(buf);
+ buf = NULL;
+ buf_len = name_len + data_len;
+ if (contig_buf)
+ buf = kmalloc(buf_len, GFP_NOFS);
+ if (!buf) {
+ buf = vmalloc(buf_len);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ contig_buf = false;
+ }
+ }
+
read_extent_buffer(eb, buf, (unsigned long)(di + 1),
name_len + data_len);
@@ -1071,7 +1093,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
}
out:
- kfree(buf);
+ if (contig_buf)
+ kfree(buf);
+ else
+ vfree(buf);
return ret;
}
--
1.9.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 https://urldefense.proofpoint.com/v1/url?u=http://vger.kernel.org/majordomo-info.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=0g6WGYzRdvDGTNBGE%2B9Cc6zaQIOCx8CwbRFIJFgZcAM%3D%0A&s=cd46d14d0b643be3734a57e10521a9ccd5b5ec3732bb96b6f83da18df6a44c98
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] Btrfs: send, lower mem requirements for processing xattrs
2014-08-10 23:52 ` Josef Bacik
@ 2014-08-11 0:10 ` Filipe David Manana
2014-08-20 14:28 ` David Sterba
1 sibling, 0 replies; 10+ messages in thread
From: Filipe David Manana @ 2014-08-11 0:10 UTC (permalink / raw)
To: Josef Bacik
Cc: Filipe Manana, linux-btrfs@vger.kernel.org,
lists@colorremedies.com
On Mon, Aug 11, 2014 at 12:52 AM, Josef Bacik <jbacik@fb.com> wrote:
> Sigh I can only top post from my phone. Instead of using contig_bug just use is_vmalloc_addr. Thanks,
Thanks Josef, I wasn't aware of that helper function.
>
> Josef
>
> Filipe Manana <fdmanana@suse.com> wrote:
>
>
> Maximum xattr size can be up to nearly the leaf size. For an fs with a
> leaf size larger than the page size, using kmalloc requires allocating
> multiple pages that are contiguous, which might not be possible if
> there's heavy memory fragmentation. Therefore fallback to vmalloc if
> we fail to allocate with kmalloc. Also start with a smaller buffer size,
> since xattr values typically are smaller than a page.
>
> Reported-by: Chris Murphy <lists@colorremedies.com>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> fs/btrfs/send.c | 41 +++++++++++++++++++++++++++++++++--------
> 1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 3c63b29..215064d 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -997,6 +997,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
> struct btrfs_key di_key;
> char *buf = NULL;
> int buf_len;
> + bool contig_buf;
> u32 name_len;
> u32 data_len;
> u32 cur;
> @@ -1006,11 +1007,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
> int num;
> u8 type;
>
> - if (found_key->type == BTRFS_XATTR_ITEM_KEY)
> - buf_len = BTRFS_MAX_XATTR_SIZE(root);
> - else
> - buf_len = PATH_MAX;
> -
> + /*
> + * Start with a small buffer (1 page). If later we end up needing more
> + * space, which can happen for xattrs on a fs with a leaf size > 4Kb,
> + * attempt to increase the buffer. Typically xattr values are small.
> + */
> + buf_len = PATH_MAX;
> + contig_buf = true;
> buf = kmalloc(buf_len, GFP_NOFS);
> if (!buf) {
> ret = -ENOMEM;
> @@ -1037,7 +1040,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
> ret = -ENAMETOOLONG;
> goto out;
> }
> - if (name_len + data_len > buf_len) {
> + if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
> ret = -E2BIG;
> goto out;
> }
> @@ -1045,12 +1048,31 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
> /*
> * Path too long
> */
> - if (name_len + data_len > buf_len) {
> + if (name_len + data_len > PATH_MAX) {
> ret = -ENAMETOOLONG;
> goto out;
> }
> }
>
> + if (name_len + data_len > buf_len) {
> + if (contig_buf)
> + kfree(buf);
> + else
> + vfree(buf);
> + buf = NULL;
> + buf_len = name_len + data_len;
> + if (contig_buf)
> + buf = kmalloc(buf_len, GFP_NOFS);
> + if (!buf) {
> + buf = vmalloc(buf_len);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + contig_buf = false;
> + }
> + }
> +
> read_extent_buffer(eb, buf, (unsigned long)(di + 1),
> name_len + data_len);
>
> @@ -1071,7 +1093,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
> }
>
> out:
> - kfree(buf);
> + if (contig_buf)
> + kfree(buf);
> + else
> + vfree(buf);
> return ret;
> }
>
> --
> 1.9.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 https://urldefense.proofpoint.com/v1/url?u=http://vger.kernel.org/majordomo-info.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=0g6WGYzRdvDGTNBGE%2B9Cc6zaQIOCx8CwbRFIJFgZcAM%3D%0A&s=cd46d14d0b643be3734a57e10521a9ccd5b5ec3732bb96b6f83da18df6a44c98
> --
> 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
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Btrfs: send, lower mem requirements for processing xattrs
2014-08-10 23:52 ` Josef Bacik
2014-08-11 0:10 ` Filipe David Manana
@ 2014-08-20 14:28 ` David Sterba
1 sibling, 0 replies; 10+ messages in thread
From: David Sterba @ 2014-08-20 14:28 UTC (permalink / raw)
To: Josef Bacik
Cc: Filipe Manana, linux-btrfs@vger.kernel.org,
lists@colorremedies.com
On Sun, Aug 10, 2014 at 11:52:09PM +0000, Josef Bacik wrote:
> Sigh I can only top post from my phone.
Can you at least snip the original text?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 0:25 [PATCH] Btrfs: send, lower mem requirements for processing xattrs Filipe Manana
2014-08-10 23:52 ` Josef Bacik
@ 2014-08-11 1:22 ` Filipe Manana
2014-08-11 0:38 ` Josef Bacik
2014-08-11 1:52 ` [PATCH v3] " Filipe Manana
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Filipe Manana @ 2014-08-11 1:22 UTC (permalink / raw)
To: linux-btrfs; +Cc: lists, jbacik, Filipe Manana
Maximum xattr size can be up to nearly the leaf size. For an fs with a
leaf size larger than the page size, using kmalloc requires allocating
multiple pages that are contiguous, which might not be possible if
there's heavy memory fragmentation. Therefore fallback to vmalloc if
we fail to allocate with kmalloc. Also start with a smaller buffer size,
since xattr values typically are smaller than a page.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: Use is_vmalloc_addr() instead of keeping a boolean variable around.
fs/btrfs/send.c | 39 +++++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 3c63b29..a7ce318 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1006,11 +1006,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
int num;
u8 type;
- if (found_key->type == BTRFS_XATTR_ITEM_KEY)
- buf_len = BTRFS_MAX_XATTR_SIZE(root);
- else
- buf_len = PATH_MAX;
-
+ /*
+ * Start with a small buffer (1 page). If later we end up needing more
+ * space, which can happen for xattrs on a fs with a leaf size greater
+ * then the page size, attempt to increase the buffer. Typically xattr
+ * values are small.
+ */
+ buf_len = PATH_MAX;
buf = kmalloc(buf_len, GFP_NOFS);
if (!buf) {
ret = -ENOMEM;
@@ -1037,7 +1039,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
ret = -ENAMETOOLONG;
goto out;
}
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
ret = -E2BIG;
goto out;
}
@@ -1045,12 +1047,30 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
/*
* Path too long
*/
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > PATH_MAX) {
ret = -ENAMETOOLONG;
goto out;
}
}
+ if (name_len + data_len > buf_len) {
+ buf_len = name_len + data_len;
+ if (is_vmalloc_addr(buf)) {
+ vfree(buf);
+ buf = NULL;
+ } else {
+ kfree(buf);
+ buf = kmalloc(buf_len, GFP_NOFS);
+ }
+ if (!buf) {
+ buf = vmalloc(buf_len);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+ }
+
read_extent_buffer(eb, buf, (unsigned long)(di + 1),
name_len + data_len);
@@ -1071,7 +1091,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
}
out:
- kfree(buf);
+ if (is_vmalloc_addr(buf))
+ vfree(buf);
+ else
+ kfree(buf);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 1:22 ` [PATCH v2] " Filipe Manana
@ 2014-08-11 0:38 ` Josef Bacik
0 siblings, 0 replies; 10+ messages in thread
From: Josef Bacik @ 2014-08-11 0:38 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs@vger.kernel.org, lists@colorremedies.com
Sorry one more thing, just do krealloc instead of the kfrww+kmalloc. Thanks,
Josef
Filipe Manana <fdmanana@suse.com> wrote:
Maximum xattr size can be up to nearly the leaf size. For an fs with a
leaf size larger than the page size, using kmalloc requires allocating
multiple pages that are contiguous, which might not be possible if
there's heavy memory fragmentation. Therefore fallback to vmalloc if
we fail to allocate with kmalloc. Also start with a smaller buffer size,
since xattr values typically are smaller than a page.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: Use is_vmalloc_addr() instead of keeping a boolean variable around.
fs/btrfs/send.c | 39 +++++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 3c63b29..a7ce318 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1006,11 +1006,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
int num;
u8 type;
- if (found_key->type == BTRFS_XATTR_ITEM_KEY)
- buf_len = BTRFS_MAX_XATTR_SIZE(root);
- else
- buf_len = PATH_MAX;
-
+ /*
+ * Start with a small buffer (1 page). If later we end up needing more
+ * space, which can happen for xattrs on a fs with a leaf size greater
+ * then the page size, attempt to increase the buffer. Typically xattr
+ * values are small.
+ */
+ buf_len = PATH_MAX;
buf = kmalloc(buf_len, GFP_NOFS);
if (!buf) {
ret = -ENOMEM;
@@ -1037,7 +1039,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
ret = -ENAMETOOLONG;
goto out;
}
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
ret = -E2BIG;
goto out;
}
@@ -1045,12 +1047,30 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
/*
* Path too long
*/
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > PATH_MAX) {
ret = -ENAMETOOLONG;
goto out;
}
}
+ if (name_len + data_len > buf_len) {
+ buf_len = name_len + data_len;
+ if (is_vmalloc_addr(buf)) {
+ vfree(buf);
+ buf = NULL;
+ } else {
+ kfree(buf);
+ buf = kmalloc(buf_len, GFP_NOFS);
+ }
+ if (!buf) {
+ buf = vmalloc(buf_len);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+ }
+
read_extent_buffer(eb, buf, (unsigned long)(di + 1),
name_len + data_len);
@@ -1071,7 +1091,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
}
out:
- kfree(buf);
+ if (is_vmalloc_addr(buf))
+ vfree(buf);
+ else
+ kfree(buf);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 0:25 [PATCH] Btrfs: send, lower mem requirements for processing xattrs Filipe Manana
2014-08-10 23:52 ` Josef Bacik
2014-08-11 1:22 ` [PATCH v2] " Filipe Manana
@ 2014-08-11 1:52 ` Filipe Manana
2014-08-11 2:09 ` [PATCH v4] " Filipe Manana
2014-08-20 9:45 ` [PATCH v5] " Filipe Manana
4 siblings, 0 replies; 10+ messages in thread
From: Filipe Manana @ 2014-08-11 1:52 UTC (permalink / raw)
To: linux-btrfs; +Cc: lists, jbacik, Filipe Manana
Maximum xattr size can be up to nearly the leaf size. For an fs with a
leaf size larger than the page size, using kmalloc requires allocating
multiple pages that are contiguous, which might not be possible if
there's heavy memory fragmentation. Therefore fallback to vmalloc if
we fail to allocate with kmalloc. Also start with a smaller buffer size,
since xattr values typically are smaller than a page.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: Use is_vmalloc_addr() instead of keeping a boolean variable around.
V3: Use krealloc instead of kfree + kmalloc.
fs/btrfs/send.c | 41 +++++++++++++++++++++++++++++++++--------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 3c63b29..8b2780d 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1006,11 +1006,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
int num;
u8 type;
- if (found_key->type == BTRFS_XATTR_ITEM_KEY)
- buf_len = BTRFS_MAX_XATTR_SIZE(root);
- else
- buf_len = PATH_MAX;
-
+ /*
+ * Start with a small buffer (1 page). If later we end up needing more
+ * space, which can happen for xattrs on a fs with a leaf size greater
+ * then the page size, attempt to increase the buffer. Typically xattr
+ * values are small.
+ */
+ buf_len = PATH_MAX;
buf = kmalloc(buf_len, GFP_NOFS);
if (!buf) {
ret = -ENOMEM;
@@ -1037,7 +1039,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
ret = -ENAMETOOLONG;
goto out;
}
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
ret = -E2BIG;
goto out;
}
@@ -1045,12 +1047,32 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
/*
* Path too long
*/
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > PATH_MAX) {
ret = -ENAMETOOLONG;
goto out;
}
}
+ if (name_len + data_len > buf_len) {
+ buf_len = name_len + data_len;
+ if (is_vmalloc_addr(buf)) {
+ vfree(buf);
+ buf = NULL;
+ } else {
+ char *tmp = krealloc(buf, buf_len, GFP_NOFS);
+ if (!tmp)
+ kfree(buf);
+ buf = tmp;
+ }
+ if (!buf) {
+ buf = vmalloc(buf_len);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+ }
+
read_extent_buffer(eb, buf, (unsigned long)(di + 1),
name_len + data_len);
@@ -1071,7 +1093,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
}
out:
- kfree(buf);
+ if (is_vmalloc_addr(buf))
+ vfree(buf);
+ else
+ kfree(buf);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 0:25 [PATCH] Btrfs: send, lower mem requirements for processing xattrs Filipe Manana
` (2 preceding siblings ...)
2014-08-11 1:52 ` [PATCH v3] " Filipe Manana
@ 2014-08-11 2:09 ` Filipe Manana
2014-08-19 14:46 ` David Sterba
2014-08-20 9:45 ` [PATCH v5] " Filipe Manana
4 siblings, 1 reply; 10+ messages in thread
From: Filipe Manana @ 2014-08-11 2:09 UTC (permalink / raw)
To: linux-btrfs; +Cc: lists, jbacik, Filipe Manana
Maximum xattr size can be up to nearly the leaf size. For an fs with a
leaf size larger than the page size, using kmalloc requires allocating
multiple pages that are contiguous, which might not be possible if
there's heavy memory fragmentation. Therefore fallback to vmalloc if
we fail to allocate with kmalloc. Also start with a smaller buffer size,
since xattr values typically are smaller than a page.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: Use is_vmalloc_addr() instead of keeping a boolean variable around.
V3: Use krealloc instead of kfree + kmalloc.
V4: Fixed a checkpatch warning about missing blank line after var declaration.
fs/btrfs/send.c | 42 ++++++++++++++++++++++++++++++++++--------
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 3c63b29..b29fc5c 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1006,11 +1006,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
int num;
u8 type;
- if (found_key->type == BTRFS_XATTR_ITEM_KEY)
- buf_len = BTRFS_MAX_XATTR_SIZE(root);
- else
- buf_len = PATH_MAX;
-
+ /*
+ * Start with a small buffer (1 page). If later we end up needing more
+ * space, which can happen for xattrs on a fs with a leaf size greater
+ * then the page size, attempt to increase the buffer. Typically xattr
+ * values are small.
+ */
+ buf_len = PATH_MAX;
buf = kmalloc(buf_len, GFP_NOFS);
if (!buf) {
ret = -ENOMEM;
@@ -1037,7 +1039,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
ret = -ENAMETOOLONG;
goto out;
}
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
ret = -E2BIG;
goto out;
}
@@ -1045,12 +1047,33 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
/*
* Path too long
*/
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > PATH_MAX) {
ret = -ENAMETOOLONG;
goto out;
}
}
+ if (name_len + data_len > buf_len) {
+ buf_len = name_len + data_len;
+ if (is_vmalloc_addr(buf)) {
+ vfree(buf);
+ buf = NULL;
+ } else {
+ char *tmp = krealloc(buf, buf_len, GFP_NOFS);
+
+ if (!tmp)
+ kfree(buf);
+ buf = tmp;
+ }
+ if (!buf) {
+ buf = vmalloc(buf_len);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+ }
+
read_extent_buffer(eb, buf, (unsigned long)(di + 1),
name_len + data_len);
@@ -1071,7 +1094,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
}
out:
- kfree(buf);
+ if (is_vmalloc_addr(buf))
+ vfree(buf);
+ else
+ kfree(buf);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 2:09 ` [PATCH v4] " Filipe Manana
@ 2014-08-19 14:46 ` David Sterba
0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2014-08-19 14:46 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs, lists, jbacik
On Mon, Aug 11, 2014 at 03:09:35AM +0100, Filipe Manana wrote:
> + if (name_len + data_len > buf_len) {
> + buf_len = name_len + data_len;
> + if (is_vmalloc_addr(buf)) {
> + vfree(buf);
> + buf = NULL;
> + } else {
> + char *tmp = krealloc(buf, buf_len, GFP_NOFS);
This could fail with a warning (high order allocation) so I suggest to
add __GFP_NOWARN, the vmalloc fallback will catch fragmented memory case
and fail if theres no memory.
> +
> + if (!tmp)
> + kfree(buf);
> + buf = tmp;
> + }
> + if (!buf) {
> + buf = vmalloc(buf_len);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + }
> + }
> +
> read_extent_buffer(eb, buf, (unsigned long)(di + 1),
> name_len + data_len);
>
> @@ -1071,7 +1094,10 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
> }
>
> out:
> - kfree(buf);
> + if (is_vmalloc_addr(buf))
> + vfree(buf);
> + else
> + kfree(buf);
There's even kvfree to do this.
> return ret;
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5] Btrfs: send, lower mem requirements for processing xattrs
2014-08-11 0:25 [PATCH] Btrfs: send, lower mem requirements for processing xattrs Filipe Manana
` (3 preceding siblings ...)
2014-08-11 2:09 ` [PATCH v4] " Filipe Manana
@ 2014-08-20 9:45 ` Filipe Manana
4 siblings, 0 replies; 10+ messages in thread
From: Filipe Manana @ 2014-08-20 9:45 UTC (permalink / raw)
To: linux-btrfs; +Cc: lists, Filipe Manana
Maximum xattr size can be up to nearly the leaf size. For an fs with a
leaf size larger than the page size, using kmalloc requires allocating
multiple pages that are contiguous, which might not be possible if
there's heavy memory fragmentation. Therefore fallback to vmalloc if
we fail to allocate with kmalloc. Also start with a smaller buffer size,
since xattr values typically are smaller than a page.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: Use is_vmalloc_addr() instead of keeping a boolean variable around.
V3: Use krealloc instead of kfree + kmalloc.
V4: Fixed a checkpatch warning about missing blank line after var declaration.
V5: Use kvfree() and pass __GFP_NOWARN to krealloc().
fs/btrfs/send.c | 40 ++++++++++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 3c63b29..3290da9 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1006,11 +1006,13 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
int num;
u8 type;
- if (found_key->type == BTRFS_XATTR_ITEM_KEY)
- buf_len = BTRFS_MAX_XATTR_SIZE(root);
- else
- buf_len = PATH_MAX;
-
+ /*
+ * Start with a small buffer (1 page). If later we end up needing more
+ * space, which can happen for xattrs on a fs with a leaf size greater
+ * then the page size, attempt to increase the buffer. Typically xattr
+ * values are small.
+ */
+ buf_len = PATH_MAX;
buf = kmalloc(buf_len, GFP_NOFS);
if (!buf) {
ret = -ENOMEM;
@@ -1037,7 +1039,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
ret = -ENAMETOOLONG;
goto out;
}
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root)) {
ret = -E2BIG;
goto out;
}
@@ -1045,12 +1047,34 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
/*
* Path too long
*/
- if (name_len + data_len > buf_len) {
+ if (name_len + data_len > PATH_MAX) {
ret = -ENAMETOOLONG;
goto out;
}
}
+ if (name_len + data_len > buf_len) {
+ buf_len = name_len + data_len;
+ if (is_vmalloc_addr(buf)) {
+ vfree(buf);
+ buf = NULL;
+ } else {
+ char *tmp = krealloc(buf, buf_len,
+ GFP_NOFS | __GFP_NOWARN);
+
+ if (!tmp)
+ kfree(buf);
+ buf = tmp;
+ }
+ if (!buf) {
+ buf = vmalloc(buf_len);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ }
+ }
+
read_extent_buffer(eb, buf, (unsigned long)(di + 1),
name_len + data_len);
@@ -1071,7 +1095,7 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path,
}
out:
- kfree(buf);
+ kvfree(buf);
return ret;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread