All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFCv4] new ioctl TREE_SEARCH_V2
@ 2014-01-30 15:23 Gerhard Heift
  2014-01-30 15:23 ` [PATCH RFCv4 1/7] btrfs: tree_search: eliminate redundant nr_items check Gerhard Heift
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:23 UTC (permalink / raw)
  To: linux-btrfs

This patch series first rewrites tree_search to copy found items directly to
userspace and then adds a new ioctl TREE_SEARCH_V2 with which we could store
them in a varying buffer. Now even items larger than 3992 bytes or a large
amount of items can be returned. This is the case for some EXTENT_CSUM items,
which could have a size up to 16k.

I had a few questions:
  Which value should I assign to TREE_SEARCH_V2?
  * Chosen value is ok [1].
  Should we limit the buffer size?
  * David suggested [1] a minimum of 64k, I've chosen a cap of 16M.
  What about documentation?
  * I documented the new struct in the header file.

Gerhard

[1] http://article.gmane.org/gmane.comp.file-systems.btrfs/32060

Changelog

RFCv4
  * fixed copy to userspace (used wrong pointers)
  * eliminate a redundant check for nr_items

RFCv3
  * introduced read_extent_buffer_to_user
  * direct copy to user without intermediate buffer
  * use local variables for args
  * fixed wrong error code
  * removed unused var check
  * fixed minor style issues
  * return needed buffer to userspace on EOVERFLOW

RFCv2
  * fixed a build bug caused by using a wrong patch
  * added a patch to expand a buffer lifetime

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 1/7] btrfs: tree_search: eliminate redundant nr_items check
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
@ 2014-01-30 15:23 ` Gerhard Heift
  2014-01-30 15:23 ` [PATCH RFCv4 2/7] btrfs: tree_search, search_ioctl: accept varying buffer Gerhard Heift
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:23 UTC (permalink / raw)
  To: linux-btrfs

If the amount of items reached the given limit of nr_items, we can leave
copy_to_sk without updating the key. Also by returning 1 we leave the loop in
search_ioctl without rechecking if we reached the given limit.

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/ioctl.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 34772cb..b1c5b4f 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1892,7 +1892,7 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		if (sizeof(sh) + item_len + *sk_offset >
 		    BTRFS_SEARCH_ARGS_BUFSIZE) {
 			ret = 1;
-			goto overflow;
+			goto out;
 		}
 
 		sh.objectid = key->objectid;
@@ -1914,8 +1914,10 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		}
 		(*num_found)++;
 
-		if (*num_found >= sk->nr_items)
-			break;
+		if (*num_found >= sk->nr_items) {
+			ret = 1;
+			goto out;
+		}
 	}
 advance_key:
 	ret = 0;
@@ -1930,7 +1932,7 @@ advance_key:
 		key->objectid++;
 	} else
 		ret = 1;
-overflow:
+out:
 	return ret;
 }
 
@@ -1982,7 +1984,7 @@ static noinline int search_ioctl(struct inode *inode,
 		ret = copy_to_sk(root, path, &key, sk, args->buf,
 				 &sk_offset, &num_found);
 		btrfs_release_path(path);
-		if (ret || num_found >= sk->nr_items)
+		if (ret)
 			break;
 
 	}
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 2/7] btrfs: tree_search, search_ioctl: accept varying buffer
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
  2014-01-30 15:23 ` [PATCH RFCv4 1/7] btrfs: tree_search: eliminate redundant nr_items check Gerhard Heift
@ 2014-01-30 15:23 ` Gerhard Heift
  2014-01-30 15:23 ` [PATCH RFCv4 3/7] btrfs: tree_search, copy_to_sk: return EOVERFLOW for too small buffer Gerhard Heift
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:23 UTC (permalink / raw)
  To: linux-btrfs

rewrite search_ioctl to accept a buffer with varying size

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/ioctl.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b1c5b4f..9b66eac 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1854,6 +1854,7 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 			       struct btrfs_path *path,
 			       struct btrfs_key *key,
 			       struct btrfs_ioctl_search_key *sk,
