Re: [PATCH 3/6] Btrfs: refactor btrfs_file_write_iter

[kbuild test robot]

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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH 3/6] Btrfs: refactor btrfs_file_write_iter

[Liu Bo]

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 
---
 fs/btrfs/file.c | 128 ++--
 1 file changed, 78 insertions(+), 50 deletions(-)

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, >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