Re: [PATCH v2 1/5] add metadata_incore ioctl in vfs

Previous thread: [PATCH v2 0/5] add new ioctls to do metadata readahead in btrfs by Shaohua Li on Monday, January 3, 2011 - 10:40 pm. (2 messages)

Next thread: [PATCH v2 2/5] implement metadata_incore in btrfs by Shaohua Li on Monday, January 3, 2011 - 10:40 pm. (1 message)
From: Shaohua Li
Date: Monday, January 3, 2011 - 10:40 pm

Add an ioctl to dump filesystem's metadata in memory in vfs. Userspace collects
such info and uses it to do metadata readahead.
Filesystem can hook to super_operations.metadata_incore to get metadata in
specific approach. Next patch will give an example how to implement
.metadata_incore in btrfs.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

---
 fs/compat_ioctl.c  |    1 
 fs/ioctl.c         |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |   15 ++++++++++
 3 files changed, 91 insertions(+)

Index: linux/fs/ioctl.c
===================================================================
--- linux.orig/fs/ioctl.c	2010-12-31 08:52:00.000000000 +0800
+++ linux/fs/ioctl.c	2011-01-03 21:15:48.000000000 +0800
@@ -530,6 +530,78 @@ static int ioctl_fsthaw(struct file *fil
 }
 
 /*
+ * Copy info about metadata in memory to userspace
+ * Returns:
+ * > 0, number of metadata_incore_ent entries copied to userspace
+ * = 0, no more metadata
+ * < 0, error
+ */
+static int ioctl_metadata_incore(struct file *filp, void __user *argp)
+{
+	struct super_block *sb = filp->f_path.dentry->d_inode->i_sb;
+	struct metadata_incore_args args;
+	struct metadata_incore_ent ent;
+	loff_t offset, last_offset = 0;
+	ssize_t size, last_size = 0;
+	__u64 __user vec_addr;
+	int entries = 0;
+
+	if (!sb->s_op->metadata_incore)
+		return -EINVAL;
+
+	if (copy_from_user(&args, argp, sizeof(args)))
+		return -EFAULT;
+
+	/* we check metadata info in page unit */
+	if (args.offset & ~PAGE_CACHE_MASK)
+		return -EINVAL;
+
+	if ((args.vec_size % sizeof(struct metadata_incore_ent)) != 0)
+		return -EINVAL;
+
+	offset = args.offset;
+
+	ent.unused = 0;
+	vec_addr = args.vec_addr;
+
+	while (vec_addr < args.vec_addr + args.vec_size) {
+		if (signal_pending(current))
+			return -EINTR;
+		cond_resched();
+
+		if (sb->s_op->metadata_incore(sb, &offset, &size) < 0)
+			break;
+		/* A merge or offset == 0 */
+		if (offset == last_offset + last_size) ...
From: Arnd Bergmann
Date: Tuesday, January 4, 2011 - 2:40 am

__user only makes sense on pointers. Just make this a "struct
metadata_incore_ent __user *", which will also take care of the

We usually try hard to avoid ioctls with indirect pointers
in them. The implementation is correct (most people
get this wrong), besides the extraneous __user keyword in
there.

Have you tried passing just a single metadata_incore_ent
at the ioctl and looping in user space? I would guess the
extra overhead of that would be small enough, but that might
need to be measured.

	Arnd
--

Previous thread: [PATCH v2 0/5] add new ioctls to do metadata readahead in btrfs by Shaohua Li on Monday, January 3, 2011 - 10:40 pm. (2 messages)

Next thread: [PATCH v2 2/5] implement metadata_incore in btrfs by Shaohua Li on Monday, January 3, 2011 - 10:40 pm. (1 message)