Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
From: Qu Wenruo <hidden>
Date: 2021-08-09 12:14:03
Also in:
oe-kbuild-all
On 2021/8/9 下午7:32, Dan Carpenter wrote:
Hi Qu, url: https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-defrag-rework-to-support-sector-perfect-defrag/20210806-161501 base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next config: h8300-randconfig-m031-20210804 (attached as .config) compiler: h8300-linux-gcc (GCC) 10.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <redacted> Reported-by: Dan Carpenter <redacted> New smatch warnings: fs/btrfs/ioctl.c:1869 btrfs_defrag_file() error: uninitialized symbol 'ret'.
OK, it's indeed a possible case, and I even find a special case where we don't need to go through the branch reported by the static checker.
vim +/ret +1869 fs/btrfs/ioctl.c
fe90d1614439a8 Qu Wenruo 2021-08-06 1757 int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
4cb5300bc839b8 Chris Mason 2011-05-24 1758 struct btrfs_ioctl_defrag_range_args *range,
4cb5300bc839b8 Chris Mason 2011-05-24 1759 u64 newer_than, unsigned long max_to_defrag)
4cb5300bc839b8 Chris Mason 2011-05-24 1760 {
0b246afa62b0cf Jeff Mahoney 2016-06-22 1761 struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1762 unsigned long sectors_defragged = 0;
151a31b25e5c94 Li Zefan 2011-09-02 1763 u64 isize = i_size_read(inode);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1764 u64 cur;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1765 u64 last_byte;
1e2ef46d89ee41 David Sterba 2017-07-17 1766 bool do_compress = range->flags & BTRFS_DEFRAG_RANGE_COMPRESS;
fe90d1614439a8 Qu Wenruo 2021-08-06 1767 bool ra_allocated = false;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1768 int compress_type = BTRFS_COMPRESS_ZLIB;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1769 int ret;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1770 u32 extent_thresh = range->extent_thresh;
4cb5300bc839b8 Chris Mason 2011-05-24 1771
0abd5b17249ea5 Liu Bo 2013-04-16 1772 if (isize == 0)
0abd5b17249ea5 Liu Bo 2013-04-16 1773 return 0;
0abd5b17249ea5 Liu Bo 2013-04-16 1774
0abd5b17249ea5 Liu Bo 2013-04-16 1775 if (range->start >= isize)
0abd5b17249ea5 Liu Bo 2013-04-16 1776 return -EINVAL;Firstly, we skip several invalid cases, like empty file or range beyond isize. Notice that now range->start < isize; AKA range->start <= isize - 1;
1a419d85a76853 Li Zefan 2010-10-25 1777
1e2ef46d89ee41 David Sterba 2017-07-17 1778 if (do_compress) {
ce96b7ffd11e26 Chengguang Xu 2019-10-10 1779 if (range->compress_type >= BTRFS_NR_COMPRESS_TYPES)
1a419d85a76853 Li Zefan 2010-10-25 1780 return -EINVAL;
1a419d85a76853 Li Zefan 2010-10-25 1781 if (range->compress_type)
1a419d85a76853 Li Zefan 2010-10-25 1782 compress_type = range->compress_type;
1a419d85a76853 Li Zefan 2010-10-25 1783 }
f46b5a66b3316e Christoph Hellwig 2008-06-11 1784
0abd5b17249ea5 Liu Bo 2013-04-16 1785 if (extent_thresh == 0)
ee22184b53c823 Byongho Lee 2015-12-15 1786 extent_thresh = SZ_256K;
940100a4a7b78b Chris Mason 2010-03-10 1787
d0b928ff1ed56a Qu Wenruo 2021-08-06 1788 if (range->start + range->len > range->start) {
d0b928ff1ed56a Qu Wenruo 2021-08-06 1789 /* Got a specific range */
d0b928ff1ed56a Qu Wenruo 2021-08-06 1790 last_byte = min(isize, range->start + range->len) - 1;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1791 } else {
d0b928ff1ed56a Qu Wenruo 2021-08-06 1792 /* Defrag until file end */
d0b928ff1ed56a Qu Wenruo 2021-08-06 1793 last_byte = isize - 1;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1794 }No matter the range->len, last_byte <= isize - 1; Also start->range <= isize - 1; Thus we can have a worst case where start->range == last_byte.
d0b928ff1ed56a Qu Wenruo 2021-08-06 1795
4cb5300bc839b8 Chris Mason 2011-05-24 1796 /*
fe90d1614439a8 Qu Wenruo 2021-08-06 1797 * If we were not given a ra, allocate a readahead context. As
0a52d108089f33 David Sterba 2017-06-22 1798 * readahead is just an optimization, defrag will work without it so
0a52d108089f33 David Sterba 2017-06-22 1799 * we don't error out.
4cb5300bc839b8 Chris Mason 2011-05-24 1800 */
fe90d1614439a8 Qu Wenruo 2021-08-06 1801 if (!ra) {
fe90d1614439a8 Qu Wenruo 2021-08-06 1802 ra_allocated = true;
63e727ecd238be David Sterba 2017-06-22 1803 ra = kzalloc(sizeof(*ra), GFP_KERNEL);
0a52d108089f33 David Sterba 2017-06-22 1804 if (ra)
4cb5300bc839b8 Chris Mason 2011-05-24 1805 file_ra_state_init(ra, inode->i_mapping);
4cb5300bc839b8 Chris Mason 2011-05-24 1806 }
4cb5300bc839b8 Chris Mason 2011-05-24 1807
d0b928ff1ed56a Qu Wenruo 2021-08-06 1808 /* Align the range */
d0b928ff1ed56a Qu Wenruo 2021-08-06 1809 cur = round_down(range->start, fs_info->sectorsize);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1810 last_byte = round_up(last_byte, fs_info->sectorsize) - 1;Even for the worst case, range->start == last_byte. If range->start = last_byte = 4K (aka isize = 4K + 1), then: cur = 4K; last_byte = 4K - 1; Now we don't need to go through the while() loop at all.
4cb5300bc839b8 Chris Mason 2011-05-24 1811
d0b928ff1ed56a Qu Wenruo 2021-08-06 1812 while (cur < last_byte) {
d0b928ff1ed56a Qu Wenruo 2021-08-06 1813 u64 cluster_end;
1e701a3292e25a Chris Mason 2010-03-11 1814
d0b928ff1ed56a Qu Wenruo 2021-08-06 1815 /* The cluster size 256K should always be page aligned */
d0b928ff1ed56a Qu Wenruo 2021-08-06 1816 BUILD_BUG_ON(!IS_ALIGNED(CLUSTER_SIZE, PAGE_SIZE));
008873eafbc77d Li Zefan 2011-09-02 1817
d0b928ff1ed56a Qu Wenruo 2021-08-06 1818 /* We want the cluster ends at page boundary when possible */
d0b928ff1ed56a Qu Wenruo 2021-08-06 1819 cluster_end = (((cur >> PAGE_SHIFT) +
d0b928ff1ed56a Qu Wenruo 2021-08-06 1820 (SZ_256K >> PAGE_SHIFT)) << PAGE_SHIFT) - 1;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1821 cluster_end = min(cluster_end, last_byte);
940100a4a7b78b Chris Mason 2010-03-10 1822
64708539cd23b3 Josef Bacik 2021-02-10 1823 btrfs_inode_lock(inode, 0);
eede2bf34f4fa8 Omar Sandoval 2016-11-03 1824 if (IS_SWAPFILE(inode)) {
eede2bf34f4fa8 Omar Sandoval 2016-11-03 1825 ret = -ETXTBSY;
64708539cd23b3 Josef Bacik 2021-02-10 1826 btrfs_inode_unlock(inode, 0);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1827 break;
ecb8bea87d05fd Liu Bo 2012-03-29 1828 }
d0b928ff1ed56a Qu Wenruo 2021-08-06 1829 if (!(inode->i_sb->s_flags & SB_ACTIVE)) {
64708539cd23b3 Josef Bacik 2021-02-10 1830 btrfs_inode_unlock(inode, 0);
4cb5300bc839b8 Chris Mason 2011-05-24 1831 break;
Can we hit this break statement on the first iteration through the loop?
3eaa2885276fd6 Chris Mason 2008-07-24 1832 }
d0b928ff1ed56a Qu Wenruo 2021-08-06 1833 if (do_compress)
d0b928ff1ed56a Qu Wenruo 2021-08-06 1834 BTRFS_I(inode)->defrag_compress = compress_type;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1835 ret = defrag_one_cluster(BTRFS_I(inode), ra, cur,
d0b928ff1ed56a Qu Wenruo 2021-08-06 1836 cluster_end + 1 - cur, extent_thresh,
d0b928ff1ed56a Qu Wenruo 2021-08-06 1837 newer_than, do_compress,
d0b928ff1ed56a Qu Wenruo 2021-08-06 1838 §ors_defragged, max_to_defrag);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1839 btrfs_inode_unlock(inode, 0);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1840 if (ret < 0)
d0b928ff1ed56a Qu Wenruo 2021-08-06 1841 break;
d0b928ff1ed56a Qu Wenruo 2021-08-06 1842 cur = cluster_end + 1;
4cb5300bc839b8 Chris Mason 2011-05-24 1843 }
f46b5a66b3316e Christoph Hellwig 2008-06-11 1844
d0b928ff1ed56a Qu Wenruo 2021-08-06 1845 if (ra_allocated)
d0b928ff1ed56a Qu Wenruo 2021-08-06 1846 kfree(ra);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1847 if (sectors_defragged) {
d0b928ff1ed56a Qu Wenruo 2021-08-06 1848 /*
d0b928ff1ed56a Qu Wenruo 2021-08-06 1849 * We have defragged some sectors, for compression case
d0b928ff1ed56a Qu Wenruo 2021-08-06 1850 * they need to be written back immediately.
d0b928ff1ed56a Qu Wenruo 2021-08-06 1851 */
d0b928ff1ed56a Qu Wenruo 2021-08-06 1852 if (range->flags & BTRFS_DEFRAG_RANGE_START_IO) {
1e701a3292e25a Chris Mason 2010-03-11 1853 filemap_flush(inode->i_mapping);
dec8ef90552f7b Filipe Manana 2014-03-01 1854 if (test_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
dec8ef90552f7b Filipe Manana 2014-03-01 1855 &BTRFS_I(inode)->runtime_flags))
1e701a3292e25a Chris Mason 2010-03-11 1856 filemap_flush(inode->i_mapping);
dec8ef90552f7b Filipe Manana 2014-03-01 1857 }
d0b928ff1ed56a Qu Wenruo 2021-08-06 1858 if (range->compress_type == BTRFS_COMPRESS_LZO)
0b246afa62b0cf Jeff Mahoney 2016-06-22 1859 btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1860 else if (range->compress_type == BTRFS_COMPRESS_ZSTD)
5c1aab1dd5445e Nick Terrell 2017-08-09 1861 btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
d0b928ff1ed56a Qu Wenruo 2021-08-06 1862 ret = sectors_defragged;
1a419d85a76853 Li Zefan 2010-10-25 1863 }We also skip above (sectors_defraged) branch.
1e2ef46d89ee41 David Sterba 2017-07-17 1864 if (do_compress) {
64708539cd23b3 Josef Bacik 2021-02-10 1865 btrfs_inode_lock(inode, 0);
eec63c65dcbeb1 David Sterba 2017-07-17 1866 BTRFS_I(inode)->defrag_compress = BTRFS_COMPRESS_NONE;
64708539cd23b3 Josef Bacik 2021-02-10 1867 btrfs_inode_unlock(inode, 0);
633085c79c84c3 Filipe David Borba Manana 2013-08-16 1868 }
940100a4a7b78b Chris Mason 2010-03-10 @1869 return ret;And @ret is indeed uninitialized. Will fix it in next update. Thanks, Qu
f46b5a66b3316e Christoph Hellwig 2008-06-11 1870 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org