diff mbox

[3/6] Btrfs: refactor btrfs_file_write_iter

Message ID 1481147110-20048-4-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo Dec. 7, 2016, 9:45 p.m. UTC
This adds a helper function btrfs_file_write_check for file checks, permission
check and necessary time and size extention.

With this, we simplify btrfs_file_write_iter by putting details into seperated
buffered_write and direct_write callback.

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

Comments

kernel test robot Dec. 8, 2016, 12:44 a.m. UTC | #1
Hi Liu,

[auto build test WARNING on tip/perf/core]
[also build test WARNING on v4.9-rc8 next-20161207]
[cannot apply to btrfs/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Liu-Bo/btrfs-dax-IO/20161208-082651
config: x86_64-randconfig-x001-201649 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   fs/btrfs/file.c: In function 'btrfs_file_write_iter':
>> fs/btrfs/file.c:1823:5: warning: 'oldsize' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (oldsize < pos)
        ^
   fs/btrfs/file.c:1809:9: note: 'oldsize' was declared here
     loff_t oldsize;
            ^~~~~~~

vim +/oldsize +1823 fs/btrfs/file.c

  1807		struct inode *inode = file_inode(file);
  1808		loff_t pos;
  1809		loff_t oldsize;
  1810		ssize_t ret;
  1811	
  1812		inode_lock(inode);
  1813		ret = btrfs_file_write_check(iocb, from);
  1814		if (ret)
  1815			goto out;
  1816	
  1817		current->backing_dev_info = inode_to_bdi(inode);
  1818	
  1819		pos = iocb->ki_pos;
  1820		ret = __btrfs_buffered_write(file, from, pos);
  1821		if (ret > 0)
  1822			iocb->ki_pos = pos + ret;
> 1823		if (oldsize < pos)
  1824			pagecache_isize_extended(inode, oldsize,
  1825						i_size_read(inode));
  1826	
  1827		current->backing_dev_info = NULL;
  1828	out:
  1829		inode_unlock(inode);
  1830		return ret;
  1831	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 3a14c87..06e55e8 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -44,6 +44,7 @@ 
 #include "compression.h"
 
 static struct kmem_cache *btrfs_inode_defrag_cachep;
+static ssize_t btrfs_file_write_check(struct kiocb *iocb, struct iov_iter *from);
 /*
  * when auto defrag is enabled we
  * queue up these defrag structs to remember which
@@ -1735,20 +1736,25 @@  static noinline ssize_t __btrfs_buffered_write(struct file *file,
 	return num_written ? num_written : ret;
 }
 
-static ssize_t __btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
+static ssize_t btrfs_file_direct_write(struct kiocb *iocb,
+				       struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file_inode(file);
 	loff_t pos = iocb->ki_pos;
-	ssize_t written;
+	ssize_t written = 0;
 	ssize_t written_buffered;
 	loff_t endbyte;
 	int err;
 
-	written = generic_file_direct_write(iocb, from);
+	inode_lock(inode);
+	err = btrfs_file_write_check(iocb, from);
+	if (err)
+		goto out;
 
+	written = generic_file_direct_write(iocb, from);
 	if (written < 0 || !iov_iter_count(from))
-		return written;
+		goto out;
 
 	pos += written;
 	written_buffered = __btrfs_buffered_write(file, from, pos);
@@ -1772,6 +1778,7 @@  static ssize_t __btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 	invalidate_mapping_pages(file->f_mapping, pos >> PAGE_SHIFT,
 				 endbyte >> PAGE_SHIFT);
 out:
+	inode_unlock(inode);
 	return written ? written : err;
 }
 
@@ -1793,47 +1800,56 @@  static void update_time_for_write(struct inode *inode)
 		inode_inc_iversion(inode);
 }
 
-static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
-				    struct iov_iter *from)
+static ssize_t btrfs_file_buffered_write(struct kiocb *iocb,
+					 struct iov_iter *from)
+{
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file_inode(file);
+	loff_t pos;
+	loff_t oldsize;
+	ssize_t ret;
+
+	inode_lock(inode);
+	ret = btrfs_file_write_check(iocb, from);
+	if (ret)
+		goto out;
+
+	current->backing_dev_info = inode_to_bdi(inode);
+
+	pos = iocb->ki_pos;
+	ret = __btrfs_buffered_write(file, from, pos);
+	if (ret > 0)
+		iocb->ki_pos = pos + ret;
+	if (oldsize < pos)
+		pagecache_isize_extended(inode, oldsize,
+					i_size_read(inode));
+
+	current->backing_dev_info = NULL;
+out:
+	inode_unlock(inode);
+	return ret;
+}
+
+static ssize_t btrfs_file_write_check(struct kiocb *iocb,
+				      struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file_inode(file);
 	struct btrfs_root *root = BTRFS_I(inode)->root;
-	u64 start_pos;
-	u64 end_pos;
-	ssize_t num_written = 0;
-	bool sync = (file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host);
 	ssize_t err;
 	loff_t pos;
 	size_t count;
 	loff_t oldsize;
-	int clean_page = 0;
+	u64 start_pos;
+	u64 end_pos;
 
-	inode_lock(inode);
 	err = generic_write_checks(iocb, from);
-	if (err <= 0) {
-		inode_unlock(inode);
+	if (err <= 0)
 		return err;
-	}
 
-	current->backing_dev_info = inode_to_bdi(inode);
 	err = file_remove_privs(file);
-	if (err) {
-		inode_unlock(inode);
-		goto out;
-	}
-
-	/*
-	 * If BTRFS flips readonly due to some impossible error
-	 * (fs_info->fs_state now has BTRFS_SUPER_FLAG_ERROR),
-	 * although we have opened a file as writable, we have
-	 * to stop this write operation to ensure FS consistency.
-	 */
-	if (test_bit(BTRFS_FS_STATE_ERROR, &root->fs_info->fs_state)) {
-		inode_unlock(inode);
-		err = -EROFS;
-		goto out;
-	}
+	if (err)
+		return err;
 
 	/*
 	 * We reserve space for updating the inode when we reserve space for the
@@ -1851,30 +1867,43 @@  static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 		/* Expand hole size to cover write data, preventing empty gap */
 		end_pos = round_up(pos + count, root->sectorsize);
 		err = btrfs_cont_expand(inode, oldsize, end_pos);
-		if (err) {
-			inode_unlock(inode);
-			goto out;
-		}
-		if (start_pos > round_up(oldsize, root->sectorsize))
-			clean_page = 1;
+		if (err)
+			return err;
 	}
 