+			       size_t buf_size,
 			       char *buf,
 			       unsigned long *sk_offset,
 			       int *num_found)
@@ -1886,11 +1887,10 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		if (!key_in_sk(key, sk))
 			continue;
 
-		if (sizeof(sh) + item_len > BTRFS_SEARCH_ARGS_BUFSIZE)
+		if (sizeof(sh) + item_len > buf_size)
 			item_len = 0;
 
-		if (sizeof(sh) + item_len + *sk_offset >
-		    BTRFS_SEARCH_ARGS_BUFSIZE) {
+		if (sizeof(sh) + item_len + *sk_offset > buf_size) {
 			ret = 1;
 			goto out;
 		}
@@ -1937,17 +1937,21 @@ out:
 }
 
 static noinline int search_ioctl(struct inode *inode,
-				 struct btrfs_ioctl_search_args *args)
+				 struct btrfs_ioctl_search_key *sk,
+				 size_t buf_size,
+				 char *buf)
 {
 	struct btrfs_root *root;
 	struct btrfs_key key;
 	struct btrfs_path *path;
-	struct btrfs_ioctl_search_key *sk = &args->key;
 	struct btrfs_fs_info *info = BTRFS_I(inode)->root->fs_info;
 	int ret;
 	int num_found = 0;
 	unsigned long sk_offset = 0;
 
+	if (buf_size < sizeof(struct btrfs_ioctl_search_header))
+		return -EOVERFLOW;
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
@@ -1981,7 +1985,7 @@ static noinline int search_ioctl(struct inode *inode,
 				ret = 0;
 			goto err;
 		}
-		ret = copy_to_sk(root, path, &key, sk, args->buf,
+		ret = copy_to_sk(root, path, &key, sk, buf_size, buf,
 				 &sk_offset, &num_found);
 		btrfs_release_path(path);
 		if (ret)
@@ -2010,7 +2014,7 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
 		return PTR_ERR(args);
 
 	inode = file_inode(file);
-	ret = search_ioctl(inode, args);
+	ret = search_ioctl(inode, &args->key, sizeof(args->buf), args->buf);
 	if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
 		ret = -EFAULT;
 	kfree(args);
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 3/7] btrfs: tree_search, copy_to_sk: return EOVERFLOW for too small buffer
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
  2014-01-30 15:23 ` [PATCH RFCv4 1/7] btrfs: tree_search: eliminate redundant nr_items check Gerhard Heift
  2014-01-30 15:23 ` [PATCH RFCv4 2/7] btrfs: tree_search, search_ioctl: accept varying buffer Gerhard Heift
@ 2014-01-30 15:23 ` Gerhard Heift
  2014-01-30 15:24 ` [PATCH RFCv4 4/7] btrfs: tree_search, copy_to_sk: return needed size on EOVERFLOW Gerhard Heift
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:23 UTC (permalink / raw)
  To: linux-btrfs

In copy_to_sk, if an item is too large for the given buffer, it now returns
-EOVERFLOW instead of copying a search_header with len = 0. For backward
compatibility for the first item it still copies such a header to the buffer,
but not any other following items, which could have fitted.

tree_search changes -EOVERFLOW back to 0 to behave similiar to the way it
behaved before this patch.

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/ioctl.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9b66eac..4e32ac2 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1887,8 +1887,20 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		if (!key_in_sk(key, sk))
 			continue;
 
-		if (sizeof(sh) + item_len > buf_size)
+		if (sizeof(sh) + item_len > buf_size) {
+			if (*num_found) {
+				ret = 1;
+				goto out;
+			}
+
+			/*
+			 * return one empty item back for v1, which does not
+			 * handle -EOVERFLOW
+			 */
+
 			item_len = 0;
+			ret = -EOVERFLOW;
+		}
 
 		if (sizeof(sh) + item_len + *sk_offset > buf_size) {
 			ret = 1;
@@ -1914,6 +1926,9 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		}
 		(*num_found)++;
 
+		if (ret) /* -EOVERFLOW from above */
+			goto out;
+
 		if (*num_found >= sk->nr_items) {
 			ret = 1;
 			goto out;
@@ -1992,7 +2007,8 @@ static noinline int search_ioctl(struct inode *inode,
 			break;
 
 	}
-	ret = 0;
+	if (ret > 0)
+		ret = 0;
 err:
 	sk->nr_items = num_found;
 	btrfs_free_path(path);
@@ -2015,6 +2031,14 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
 
 	inode = file_inode(file);
 	ret = search_ioctl(inode, &args->key, sizeof(args->buf), args->buf);
+
+	/*
+	 * In the origin implementation an overflow is handled by returning a
+	 * search header with a len of zero, so reset ret.
+	 */
+	if (ret == -EOVERFLOW)
+		ret = 0;
+
 	if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
 		ret = -EFAULT;
 	kfree(args);
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 4/7] btrfs: tree_search, copy_to_sk: return needed size on EOVERFLOW
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (2 preceding siblings ...)
  2014-01-30 15:23 ` [PATCH RFCv4 3/7] btrfs: tree_search, copy_to_sk: return EOVERFLOW for too small buffer Gerhard Heift
@ 2014-01-30 15:24 ` Gerhard Heift
  2014-01-30 15:24 ` [PATCH RFCv4 5/7] btrfs: new function read_extent_buffer_to_user Gerhard Heift
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:24 UTC (permalink / raw)
  To: linux-btrfs

If an item in tree_search is too large to be stored in the given buffer, return
the needed size (including the header).

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/ioctl.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 4e32ac2..1180293 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1854,7 +1854,7 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 			       struct btrfs_path *path,
 			       struct btrfs_key *key,
 			       struct btrfs_ioctl_search_key *sk,
-			       size_t buf_size,
+			       size_t *buf_size,
 			       char *buf,
 			       unsigned long *sk_offset,
 			       int *num_found)
@@ -1887,7 +1887,7 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		if (!key_in_sk(key, sk))
 			continue;
 
-		if (sizeof(sh) + item_len > buf_size) {
+		if (sizeof(sh) + item_len > *buf_size) {
 			if (*num_found) {
 				ret = 1;
 				goto out;
@@ -1898,11 +1898,12 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 			 * handle -EOVERFLOW
 			 */
 
+			*buf_size = sizeof(sh) + item_len;
 			item_len = 0;
 			ret = -EOVERFLOW;
 		}
 
-		if (sizeof(sh) + item_len + *sk_offset > buf_size) {
+		if (sizeof(sh) + item_len + *sk_offset > *buf_size) {
 			ret = 1;
 			goto out;
 		}
@@ -1953,7 +1954,7 @@ out:
 
 static noinline int search_ioctl(struct inode *inode,
 				 struct btrfs_ioctl_search_key *sk,
-				 size_t buf_size,
+				 size_t *buf_size,
 				 char *buf)
 {
 	struct btrfs_root *root;
@@ -1964,8 +1965,10 @@ static noinline int search_ioctl(struct inode *inode,
 	int num_found = 0;
 	unsigned long sk_offset = 0;
 
-	if (buf_size < sizeof(struct btrfs_ioctl_search_header))
+	if (*buf_size < sizeof(struct btrfs_ioctl_search_header)) {
+		*buf_size = sizeof(struct btrfs_ioctl_search_header);
 		return -EOVERFLOW;
+	}
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -2018,9 +2021,10 @@ err:
 static noinline int btrfs_ioctl_tree_search(struct file *file,
 					   void __user *argp)
 {
-	 struct btrfs_ioctl_search_args *args;
-	 struct inode *inode;
-	 int ret;
+	struct btrfs_ioctl_search_args *args;
+	struct inode *inode;
+	int ret;
+	size_t buf_size;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
@@ -2029,8 +2033,10 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
 	if (IS_ERR(args))
 		return PTR_ERR(args);
 
+	buf_size = sizeof(args->buf);
+
 	inode = file_inode(file);
-	ret = search_ioctl(inode, &args->key, sizeof(args->buf), args->buf);
+	ret = search_ioctl(inode, &args->key, &buf_size, args->buf);
 
 	/*
 	 * In the origin implementation an overflow is handled by returning a
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 5/7] btrfs: new function read_extent_buffer_to_user
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (3 preceding siblings ...)
  2014-01-30 15:24 ` [PATCH RFCv4 4/7] btrfs: tree_search, copy_to_sk: return needed size on EOVERFLOW Gerhard Heift
@ 2014-01-30 15:24 ` Gerhard Heift
  2014-01-30 15:24 ` [PATCH RFCv4 6/7] btrfs: tree_search, search_ioctl: direct copy to userspace Gerhard Heift
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:24 UTC (permalink / raw)
  To: linux-btrfs

This new function reads the content of an extent directly to user memory.

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/extent_io.c | 37 +++++++++++++++++++++++++++++++++++++
 fs/btrfs/extent_io.h |  3 +++
 2 files changed, 40 insertions(+)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index fbe501d..d1e9dd8 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4956,6 +4956,43 @@ void read_extent_buffer(struct extent_buffer *eb, void *dstv,
 	}
 }
 
