| Message ID | 8uvx1msa7pjsyoq758f3sw4x.1407714728203@email.android.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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
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?
--
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
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; }
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(-) -- 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