linux-kernel - [PATCH 28/52] Do fallocate() to grow file before mapping for file growing writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20181210171318.16998-29-vgoyal@redhat.com>
Date:   Mon, 10 Dec 2018 12:12:54 -0500
From:   Vivek Goyal <vgoyal@...hat.com>
To:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     vgoyal@...hat.com, miklos@...redi.hu, stefanha@...hat.com,
        dgilbert@...hat.com, sweil@...hat.com, swhiteho@...hat.com
Subject: [PATCH 28/52] Do fallocate() to grow file before mapping for file growing writes

How to handle file growing writes. For now, this patch does fallocate() to
grow file and then map it using dax. We need to figure out what's the best
way to handle it.

This patch does fallocate() and setup mapping operations in
fuse_dax_write_iter(), instead of iomap_begin(). I don't have access to file
pointer needed to send a message to fuse daemon in iomap_begin().

Dave Chinner has expressed concers with this approach as this is not
atomic. If guest crashes after falloc() but before data was written,
user will think that filesystem lost its data. So this is still an
outstanding issue.

Signed-off-by: Vivek Goyal <vgoyal@...hat.com>
---
 fs/fuse/file.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 94ad76382a6f..41d773ba2c72 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -28,6 +28,9 @@ INTERVAL_TREE_DEFINE(struct fuse_dax_mapping,
                      rb, __u64, __subtree_last,
                      START, LAST, static inline, fuse_dax_interval_tree);
 
+static long __fuse_file_fallocate(struct file *file, int mode,
+					loff_t offset, loff_t length);
+
 static int fuse_send_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
 			  int opcode, struct fuse_open_out *outargp)
 {
@@ -1819,6 +1822,22 @@ static ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	/* TODO file_update_time() but we don't want metadata I/O */
 
 	/* TODO handle growing the file */
+	/* Grow file here if need be. iomap_begin() does not have access
+	 * to file pointer
+	 */
+	if (iov_iter_rw(from) == WRITE &&
+	    ((iocb->ki_pos + iov_iter_count(from)) > i_size_read(inode))) {
+		ret = __fuse_file_fallocate(iocb->ki_filp, 0, iocb->ki_pos,
+						iov_iter_count(from));
+		if (ret < 0) {
+			printk("fallocate(offset=0x%llx length=0x%lx)"
+			" failed. err=%ld\n", iocb->ki_pos,
+			iov_iter_count(from), ret);
+			goto out;
+		}
+		pr_debug("fallocate(offset=0x%llx length=0x%lx)"
+		" succeed. ret=%ld\n", iocb->ki_pos, iov_iter_count(from), ret);
+	}
 
 	ret = dax_iomap_rw(iocb, from, &fuse_iomap_ops);
 
@@ -3331,8 +3350,12 @@ fuse_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 	return ret;
 }
 
-static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
-				loff_t length)
+/*
+ * This variant does not take any inode lock and if locking is required,
+ * caller is supposed to hold lock
+ */
+static long __fuse_file_fallocate(struct file *file, int mode,
+					loff_t offset, loff_t length)
 {
 	struct fuse_file *ff = file->private_data;
 	struct inode *inode = file_inode(file);
@@ -3346,8 +3369,6 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
 		.mode = mode
 	};
 	int err;
-	bool lock_inode = !(mode & FALLOC_FL_KEEP_SIZE) ||
-			   (mode & FALLOC_FL_PUNCH_HOLE);
 
 	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
 		return -EOPNOTSUPP;
@@ -3355,17 +3376,13 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
 	if (fc->no_fallocate)
 		return -EOPNOTSUPP;
 
-	if (lock_inode) {
-		inode_lock(inode);
-		if (mode & FALLOC_FL_PUNCH_HOLE) {
-			loff_t endbyte = offset + length - 1;
-			err = filemap_write_and_wait_range(inode->i_mapping,
-							   offset, endbyte);
-			if (err)
-				goto out;
-
-			fuse_sync_writes(inode);
-		}
+	if (mode & FALLOC_FL_PUNCH_HOLE) {
+		loff_t endbyte = offset + length - 1;
+		err = filemap_write_and_wait_range(inode->i_mapping, offset,
+							endbyte);
+		if (err)
+			goto out;
+		fuse_sync_writes(inode);
 	}
 
 	if (!(mode & FALLOC_FL_KEEP_SIZE))
@@ -3401,9 +3418,31 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
 	if (!(mode & FALLOC_FL_KEEP_SIZE))
 		clear_bit(FUSE_I_SIZE_UNSTABLE, &fi->state);
 
+	return err;
+}
+
+static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
+				loff_t length)
+{
+	struct fuse_file *ff = file->private_data;
+	struct inode *inode = file_inode(file);
+	struct fuse_conn *fc = ff->fc;
+	int err;
+	bool lock_inode = !(mode & FALLOC_FL_KEEP_SIZE) ||
+			   (mode & FALLOC_FL_PUNCH_HOLE);
+
+	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
+		return -EOPNOTSUPP;
+
+	if (fc->no_fallocate)
+		return -EOPNOTSUPP;
+
 	if (lock_inode)
-		inode_unlock(inode);
+		inode_lock(inode);
 
+	err = __fuse_file_fallocate(file, mode, offset, length);
+	if (lock_inode)
+		inode_unlock(inode);
 	return err;
 }
 
-- 
2.13.6