+int read_extent_buffer_to_user(struct extent_buffer *eb, void __user *dstv,
+			unsigned long start,
+			unsigned long len)
+{
+	size_t cur;
+	size_t offset;
+	struct page *page;
+	char *kaddr;
+	char __user *dst = (char __user *)dstv;
+	size_t start_offset = eb->start & ((u64)PAGE_CACHE_SIZE - 1);
+	unsigned long i = (start_offset + start) >> PAGE_CACHE_SHIFT;
+	int ret = 0;
+
+	WARN_ON(start > eb->len);
+	WARN_ON(start + len > eb->start + eb->len);
+
+	offset = (start_offset + start) & (PAGE_CACHE_SIZE - 1);
+
+	while (len > 0) {
+		page = extent_buffer_page(eb, i);
+
+		cur = min(len, (PAGE_CACHE_SIZE - offset));
+		kaddr = page_address(page);
+		if (copy_to_user(dst, kaddr + offset, cur)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		dst += cur;
+		len -= cur;
+		offset = 0;
+		i++;
+	}
+
+	return ret;
+}
+
 int map_private_extent_buffer(struct extent_buffer *eb, unsigned long start,
 			       unsigned long min_len, char **map,
 			       unsigned long *map_start,
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 58b27e5..d0b1541 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -304,6 +304,9 @@ int memcmp_extent_buffer(struct extent_buffer *eb, const void *ptrv,
 void read_extent_buffer(struct extent_buffer *eb, void *dst,
 			unsigned long start,
 			unsigned long len);
+int read_extent_buffer_to_user(struct extent_buffer *eb, void __user *dst,
+			       unsigned long start,
+			       unsigned long len);
 void write_extent_buffer(struct extent_buffer *eb, const void *src,
 			 unsigned long start, unsigned long len);
 void copy_extent_buffer(struct extent_buffer *dst, struct extent_buffer *src,
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 6/7] btrfs: tree_search, search_ioctl: direct copy to userspace
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (4 preceding siblings ...)
  2014-01-30 15:24 ` [PATCH RFCv4 5/7] btrfs: new function read_extent_buffer_to_user Gerhard Heift
@ 2014-01-30 15:24 ` Gerhard Heift
  2014-01-30 15:24 ` [PATCH RFCv4 7/7] btrfs: new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:24 UTC (permalink / raw)
  To: linux-btrfs

By copying each found item seperatly to userspace, we do not need extra
buffer in the kernel.

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/ioctl.c | 48 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 1180293..bab8c67 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1855,7 +1855,7 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 			       struct btrfs_key *key,
 			       struct btrfs_ioctl_search_key *sk,
 			       size_t *buf_size,
-			       char *buf,
+			       char __user *ubuf,
 			       unsigned long *sk_offset,
 			       int *num_found)
 {
@@ -1915,14 +1915,22 @@ static noinline int copy_to_sk(struct btrfs_root *root,
 		sh.transid = found_transid;
 
 		/* copy search result header */
-		memcpy(buf + *sk_offset, &sh, sizeof(sh));
+		if (copy_to_user(ubuf + *sk_offset, &sh, sizeof(sh))) {
+			ret = -EFAULT;
+			goto out;
+		}
+
 		*sk_offset += sizeof(sh);
 
 		if (item_len) {
-			char *p = buf + *sk_offset;
+			char __user *up = ubuf + *sk_offset;
 			/* copy the item */
-			read_extent_buffer(leaf, p,
-					   item_off, item_len);
+			if (read_extent_buffer_to_user(leaf, up,
+						       item_off, item_len)) {
+				ret = -EFAULT;
+				goto out;
+			}
+
 			*sk_offset += item_len;
 		}
 		(*num_found)++;
@@ -1949,13 +1957,22 @@ advance_key:
 	} else
 		ret = 1;
 out:
+	/*
+	 *  0: all items from this leaf copied, continue with next
+	 *  1: * more items can be copied, but unused buffer is too small
+	 *     * all items were found
+	 *     Either way, it will stops the loop which iterates to the next
+	 *     leaf
+	 *  -EOVERFLOW: item was to large for buffer
+	 *  -EFAULT: could not copy extent buffer back to userspace
+	 */
 	return ret;
 }
 
 static noinline int search_ioctl(struct inode *inode,
 				 struct btrfs_ioctl_search_key *sk,
 				 size_t *buf_size,
-				 char *buf)
+				 char __user *ubuf)
 {
 	struct btrfs_root *root;
 	struct btrfs_key key;
@@ -2003,7 +2020,7 @@ static noinline int search_ioctl(struct inode *inode,
 				ret = 0;
 			goto err;
 		}
-		ret = copy_to_sk(root, path, &key, sk, buf_size, buf,
+		ret = copy_to_sk(root, path, &key, sk, buf_size, ubuf,
 				 &sk_offset, &num_found);
 		btrfs_release_path(path);
 		if (ret)
@@ -2021,7 +2038,8 @@ err:
 static noinline int btrfs_ioctl_tree_search(struct file *file,
 					   void __user *argp)
 {
-	struct btrfs_ioctl_search_args *args;
+	struct btrfs_ioctl_search_args __user *uargs;
+	struct btrfs_ioctl_search_key sk;
 	struct inode *inode;
 	int ret;
 	size_t buf_size;
@@ -2029,14 +2047,15 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	args = memdup_user(argp, sizeof(*args));
-	if (IS_ERR(args))
-		return PTR_ERR(args);
+	uargs = (struct btrfs_ioctl_search_args __user *)argp;
 
-	buf_size = sizeof(args->buf);
+	if (copy_from_user(&sk, &uargs->key, sizeof(sk)))
+		return -EFAULT;
+
+	buf_size = sizeof(uargs->buf);
 
 	inode = file_inode(file);
-	ret = search_ioctl(inode, &args->key, &buf_size, args->buf);
+	ret = search_ioctl(inode, &sk, &buf_size, uargs->buf);
 
 	/*
 	 * In the origin implementation an overflow is handled by returning a
@@ -2045,9 +2064,8 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
 	if (ret == -EOVERFLOW)
 		ret = 0;
 
-	if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
+	if (ret == 0 && copy_to_user(&uargs->key, &sk, sizeof(sk)))
 		ret = -EFAULT;
-	kfree(args);
 	return ret;
 }
 
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFCv4 7/7] btrfs: new ioctl TREE_SEARCH_V2
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (5 preceding siblings ...)
  2014-01-30 15:24 ` [PATCH RFCv4 6/7] btrfs: tree_search, search_ioctl: direct copy to userspace Gerhard Heift
@ 2014-01-30 15:24 ` Gerhard Heift
  2014-01-31 17:40 ` [PATCH RFCv4] " David Sterba
  2014-04-04 11:36 ` David Sterba
  8 siblings, 0 replies; 10+ messages in thread
From: Gerhard Heift @ 2014-01-30 15:24 UTC (permalink / raw)
  To: linux-btrfs

This new ioctl call allows the user to supply a buffer of varying size in which
a tree search can store its results. This is much more flexible if you want to
receive items which are larger than the current fixed buffer of 3992 bytes or
if you want to fetch more items at once. Items larger than this buffer are for
example some of the type EXTENT_CSUM.

Signed-off-by: Gerhard Heift <Gerhard@Heift.Name>
---
 fs/btrfs/ioctl.c           | 40 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/btrfs.h | 10 ++++++++++
 2 files changed, 50 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index bab8c67..763378e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2069,6 +2069,44 @@ static noinline int btrfs_ioctl_tree_search(struct file *file,
 	return ret;
 }
 
+static noinline int btrfs_ioctl_tree_search_v2(struct file *file,
+					       void __user *argp)
+{
+	struct btrfs_ioctl_search_args_v2 __user *uarg;
+	struct btrfs_ioctl_search_args_v2 args;
+	struct inode *inode;
+	int ret;
+	size_t buf_size;
+	const size_t buf_limit = 16 * 1024 * 1024;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	/* copy search header and buffer size */
+	uarg = (struct btrfs_ioctl_search_args_v2 __user *)argp;
+	if (copy_from_user(&args, uarg, sizeof(args)))
+		return -EFAULT;
+
+	buf_size = args.buf_size;
+
+	if (buf_size < sizeof(struct btrfs_ioctl_search_header))
+		return -EOVERFLOW;
+
+	/* limit result size to 16MB */
+	if (buf_size > buf_limit)
+		buf_size = buf_limit;
+
+	inode = file_inode(file);
+	ret = search_ioctl(inode, &args.key, &buf_size, &uarg->buf[0]);
+	if (ret == 0 && copy_to_user(&uarg->key, &args.key, sizeof(args.key)))
+		ret = -EFAULT;
+	else if (ret == -EOVERFLOW &&
+		copy_to_user(&uarg->buf_size, &buf_size, sizeof(buf_size)))
+		ret = -EFAULT;
+
+	return ret;
+}
+
 /*
  * Search INODE_REFs to identify path name of 'dirid' directory
  * in a 'tree_id' tree. and sets path name to 'name'.
@@ -4825,6 +4863,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_trans_end(file);
 	case BTRFS_IOC_TREE_SEARCH:
 		return btrfs_ioctl_tree_search(file, argp);
+	case BTRFS_IOC_TREE_SEARCH_V2:
+		return btrfs_ioctl_tree_search_v2(file, argp);
 	case BTRFS_IOC_INO_LOOKUP:
 		return btrfs_ioctl_ino_lookup(file, argp);
 	case BTRFS_IOC_INO_PATHS:
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 1b8a0f4..beeda99 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -301,6 +301,14 @@ struct btrfs_ioctl_search_args {
 	char buf[BTRFS_SEARCH_ARGS_BUFSIZE];
 };
 
+struct btrfs_ioctl_search_args_v2 {
+	struct btrfs_ioctl_search_key key; /* in/out - search parameters */
+	size_t buf_size;		   /* in - size of buffer
+					    * out - on EOVERFLOW: needed size
+					    *       to store item */
+	char buf[0];                       /* out - found items */
+};
+
 struct btrfs_ioctl_clone_range_args {
   __s64 src_fd;
   __u64 src_offset, src_length;
@@ -553,6 +561,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 				struct btrfs_ioctl_defrag_range_args)
 #define BTRFS_IOC_TREE_SEARCH _IOWR(BTRFS_IOCTL_MAGIC, 17, \
 				   struct btrfs_ioctl_search_args)
+#define BTRFS_IOC_TREE_SEARCH_V2 _IOWR(BTRFS_IOCTL_MAGIC, 17, \
+					   struct btrfs_ioctl_search_args_v2)
 #define BTRFS_IOC_INO_LOOKUP _IOWR(BTRFS_IOCTL_MAGIC, 18, \
 				   struct btrfs_ioctl_ino_lookup_args)
 #define BTRFS_IOC_DEFAULT_SUBVOL _IOW(BTRFS_IOCTL_MAGIC, 19, __u64)
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RFCv4] new ioctl TREE_SEARCH_V2
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (6 preceding siblings ...)
  2014-01-30 15:24 ` [PATCH RFCv4 7/7] btrfs: new ioctl TREE_SEARCH_V2 Gerhard Heift
@ 2014-01-31 17:40 ` David Sterba
  2014-04-04 11:36 ` David Sterba
  8 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2014-01-31 17:40 UTC (permalink / raw)
  To: Gerhard Heift; +Cc: linux-btrfs

On Thu, Jan 30, 2014 at 04:23:56PM +0100, Gerhard Heift wrote:
> This patch series first rewrites tree_search to copy found items directly to
> userspace and then adds a new ioctl TREE_SEARCH_V2 with which we could store
> them in a varying buffer. Now even items larger than 3992 bytes or a large
> amount of items can be returned. This is the case for some EXTENT_CSUM items,
> which could have a size up to 16k.

>   Should we limit the buffer size?
>   * David suggested [1] a minimum of 64k, I've chosen a cap of 16M.

Well, 64M is a lot, but as it's only an upper limit, ok.

I have scrolled through the patches, overall i looks ok, can IMO go into
btrfs-next, but more testing is obviously needed. I'll stick a
reviewed-by once I give it a bit of testing myself.


thanks,
david

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFCv4] new ioctl TREE_SEARCH_V2
  2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
                   ` (7 preceding siblings ...)
  2014-01-31 17:40 ` [PATCH RFCv4] " David Sterba
@ 2014-04-04 11:36 ` David Sterba
  8 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2014-04-04 11:36 UTC (permalink / raw)
  To: clm; +Cc: Gerhard Heift, linux-btrfs

Hi Chris,

On Thu, Jan 30, 2014 at 04:23:56PM +0100, Gerhard Heift wrote:
> This patch series first rewrites tree_search to copy found items directly to
> userspace and then adds a new ioctl TREE_SEARCH_V2 with which we could store
> them in a varying buffer. Now even items larger than 3992 bytes or a large
> amount of items can be returned. This is the case for some EXTENT_CSUM items,
> which could have a size up to 16k.

can you add this patchset to 3.15 queue? It hasn't gone through
btrfs-next for unknown reasons, but it should be merged because it makes
search ioctl usable with bigblocks that are now > 4k by default.

The newly added functionality is localized, I don't see huge risks adding
it even it was not in -next yet.

thanks,
david

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-04-04 11:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-30 15:23 [PATCH RFCv4] new ioctl TREE_SEARCH_V2 Gerhard Heift
2014-01-30 15:23 ` [PATCH RFCv4 1/7] btrfs: tree_search: eliminate redundant nr_items check Gerhard Heift
2014-01-30 15:23 ` [PATCH RFCv4 2/7] btrfs: tree_search, search_ioctl: accept varying buffer Gerhard Heift
2014-01-30 15:23 ` [PATCH RFCv4 3/7] btrfs: tree_search, copy_to_sk: return EOVERFLOW for too small buffer Gerhard Heift
2014-01-30 15:24 ` [PATCH RFCv4 4/7] btrfs: tree_search, copy_to_sk: return needed size on EOVERFLOW Gerhard Heift
2014-01-30 15:24 ` [PATCH RFCv4 5/7] btrfs: new function read_extent_buffer_to_user Gerhard Heift
2014-01-30 15:24 ` [PATCH RFCv4 6/7] btrfs: tree_search, search_ioctl: direct copy to userspace Gerhard Heift
2014-01-30 15:24 ` [PATCH RFCv4 7/7] btrfs: new ioctl TREE_SEARCH_V2 Gerhard Heift
2014-01-31 17:40 ` [PATCH RFCv4] " David Sterba
2014-04-04 11:36 ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.