* Re: [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock
@ 2020-12-29 16:03 kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-29 16:03 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 10388 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <8846c296bcd4d5d3c21c6a98ee467ab5060c6757.1608307404.git.rgoldwyn@suse.com>
References: <8846c296bcd4d5d3c21c6a98ee467ab5060c6757.1608307404.git.rgoldwyn@suse.com>
TO: Goldwyn Rodrigues <rgoldwyn@suse.de>
TO: linux-btrfs(a)vger.kernel.org
CC: Goldwyn Rodrigues <rgoldwyn@suse.com>
Hi Goldwyn,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.11-rc1 next-20201223]
[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
:::::: branch date: 11 days ago
:::::: commit date: 11 days ago
config: x86_64-randconfig-m001-20201229 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
fs/btrfs/file.c:1980 btrfs_direct_write() error: uninitialized symbol 'dsync'.
Old smatch warnings:
fs/btrfs/file.c:1865 __btrfs_buffered_write() error: uninitialized symbol 'ret'.
fs/btrfs/file.c:2013 btrfs_direct_write() error: uninitialized symbol 'dsync'.
vim +/dsync +1980 fs/btrfs/file.c
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1909
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1910 static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
d0215f3e5ebb580 Josef Bacik 2011-01-25 1911 {
d0215f3e5ebb580 Josef Bacik 2011-01-25 1912 struct file *file = iocb->ki_filp;
728404dacfddb53 Filipe Manana 2014-10-10 1913 struct inode *inode = file_inode(file);
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1914 struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1915 loff_t pos;
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1916 ssize_t written = 0;
d0215f3e5ebb580 Josef Bacik 2011-01-25 1917 ssize_t written_buffered;
d0215f3e5ebb580 Josef Bacik 2011-01-25 1918 loff_t endbyte;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1919 ssize_t err;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1920 unsigned int ilock_flags = 0;
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1921 struct iomap_dio *dio = NULL;
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1922 bool dsync;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1923
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1924 if (iocb->ki_flags & IOCB_NOWAIT)
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1925 ilock_flags |= BTRFS_ILOCK_TRY;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1926
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1927 /* If the write DIO is within EOF, use a shared lock */
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1928 if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1929 ilock_flags |= BTRFS_ILOCK_SHARED;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1930
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1931 relock:
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1932 err = btrfs_inode_lock(inode, ilock_flags);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1933 if (err < 0)
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1934 return err;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1935
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1936 err = generic_write_checks(iocb, from);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1937 if (err <= 0) {
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1938 btrfs_inode_unlock(inode, ilock_flags);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1939 return err;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1940 }
d0215f3e5ebb580 Josef Bacik 2011-01-25 1941
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1942 err = btrfs_write_check(iocb, from, err);
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1943 if (err < 0)
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1944 goto out;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1945
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1946 pos = iocb->ki_pos;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1947 /*
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1948 * Re-check since file size may have changed just before taking the
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1949 * lock or pos may have changed because of O_APPEND in generic_write_check()
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1950 */
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1951 if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1952 pos + iov_iter_count(from) > i_size_read(inode)) {
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1953 btrfs_inode_unlock(inode, ilock_flags);
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1954 ilock_flags &= ~BTRFS_ILOCK_SHARED;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1955 goto relock;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1956 }
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1957
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1958 if (check_direct_IO(fs_info, from, pos))
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1959 goto buffered;
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1960
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1961 dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops,
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1962 &btrfs_dio_ops, is_sync_kiocb(iocb));
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1963
d0215f3e5ebb580 Josef Bacik 2011-01-25 1964
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1965 if (IS_ERR_OR_NULL(dio)) {
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1966 err = PTR_ERR_OR_ZERO(dio);
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1967 if (err < 0 && err != -ENOTBLK)
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1968 goto out;
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1969 } else {
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1970 /*
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1971 * Remove the DSYNC flag because iomap_dio_complete() calls
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1972 * generic_write_sync() which may deadlock with btrfs_sync()
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1973 * on inode->i_rwsem
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1974 */
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1975 if (iocb->ki_flags & IOCB_DSYNC) {
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1976 dsync = true;
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1977 iocb->ki_flags &= ~IOCB_DSYNC;
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1978 }
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1979 written = iomap_dio_complete(dio);
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 @1980 if (dsync)
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1981 iocb->ki_flags |= IOCB_DSYNC;
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1982 }
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1983
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1984 if (written < 0 || !iov_iter_count(from)) {
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1985 err = written;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1986 goto out;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1987 }
d0215f3e5ebb580 Josef Bacik 2011-01-25 1988
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1989 buffered:
e4af400a9c5081e Goldwyn Rodrigues 2018-06-17 1990 pos = iocb->ki_pos;
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1991 written_buffered = __btrfs_buffered_write(iocb, from);
d0215f3e5ebb580 Josef Bacik 2011-01-25 1992 if (written_buffered < 0) {
d0215f3e5ebb580 Josef Bacik 2011-01-25 1993 err = written_buffered;
d0215f3e5ebb580 Josef Bacik 2011-01-25 1994 goto out;
d0215f3e5ebb580 Josef Bacik 2011-01-25 1995 }
075bdbdbe9f21d6 Filipe Manana 2014-10-09 1996 /*
075bdbdbe9f21d6 Filipe Manana 2014-10-09 1997 * Ensure all data is persisted. We want the next direct IO read to be
075bdbdbe9f21d6 Filipe Manana 2014-10-09 1998 * able to read what was just written.
075bdbdbe9f21d6 Filipe Manana 2014-10-09 1999 */
d0215f3e5ebb580 Josef Bacik 2011-01-25 2000 endbyte = pos + written_buffered - 1;
728404dacfddb53 Filipe Manana 2014-10-10 2001 err = btrfs_fdatawrite_range(inode, pos, endbyte);
075bdbdbe9f21d6 Filipe Manana 2014-10-09 2002 if (err)
075bdbdbe9f21d6 Filipe Manana 2014-10-09 2003 goto out;
728404dacfddb53 Filipe Manana 2014-10-10 2004 err = filemap_fdatawait_range(inode->i_mapping, pos, endbyte);
d0215f3e5ebb580 Josef Bacik 2011-01-25 2005 if (err)
d0215f3e5ebb580 Josef Bacik 2011-01-25 2006 goto out;
d0215f3e5ebb580 Josef Bacik 2011-01-25 2007 written += written_buffered;
867c4f9329e1bf7 Al Viro 2014-02-11 2008 iocb->ki_pos = pos + written_buffered;
09cbfeaf1a5a67b Kirill A. Shutemov 2016-04-01 2009 invalidate_mapping_pages(file->f_mapping, pos >> PAGE_SHIFT,
09cbfeaf1a5a67b Kirill A. Shutemov 2016-04-01 2010 endbyte >> PAGE_SHIFT);
39279cc3d2704cf Chris Mason 2007-06-12 2011 out:
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 2012 btrfs_inode_unlock(inode, ilock_flags);
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 2013 if (written > 0 && dsync)
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 2014 generic_write_sync(iocb, written);
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 2015
d0215f3e5ebb580 Josef Bacik 2011-01-25 2016 return written ? written : err;
d0215f3e5ebb580 Josef Bacik 2011-01-25 2017 }
d0215f3e5ebb580 Josef Bacik 2011-01-25 2018
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39286 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock
2020-12-18 16:07 Goldwyn Rodrigues
2020-12-19 17:50 ` kernel test robot
@ 2021-01-04 10:10 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-01-04 10:10 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 7617 bytes --]
Hi Goldwyn,
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-m001-20201229 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
fs/btrfs/file.c:1980 btrfs_direct_write() error: uninitialized symbol 'dsync'.
Old smatch warnings:
fs/btrfs/file.c:1865 __btrfs_buffered_write() error: uninitialized symbol 'ret'.
fs/btrfs/file.c:2013 btrfs_direct_write() error: uninitialized symbol 'dsync'.
vim +/dsync +1980 fs/btrfs/file.c
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1910 static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
d0215f3e5ebb580 Josef Bacik 2011-01-25 1911 {
d0215f3e5ebb580 Josef Bacik 2011-01-25 1912 struct file *file = iocb->ki_filp;
728404dacfddb53 Filipe Manana 2014-10-10 1913 struct inode *inode = file_inode(file);
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1914 struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1915 loff_t pos;
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1916 ssize_t written = 0;
d0215f3e5ebb580 Josef Bacik 2011-01-25 1917 ssize_t written_buffered;
d0215f3e5ebb580 Josef Bacik 2011-01-25 1918 loff_t endbyte;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1919 ssize_t err;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1920 unsigned int ilock_flags = 0;
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1921 struct iomap_dio *dio = NULL;
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1922 bool dsync;
^^^^^^^^^^
This needs to be "bool dsync = false;"
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1923
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1924 if (iocb->ki_flags & IOCB_NOWAIT)
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1925 ilock_flags |= BTRFS_ILOCK_TRY;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1926
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1927 /* If the write DIO is within EOF, use a shared lock */
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1928 if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1929 ilock_flags |= BTRFS_ILOCK_SHARED;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1930
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1931 relock:
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1932 err = btrfs_inode_lock(inode, ilock_flags);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1933 if (err < 0)
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1934 return err;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1935
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1936 err = generic_write_checks(iocb, from);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1937 if (err <= 0) {
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1938 btrfs_inode_unlock(inode, ilock_flags);
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1939 return err;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1940 }
d0215f3e5ebb580 Josef Bacik 2011-01-25 1941
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1942 err = btrfs_write_check(iocb, from, err);
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1943 if (err < 0)
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1944 goto out;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1945
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1946 pos = iocb->ki_pos;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1947 /*
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1948 * Re-check since file size may have changed just before taking the
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1949 * lock or pos may have changed because of O_APPEND in generic_write_check()
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1950 */
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1951 if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1952 pos + iov_iter_count(from) > i_size_read(inode)) {
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1953 btrfs_inode_unlock(inode, ilock_flags);
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1954 ilock_flags &= ~BTRFS_ILOCK_SHARED;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1955 goto relock;
e9adabb9712ef94 Goldwyn Rodrigues 2020-09-24 1956 }
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1957
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1958 if (check_direct_IO(fs_info, from, pos))
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1959 goto buffered;
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1960
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1961 dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops,
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1962 &btrfs_dio_ops, is_sync_kiocb(iocb));
4e4cabece9f9c6b Goldwyn Rodrigues 2020-09-24 1963
d0215f3e5ebb580 Josef Bacik 2011-01-25 1964
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1965 if (IS_ERR_OR_NULL(dio)) {
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1966 err = PTR_ERR_OR_ZERO(dio);
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1967 if (err < 0 && err != -ENOTBLK)
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1968 goto out;
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1969 } else {
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1970 /*
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1971 * Remove the DSYNC flag because iomap_dio_complete() calls
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1972 * generic_write_sync() which may deadlock with btrfs_sync()
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1973 * on inode->i_rwsem
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1974 */
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1975 if (iocb->ki_flags & IOCB_DSYNC) {
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1976 dsync = true;
^^^^^^^^^^^^^
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1977 iocb->ki_flags &= ~IOCB_DSYNC;
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1978 }
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1979 written = iomap_dio_complete(dio);
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 @1980 if (dsync)
^^^^^
e7a8dd2d9537a7e Goldwyn Rodrigues 2020-12-18 1981 iocb->ki_flags |= IOCB_DSYNC;
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1982 }
a42fa643169d232 Goldwyn Rodrigues 2020-09-24 1983
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1984 if (written < 0 || !iov_iter_count(from)) {
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1985 err = written;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1986 goto out;
c352370633400d1 Goldwyn Rodrigues 2020-09-24 1987 }
d0215f3e5ebb580 Josef Bacik 2011-01-25 1988
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39286 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock
2020-12-18 16:07 Goldwyn Rodrigues
@ 2020-12-19 17:50 ` kernel test robot
2021-01-04 10:10 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-19 17:50 UTC (permalink / raw)
To: Goldwyn Rodrigues, linux-btrfs
Cc: kbuild-all, clang-built-linux, Goldwyn Rodrigues
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock
@ 2020-12-19 17:50 ` kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-12-19 17:50 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6267 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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39290 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock
@ 2020-12-18 16:07 Goldwyn Rodrigues
2020-12-19 17:50 ` kernel test robot
2021-01-04 10:10 ` Dan Carpenter
0 siblings, 2 replies; 5+ messages in thread
From: Goldwyn Rodrigues @ 2020-12-18 16:07 UTC (permalink / raw)
To: linux-btrfs; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
btrfs_direct_write() fallsback to buffered write in case btrfs is not
able to perform or complete a direct I/O. During the fallback
inode lock is unlocked and relocked. This does not guarantee the
atomicity of the entire write since the lock can be acquired by another
write between unlock and relock.
__btrfs_buffered_write() is used to perform the direct fallback write,
which performs the write without acquiring the lock or checks.
Before calling iomap_dio_complete(), clar the IOCB_DSYNC flag.
Re-instate after calling iomap_dio_complete()
fa54fc76db94 ("btrfs: push inode locking and unlocking into buffered/direct write")
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Changes since v2:
- Manipulate iocb->ki_flags before and after calling
iomap_dio_complete()
Changes since v1:
- Fix the issue of hang with the direct I/O and sync.
- lockdep_assert_held() in __btrfs_buffered_write()
---
fs/btrfs/file.c | 77 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 27 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index e65223e3510d..b7fe2f3d179a 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1638,11 +1638,11 @@ static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
return 0;
}
-static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
+static noinline ssize_t __btrfs_buffered_write(struct kiocb *iocb,
struct iov_iter *i)
{
struct file *file = iocb->ki_filp;
- loff_t pos;
+ loff_t pos = iocb->ki_pos;
struct inode *inode = file_inode(file);
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct page **pages = NULL;
@@ -1656,24 +1656,9 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
bool only_release_metadata = false;
bool force_page_uptodate = false;
loff_t old_isize = i_size_read(inode);
- unsigned int ilock_flags = 0;
- if (iocb->ki_flags & IOCB_NOWAIT)
- ilock_flags |= BTRFS_ILOCK_TRY;
+ lockdep_assert_held(&inode->i_rwsem);
- ret = btrfs_inode_lock(inode, ilock_flags);
- if (ret < 0)
- return ret;
-
- ret = generic_write_checks(iocb, i);
- if (ret <= 0)
- goto out;
-
- ret = btrfs_write_check(iocb, i, ret);
- if (ret < 0)
- goto out;
-
- pos = iocb->ki_pos;
nrptrs = min(DIV_ROUND_UP(iov_iter_count(i), PAGE_SIZE),
PAGE_SIZE / (sizeof(struct page *)));
nrptrs = min(nrptrs, current->nr_dirtied_pause - current->nr_dirtied);
@@ -1877,10 +1862,37 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
iocb->ki_pos += num_written;
}
out:
- btrfs_inode_unlock(inode, ilock_flags);
return num_written ? num_written : ret;
}
+static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
+ struct iov_iter *i)
+{
+ struct inode *inode = file_inode(iocb->ki_filp);
+ unsigned int ilock_flags = 0;
+ ssize_t ret;
+
+ if (iocb->ki_flags & IOCB_NOWAIT)
+ ilock_flags |= BTRFS_ILOCK_TRY;
+
+ ret = btrfs_inode_lock(inode, ilock_flags);
+ if (ret < 0)
+ return ret;
+
+ ret = generic_write_checks(iocb, i);
+ if (ret <= 0)
+ goto out;
+
+ ret = btrfs_write_check(iocb, i, ret);
+ if (ret < 0)
+ goto out;
+
+ ret = __btrfs_buffered_write(iocb, i);
+out:
+ btrfs_inode_unlock(inode, ilock_flags);
+ return ret;
+}
+
static ssize_t check_direct_IO(struct btrfs_fs_info *fs_info,
const struct iov_iter *iter, loff_t offset)
{
@@ -1907,6 +1919,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
ssize_t err;
unsigned int ilock_flags = 0;
struct iomap_dio *dio = NULL;
+ bool dsync;
if (iocb->ki_flags & IOCB_NOWAIT)
ilock_flags |= BTRFS_ILOCK_TRY;
@@ -1927,10 +1940,8 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
}
err = btrfs_write_check(iocb, from, err);
- if (err < 0) {
- btrfs_inode_unlock(inode, ilock_flags);
+ if (err < 0)
goto out;
- }
pos = iocb->ki_pos;
/*
@@ -1944,22 +1955,30 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
goto relock;
}
- if (check_direct_IO(fs_info, from, pos)) {
- btrfs_inode_unlock(inode, ilock_flags);
+ if (check_direct_IO(fs_info, from, pos))
goto buffered;
- }
dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops,
&btrfs_dio_ops, is_sync_kiocb(iocb));
- btrfs_inode_unlock(inode, ilock_flags);
if (IS_ERR_OR_NULL(dio)) {
err = PTR_ERR_OR_ZERO(dio);
if (err < 0 && err != -ENOTBLK)
goto out;
} else {
+ /*
+ * Remove the DSYNC flag because iomap_dio_complete() calls
+ * generic_write_sync() which may deadlock with btrfs_sync()
+ * on inode->i_rwsem
+ */
+ if (iocb->ki_flags & IOCB_DSYNC) {
+ dsync = true;
+ iocb->ki_flags &= ~IOCB_DSYNC;
+ }
written = iomap_dio_complete(dio);
+ if (dsync)
+ iocb->ki_flags |= IOCB_DSYNC;
}
if (written < 0 || !iov_iter_count(from)) {
@@ -1969,7 +1988,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
buffered:
pos = iocb->ki_pos;
- written_buffered = btrfs_buffered_write(iocb, from);
+ written_buffered = __btrfs_buffered_write(iocb, from);
if (written_buffered < 0) {
err = written_buffered;
goto out;
@@ -1990,6 +2009,10 @@ 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:
+ btrfs_inode_unlock(inode, ilock_flags);
+ if (written > 0 && dsync)
+ generic_write_sync(iocb, written);
+
return written ? written : err;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-04 10:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-29 16:03 [PATCH v3] btrfs: Make btrfs_direct_write atomic with respect to inode_lock kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2020-12-18 16:07 Goldwyn Rodrigues
2020-12-19 17:50 ` kernel test robot
2020-12-19 17:50 ` kernel test robot
2021-01-04 10:10 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.