Btrfs devel - Dec 2010 - [PATCH] BTRFS_IOC_TREE_SEARCH: store and use the last key found

Hi all,

following the thread about the BTRFS_IOC_TREE_SEARCH ioctl [1], I made a patch 
which try to address the problem of restarting the ioctl.

In the current solution is the application during the restart of the search to 
fill the min_* fields in the "struct btrfs_ioctl_search_key". In
general the
values set are the last one returned "+1". But doing so we reduce the 
"acceptance criteria window".

I propose to add three more fields in the struct btrfs_ioctl_search_key:

+       /* starting search key fields */
+       __u32 start_type;
+       __u64 start_objectid;
+       __u64 start_offset;
 
Is resposibility of the program to fill these fields during the begin of the 
search. If these are 0, the code set these to the min_* values.
At the end of the search the kernel code stores the last key found + 1, in 
accordance of the "acceptance criteria". So for the next search these
fields
are "ready to use".

I didn''t increase the size of the "struct
btrfs_ioctl_search_key", because I
used some un-used fields. 
I left the old code untouched, and I added another IOCTL, putting a warning in 
the old one. If we accept to break the binary compatibility we can reuse the 
old IOCTL.

This patch is complementary to the Li Zefan ones [1], so for the best 
correctness both have to be applied.

Comments are welcome
Regards
G.Baroncelli

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index f87552a..52075ed 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1134,11 +1134,11 @@ advance_key:
 	if (key->offset < (u64)-1 && key->offset <
sk->max_offset)
 		key->offset++;
 	else if (key->type < (u8)-1 && key->type <
sk->max_type) {
-		key->offset = 0;
+		key->offset = sk->min_offset;
 		key->type++;
 	} else if (key->objectid < (u64)-1 && key->objectid <
sk-
>max_objectid) {
-		key->offset = 0;
-		key->type = 0;
+		key->offset = sk->min_offset;
+		key->type = sk->min_type;
 		key->objectid++;
 	} else
 		ret = 1;
@@ -1180,9 +1180,9 @@ static noinline int search_ioctl(struct inode *inode,
 		}
 	}
 
-	key.objectid = sk->min_objectid;
-	key.type = sk->min_type;
-	key.offset = sk->min_offset;
+	key.objectid = max(sk->start_objectid, sk->min_objectid);
+	key.type = max(sk->start_type,sk->min_type);
+	key.offset = max(sk->start_offset, sk->min_offset);
 
 	max_key.objectid = sk->max_objectid;
 	max_key.type = sk->max_type;
@@ -1207,6 +1207,11 @@ static noinline int search_ioctl(struct inode *inode,
 	}
 	ret = 0;
 err:
+	/* save the key for an hypothetic next iteration */
+	sk->start_objectid = key.objectid;
+	sk->start_type = key.type;
+	sk->start_offset = key.offset;
+
 	sk->nr_items = num_found;
 	btrfs_free_path(path);
 	return ret;
@@ -1234,6 +1239,39 @@ static noinline int btrfs_ioctl_tree_search(struct file 
*file,
 	return ret;
 }
 
+static noinline int btrfs_ioctl_tree_search_old(struct file *file,
+					   void __user *argp)
+{
+	struct btrfs_ioctl_search_args *args;
+	struct inode *inode;
+	int ret;
+
+	printk(KERN_WARNING "BTRFS: Pid=%d(%s) is using the buggy "
+			"BTRFS_IOC_TREE_SEARCH_V0 ioctl\n", 
+			current->pid, current->comm);
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	args = memdup_user(argp, sizeof(*args));
+	if (IS_ERR(args))
+		return PTR_ERR(args);
+
+	inode = fdentry(file)->d_inode;
+
+	/* for compatibility */
+	args->key.start_objectid = 0;
+	args->key.start_type = 0;
+	args->key.start_offset = 0;
+
+	ret = search_ioctl(inode, args);
+	if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
+		ret = -EFAULT;
+	kfree(args);
+	return ret;
+}
+
+
 /*
  * Search INODE_REFs to identify path name of ''dirid''
directory
  * in a ''tree_id'' tree. and sets path name to
''name''.
@@ -2286,6 +2324,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_trans_start(file);
 	case BTRFS_IOC_TRANS_END:
 		return btrfs_ioctl_trans_end(file);
+	case BTRFS_IOC_TREE_SEARCH_V0:
+		return btrfs_ioctl_tree_search_old(file, argp);
 	case BTRFS_IOC_TREE_SEARCH:
 		return btrfs_ioctl_tree_search(file, argp);
 	case BTRFS_IOC_INO_LOOKUP:
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index c344d12..41dfde8 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -74,12 +74,12 @@ struct btrfs_ioctl_search_key {
 	 */
 	__u32 nr_items;
 
-	/* align to 64 bits */
-	__u32 unused;
+	/* starting search key fields */
+	__u32 start_type;
+	__u64 start_objectid;
+	__u64 start_offset;
 
 	/* some extra for later */
-	__u64 unused1;
-	__u64 unused2;
 	__u64 unused3;
 	__u64 unused4;
 };
@@ -182,7 +182,12 @@ struct btrfs_ioctl_space_args {
 				struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG_RANGE _IOW(BTRFS_IOCTL_MAGIC, 16, \
 				struct btrfs_ioctl_defrag_range_args)
-#define BTRFS_IOC_TREE_SEARCH _IOWR(BTRFS_IOCTL_MAGIC, 17, \
+
+/* buggy, don''t use */
+
+#define BTRFS_IOC_TREE_SEARCH_V0 _IOWR(BTRFS_IOCTL_MAGIC, 17, \
+				   struct btrfs_ioctl_search_args)
+#define BTRFS_IOC_TREE_SEARCH_V1 _IOWR(BTRFS_IOCTL_MAGIC, 25, \
 				   struct btrfs_ioctl_search_args)
 #define BTRFS_IOC_INO_LOOKUP _IOWR(BTRFS_IOCTL_MAGIC, 18, \
 				   struct btrfs_ioctl_ino_lookup_args)


[1] http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg07513.html

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijack@inwind.it>
Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512