[PATCH] Btrfs: copy the certain type of item if min_type equals to max_type

[PATCH] Btrfs: copy the certain type of item if min_type equals to max_type

[Liu Bo]

Some tools in btrfs-progs utilize ioctl 'BTRFS_IOC_TREE_SEARCH' and
ioctl 'BTRFS_IOC_TREE_SEARCH_V2' to look up metadata btree for what
they want, and several tools in fact only look for one certain type,
where they set a certain value for both 'sk->min_type' and 'sk->max_type'.

For example,
if we want to get the information of block groups, the current btrfs
searches extent_tree and returns not only block groups's items, but also
EXTENT_ITEM's items which could cost a large amount of user's buffer,
and tools then needs to read the buffer and spends several loops to
pick up what they want.

This lets the above two ioctl only return the certain type of items
that tools wants.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/ioctl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index da94138..f795423 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1911,6 +1911,10 @@ static noinline int key_in_sk(struct btrfs_key *key,
 	struct btrfs_key test;
 	int ret;
 
+	/* All we want is this type of key. */
+	if (sk->min_type == sk->max_type && key->type != sk->min_type)
+		return 0;
+
 	test.objectid = sk->min_objectid;
 	test.type = sk->min_type;
 	test.offset = sk->min_offset;
-- 
2.5.0

Re: [PATCH] Btrfs: copy the certain type of item if min_type equals to max_type

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 2965 bytes --]

On Fri, Jan 15, 2016 at 12:52:28PM -0800, Liu Bo wrote:
> Some tools in btrfs-progs utilize ioctl 'BTRFS_IOC_TREE_SEARCH' and
> ioctl 'BTRFS_IOC_TREE_SEARCH_V2' to look up metadata btree for what
> they want, and several tools in fact only look for one certain type,
> where they set a certain value for both 'sk->min_type' and 'sk->max_type'.
> 
> For example,
> if we want to get the information of block groups, the current btrfs
> searches extent_tree and returns not only block groups's items, but also
> EXTENT_ITEM's items which could cost a large amount of user's buffer,
> and tools then needs to read the buffer and spends several loops to
> pick up what they want.
> 
> This lets the above two ioctl only return the certain type of items
> that tools wants.

   This changes the semantics of the ioctl in a subtle and
incompatible way.

   The keyspace used by btrfs trees can be viewed in two different and
semantically incompatible ways. A key is an (Ob, T, Of) tuple. The
first way of looking at this is as a one-dimensional keyspace, ordered
lexically, as Ob+T+Of. This is what btrfs uses internally, and it's
the way that the TREE_SEARCH ioctl works. A search simply returns a
linear subset of the keys between the minimum and the maximum.

   The other view of the keyspace, which is more useful in some
circumstances, is of a 3-dimensional keyspace, with the obvious
lattice-like ordering, where K1 <= K2 iff Ob1 <= Ob2 and T1 <= T2 and
Of1 <= Of2. This offers a very different interpretation of searching,
where you are carving out a rectangular block of the 3-dimensional
keyspace. This is the behaviour you're trying to impose on the search
ioctl for a specific special case of search.

   I would argue that if you want to have the second form of search
(and it's a useful one, certainly), you should implement an
alternative search ioctl, rather than trying to retrofit that
behaviour on something with very different, already well-defined
semantics.

   In other words, this change makes for an awkward and confusing
interface, and I think it shouldn't be done this way.

   Hugo.

> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>  fs/btrfs/ioctl.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index da94138..f795423 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1911,6 +1911,10 @@ static noinline int key_in_sk(struct btrfs_key *key,
>  	struct btrfs_key test;
>  	int ret;
>  
> +	/* All we want is this type of key. */
> +	if (sk->min_type == sk->max_type && key->type != sk->min_type)
> +		return 0;
> +
>  	test.objectid = sk->min_objectid;
>  	test.type = sk->min_type;
>  	test.offset = sk->min_offset;

-- 
Hugo Mills             | "He's a nutcase, you know. There's no getting away
hugo@... carfax.org.uk | from it -- he'll end up with a knighthood"
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                         Lexy, The League of Gentlemen

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Btrfs: copy the certain type of item if min_type equals to max_type

[Liu Bo]

On Fri, Jan 15, 2016 at 09:18:41PM +0000, Hugo Mills wrote:
> On Fri, Jan 15, 2016 at 12:52:28PM -0800, Liu Bo wrote:
> > Some tools in btrfs-progs utilize ioctl 'BTRFS_IOC_TREE_SEARCH' and
> > ioctl 'BTRFS_IOC_TREE_SEARCH_V2' to look up metadata btree for what
> > they want, and several tools in fact only look for one certain type,
> > where they set a certain value for both 'sk->min_type' and 'sk->max_type'.
> > 
> > For example,
> > if we want to get the information of block groups, the current btrfs
> > searches extent_tree and returns not only block groups's items, but also
> > EXTENT_ITEM's items which could cost a large amount of user's buffer,
> > and tools then needs to read the buffer and spends several loops to
> > pick up what they want.
> > 
> > This lets the above two ioctl only return the certain type of items
> > that tools wants.
> 
>    This changes the semantics of the ioctl in a subtle and
> incompatible way.
> 
>    The keyspace used by btrfs trees can be viewed in two different and
> semantically incompatible ways. A key is an (Ob, T, Of) tuple. The
> first way of looking at this is as a one-dimensional keyspace, ordered
> lexically, as Ob+T+Of. This is what btrfs uses internally, and it's
> the way that the TREE_SEARCH ioctl works. A search simply returns a
> linear subset of the keys between the minimum and the maximum.
> 
>    The other view of the keyspace, which is more useful in some
> circumstances, is of a 3-dimensional keyspace, with the obvious
> lattice-like ordering, where K1 <= K2 iff Ob1 <= Ob2 and T1 <= T2 and
> Of1 <= Of2. This offers a very different interpretation of searching,
> where you are carving out a rectangular block of the 3-dimensional
> keyspace. This is the behaviour you're trying to impose on the search
> ioctl for a specific special case of search.
> 
>    I would argue that if you want to have the second form of search
> (and it's a useful one, certainly), you should implement an
> alternative search ioctl, rather than trying to retrofit that
> behaviour on something with very different, already well-defined
> semantics.
> 
>    In other words, this change makes for an awkward and confusing
> interface, and I think it shouldn't be done this way.

OK, I also realize that we can do sk->min_type++ in some places, where
we will get wrong results.

Please ignore it.

Thanks,

-liubo

> 
>    Hugo.
> 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > ---
> >  fs/btrfs/ioctl.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index da94138..f795423 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -1911,6 +1911,10 @@ static noinline int key_in_sk(struct btrfs_key *key,
> >  	struct btrfs_key test;
> >  	int ret;
> >  
> > +	/* All we want is this type of key. */
> > +	if (sk->min_type == sk->max_type && key->type != sk->min_type)
> > +		return 0;
> > +
> >  	test.objectid = sk->min_objectid;
> >  	test.type = sk->min_type;
> >  	test.offset = sk->min_offset;
> 
> -- 
> Hugo Mills             | "He's a nutcase, you know. There's no getting away
> hugo@... carfax.org.uk | from it -- he'll end up with a knighthood"
> http://carfax.org.uk/  |
> PGP: E2AB1DE4          |                         Lexy, The League of Gentlemen