Re: [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6101 bytes --]

Hi Goldwyn,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on next-20201218]
[cannot apply to btrfs/next v5.10]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Goldwyn-Rodrigues/btrfs-Make-btrfs_direct_write-atomic-with-respect-to-inode_lock/20201219-001114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-a013-20201217 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/e7a8dd2d9537a7ec5aeadb002fc4934be1a396eb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Goldwyn-Rodrigues/btrfs-Make-btrfs_direct_write-atomic-with-respect-to-inode_lock/20201219-001114
        git checkout e7a8dd2d9537a7ec5aeadb002fc4934be1a396eb
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/file.c:1975:7: warning: variable 'dsync' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                   if (iocb->ki_flags & IOCB_DSYNC) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/file.c:1980:7: note: uninitialized use occurs here
                   if (dsync)
                       ^~~~~
   fs/btrfs/file.c:1975:3: note: remove the 'if' if its condition is always true
                   if (iocb->ki_flags & IOCB_DSYNC) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/file.c:1922:12: note: initialize the variable 'dsync' to silence this warning
           bool dsync;
                     ^
                      = 0
   1 warning generated.


vim +1975 fs/btrfs/file.c

  1909	
  1910	static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
  1911	{
  1912		struct file *file = iocb->ki_filp;
  1913		struct inode *inode = file_inode(file);
  1914		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
  1915		loff_t pos;
  1916		ssize_t written = 0;
  1917		ssize_t written_buffered;
  1918		loff_t endbyte;
  1919		ssize_t err;
  1920		unsigned int ilock_flags = 0;
  1921		struct iomap_dio *dio = NULL;
  1922		bool dsync;
  1923	
  1924		if (iocb->ki_flags & IOCB_NOWAIT)
  1925			ilock_flags |= BTRFS_ILOCK_TRY;
  1926	
  1927		/* If the write DIO is within EOF, use a shared lock */
  1928		if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
  1929			ilock_flags |= BTRFS_ILOCK_SHARED;
  1930	
  1931	relock:
  1932		err = btrfs_inode_lock(inode, ilock_flags);
  1933		if (err < 0)
  1934			return err;
  1935	
  1936		err = generic_write_checks(iocb, from);
  1937		if (err <= 0) {
  1938			btrfs_inode_unlock(inode, ilock_flags);
  1939			return err;
  1940		}
  1941	
  1942		err = btrfs_write_check(iocb, from, err);
  1943		if (err < 0)
  1944			goto out;
  1945	
  1946		pos = iocb->ki_pos;
  1947		/*
  1948		 * Re-check since file size may have changed just before taking the
  1949		 * lock or pos may have changed because of O_APPEND in generic_write_check()
  1950		 */
  1951		if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
  1952		    pos + iov_iter_count(from) > i_size_read(inode)) {
  1953			btrfs_inode_unlock(inode, ilock_flags);
  1954			ilock_flags &= ~BTRFS_ILOCK_SHARED;
  1955			goto relock;
  1956		}
  1957	
  1958		if (check_direct_IO(fs_info, from, pos))
  1959			goto buffered;
  1960	
  1961		dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops,
  1962				     &btrfs_dio_ops, is_sync_kiocb(iocb));
  1963	
  1964	
  1965		if (IS_ERR_OR_NULL(dio)) {
  1966			err = PTR_ERR_OR_ZERO(dio);
  1967			if (err < 0 && err != -ENOTBLK)
  1968				goto out;
  1969		} else {
  1970			/*
  1971			 * Remove the DSYNC flag because iomap_dio_complete() calls
  1972			 * generic_write_sync() which may deadlock with btrfs_sync()
  1973			 * on inode->i_rwsem
  1974			 */
> 1975			if (iocb->ki_flags & IOCB_DSYNC) {
  1976				dsync = true;
  1977				iocb->ki_flags &= ~IOCB_DSYNC;
  1978			}
  1979			written = iomap_dio_complete(dio);
  1980			if (dsync)
  1981				iocb->ki_flags |= IOCB_DSYNC;
  1982		}
  1983	
  1984		if (written < 0 || !iov_iter_count(from)) {
  1985			err = written;
  1986			goto out;
  1987		}
  1988	
  1989	buffered:
  1990		pos = iocb->ki_pos;
  1991		written_buffered = __btrfs_buffered_write(iocb, from);
  1992		if (written_buffered < 0) {
  1993			err = written_buffered;
  1994			goto out;
  1995		}
  1996		/*
  1997		 * Ensure all data is persisted. We want the next direct IO read to be
  1998		 * able to read what was just written.
  1999		 */
  2000		endbyte = pos + written_buffered - 1;
  2001		err = btrfs_fdatawrite_range(inode, pos, endbyte);
  2002		if (err)
  2003			goto out;
  2004		err = filemap_fdatawait_range(inode->i_mapping, pos, endbyte);
  2005		if (err)
  2006			goto out;
  2007		written += written_buffered;
  2008		iocb->ki_pos = pos + written_buffered;
  2009		invalidate_mapping_pages(file->f_mapping, pos >> PAGE_SHIFT,
  2010					 endbyte >> PAGE_SHIFT);
  2011	out:
  2012		btrfs_inode_unlock(inode, ilock_flags);
  2013		if (written > 0 && dsync)
  2014			generic_write_sync(iocb, written);
  2015	
  2016		return written ? written : err;
  2017	}
  2018	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39290 bytes --]