+	return 0;
+}
+
+static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
+				    struct iov_iter *from)
+{
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file_inode(file);
+	struct btrfs_root *root = BTRFS_I(inode)->root;
+	ssize_t num_written = 0;
+	bool sync = (file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host);
+
+	if (!iov_iter_count(from))
+		return 0;
+
+	/*
+	 * If BTRFS flips readonly due to some impossible error
+	 * (fs_info->fs_state now has BTRFS_SUPER_FLAG_ERROR),
+	 * although we have opened a file as writable, we have
+	 * to stop this write operation to ensure FS consistency.
+	 */
+	if (test_bit(BTRFS_FS_STATE_ERROR, &root->fs_info->fs_state))
+		return -EROFS;
+
 	if (sync)
 		atomic_inc(&BTRFS_I(inode)->sync_writers);
 
 	if (iocb->ki_flags & IOCB_DIRECT) {
-		num_written = __btrfs_direct_write(iocb, from);
+		num_written = btrfs_file_direct_write(iocb, from);
 	} else {
-		num_written = __btrfs_buffered_write(file, from, pos);
-		if (num_written > 0)
-			iocb->ki_pos = pos + num_written;
-		if (clean_page)
-			pagecache_isize_extended(inode, oldsize,
-						i_size_read(inode));
+		num_written = btrfs_file_buffered_write(iocb, from);
 	}
 
-	inode_unlock(inode);
-
 	/*
 	 * We also have to set last_sub_trans to the current log transid,
 	 * otherwise subsequent syncs to a file that's been synced in this
@@ -1888,9 +1917,8 @@  static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 
 	if (sync)
 		atomic_dec(&BTRFS_I(inode)->sync_writers);
-out:
-	current->backing_dev_info = NULL;
-	return num_written ? num_written : err;
+
+	return num_written;
 }
 
 int btrfs_release_file(struct inode *inode, struct file *filp)