* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
@ 2021-08-07 10:05 kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-08-07 10:05 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 24052 bytes --]
CC: clang-built-linux(a)googlegroups.com
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210806081242.257996-10-wqu@suse.com>
References: <20210806081242.257996-10-wqu@suse.com>
TO: Qu Wenruo <wqu@suse.com>
TO: linux-btrfs(a)vger.kernel.org
Hi Qu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.14-rc4 next-20210806]
[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/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
:::::: branch date: 26 hours ago
:::::: commit date: 26 hours ago
config: x86_64-randconfig-c001-20210805 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 42b9c2a17a0b63cccf3ac197a82f91b28e53e643)
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/d0b928ff1ed56a1ab892be6614fe646d4162c6d3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Qu-Wenruo/btrfs-defrag-rework-to-support-sector-perfect-defrag/20210806-161501
git checkout d0b928ff1ed56a1ab892be6614fe646d4162c6d3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:350:7: note: Assuming 'sdma0' is equal to field 'buffer_funcs_ring'
if ((adev->mman.buffer_funcs_ring == sdma0) ||
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:350:46: note: Left side of '||' is true
if ((adev->mman.buffer_funcs_ring == sdma0) ||
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:14: note: Assuming 'i' is < field 'num_instances'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:2: note: Loop condition is true. Entering loop body
for (i = 0; i < adev->sdma.num_instances; i++) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:14: note: Assuming 'i' is < field 'num_instances'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:2: note: Loop condition is true. Entering loop body
for (i = 0; i < adev->sdma.num_instances; i++) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:44: note: The value 2 is assigned to 'i'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:14: note: Assuming 'i' is < field 'num_instances'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:354:2: note: Loop condition is true. Entering loop body
for (i = 0; i < adev->sdma.num_instances; i++) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:355:40: note: The right operand of '+' is a garbage value due to array index out of bounds
rb_cntl = RREG32(mmSDMA0_GFX_RB_CNTL + sdma_offsets[i]);
^
drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:1162:47: note: expanded from macro 'RREG32'
#define RREG32(reg) amdgpu_device_rreg(adev, (reg), 0)
^~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:395:38: warning: The right operand of '+' is a garbage value due to array index out of bounds [clang-analyzer-core.UndefinedBinaryOperatorResult]
f32_cntl = RREG32(mmSDMA0_F32_CNTL + sdma_offsets[i]);
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:936:9: note: Calling 'sdma_v2_4_hw_init'
return sdma_v2_4_hw_init(adev);
^~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:909:6: note: Calling 'sdma_v2_4_start'
r = sdma_v2_4_start(adev);
^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:527:2: note: Calling 'sdma_v2_4_enable'
sdma_v2_4_enable(adev, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:389:7: note: 'enable' is false
if (!enable) {
^~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:389:2: note: Taking true branch
if (!enable) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:14: note: Assuming 'i' is < field 'num_instances'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:2: note: Loop condition is true. Entering loop body
for (i = 0; i < adev->sdma.num_instances; i++) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:396:7: note: 'enable' is false
if (enable)
^~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:396:3: note: Taking false branch
if (enable)
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:14: note: Assuming 'i' is < field 'num_instances'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:2: note: Loop condition is true. Entering loop body
for (i = 0; i < adev->sdma.num_instances; i++) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:396:7: note: 'enable' is false
if (enable)
^~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:396:3: note: Taking false branch
if (enable)
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:44: note: The value 2 is assigned to 'i'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:14: note: Assuming 'i' is < field 'num_instances'
for (i = 0; i < adev->sdma.num_instances; i++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:394:2: note: Loop condition is true. Entering loop body
for (i = 0; i < adev->sdma.num_instances; i++) {
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:395:38: note: The right operand of '+' is a garbage value due to array index out of bounds
f32_cntl = RREG32(mmSDMA0_F32_CNTL + sdma_offsets[i]);
^
drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:1162:47: note: expanded from macro 'RREG32'
#define RREG32(reg) amdgpu_device_rreg(adev, (reg), 0)
^~~
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:1000:3: warning: Value stored to 'tmp' is never read [clang-analyzer-deadcode.DeadStores]
tmp = RREG32(mmSRBM_SOFT_RESET);
^
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c:1000:3: note: Value stored to 'tmp' is never read
Suppressed 11 warnings (11 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
6 warnings generated.
Suppressed 6 warnings (6 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13 warnings generated.
>> fs/btrfs/ioctl.c:1869:2: warning: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn]
return ret;
^
fs/btrfs/ioctl.c:5133:2: note: Control jumps to 'case 1342215170:' at line 5160
switch (cmd) {
^
fs/btrfs/ioctl.c:5161:10: note: Calling 'btrfs_ioctl_defrag'
return btrfs_ioctl_defrag(file, NULL);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:3365:6: note: Assuming 'ret' is 0
if (ret)
^~~
fs/btrfs/ioctl.c:3365:2: note: Taking false branch
if (ret)
^
fs/btrfs/ioctl.c:3368:2: note: Taking false branch
if (btrfs_root_readonly(root)) {
^
fs/btrfs/ioctl.c:3374:6: note: Assuming the condition is false
if (root->fs_info->sectorsize < PAGE_SIZE) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:3374:2: note: Taking false branch
if (root->fs_info->sectorsize < PAGE_SIZE) {
^
fs/btrfs/ioctl.c:3379:2: note: Control jumps to 'case 32768:' @line 3387
switch (inode->i_mode & S_IFMT) {
^
fs/btrfs/ioctl.c:3393:7: note: Assuming the condition is false
if (!capable(CAP_SYS_ADMIN) &&
^~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:3393:31: note: Left side of '&&' is false
if (!capable(CAP_SYS_ADMIN) &&
^
fs/btrfs/ioctl.c:3400:7: note: Assuming 'range' is non-null
if (!range) {
^~~~~~
fs/btrfs/ioctl.c:3400:3: note: Taking false branch
if (!range) {
^
fs/btrfs/ioctl.c:3405:7: note: 'argp' is null
if (argp) {
^~~~
fs/btrfs/ioctl.c:3405:3: note: Taking false branch
if (argp) {
^
fs/btrfs/ioctl.c:3421:9: note: Calling 'btrfs_defrag_file'
ret = btrfs_defrag_file(file_inode(file), &file->f_ra,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:1769:2: note: 'ret' declared without an initial value
int ret;
^~~~~~~
fs/btrfs/ioctl.c:1772:6: note: Assuming 'isize' is not equal to 0
if (isize == 0)
^~~~~~~~~~
fs/btrfs/ioctl.c:1772:2: note: Taking false branch
if (isize == 0)
^
fs/btrfs/ioctl.c:1775:6: note: Assuming 'isize' is > field 'start'
if (range->start >= isize)
^~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:1775:2: note: Taking false branch
if (range->start >= isize)
^
fs/btrfs/ioctl.c:1778:6: note: Assuming 'do_compress' is false
if (do_compress) {
^~~~~~~~~~~
fs/btrfs/ioctl.c:1778:2: note: Taking false branch
if (do_compress) {
^
fs/btrfs/ioctl.c:1785:6: note: Assuming 'extent_thresh' is not equal to 0
if (extent_thresh == 0)
^~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:1785:2: note: Taking false branch
if (extent_thresh == 0)
^
fs/btrfs/ioctl.c:1788:6: note: Assuming the condition is false
if (range->start + range->len > range->start) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:1788:2: note: Taking false branch
if (range->start + range->len > range->start) {
^
fs/btrfs/ioctl.c:1801:7: note: 'ra' is non-null
if (!ra) {
^~
fs/btrfs/ioctl.c:1801:2: note: Taking false branch
if (!ra) {
^
fs/btrfs/ioctl.c:1812:9: note: Assuming 'cur' is >= 'last_byte'
while (cur < last_byte) {
^~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:1812:2: note: Loop condition is false. Execution continues on line 1845
while (cur < last_byte) {
^
fs/btrfs/ioctl.c:1845:6: note: 'ra_allocated' is false
if (ra_allocated)
^~~~~~~~~~~~
fs/btrfs/ioctl.c:1845:2: note: Taking false branch
if (ra_allocated)
^
fs/btrfs/ioctl.c:1847:6: note: 'sectors_defragged' is 0
if (sectors_defragged) {
vim +1869 fs/btrfs/ioctl.c
bd3c39b7ee16de Qu Wenruo 2021-08-06 1746
fe90d1614439a8 Qu Wenruo 2021-08-06 1747 /*
fe90d1614439a8 Qu Wenruo 2021-08-06 1748 * Btrfs entrace for defrag.
fe90d1614439a8 Qu Wenruo 2021-08-06 1749 *
fe90d1614439a8 Qu Wenruo 2021-08-06 1750 * @inode: Inode to be defragged
fe90d1614439a8 Qu Wenruo 2021-08-06 1751 * @ra: Readahead state. If NULL, one will be allocated@runtime.
fe90d1614439a8 Qu Wenruo 2021-08-06 1752 * @range: Defrag options including range and flags.
fe90d1614439a8 Qu Wenruo 2021-08-06 1753 * @newer_than: Minimal transid to defrag
fe90d1614439a8 Qu Wenruo 2021-08-06 1754 * @max_to_defrag: Max number of sectors to be defragged, if 0, the whole inode
fe90d1614439a8 Qu Wenruo 2021-08-06 1755 * will be defragged.
fe90d1614439a8 Qu Wenruo 2021-08-06 1756 */
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;
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 }
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;
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@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;
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 }
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;
f46b5a66b3316e Christoph Hellwig 2008-06-11 1870 }
f46b5a66b3316e Christoph Hellwig 2008-06-11 1871
---
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: 33196 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
2021-08-09 12:13 ` Qu Wenruo
@ 2021-08-10 6:19 ` Qu Wenruo
-1 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2021-08-10 6:19 UTC (permalink / raw)
To: Dan Carpenter, kbuild, Qu Wenruo, linux-btrfs; +Cc: lkp, kbuild-all
On 2021/8/9 下午8:13, Qu Wenruo wrote:
>
>
> 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 <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> 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.
BTW, here the proper way to calculate last_byte should be:
round_up(last_byte + 1, sectorsize) - 1;
So that we are ensured that rounded up last_byte will never be smaller
than range->start.
I have fixed both uninitialized @ret and this @last_byte problem in my
github repo already.
Thanks,
Qu
>
>> 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
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
@ 2021-08-10 6:19 ` Qu Wenruo
0 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2021-08-10 6:19 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 15842 bytes --]
On 2021/8/9 下午8:13, Qu Wenruo wrote:
>
>
> 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 <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> 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.
BTW, here the proper way to calculate last_byte should be:
round_up(last_byte + 1, sectorsize) - 1;
So that we are ensured that rounded up last_byte will never be smaller
than range->start.
I have fixed both uninitialized @ret and this @last_byte problem in my
github repo already.
Thanks,
Qu
>
>> 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(a)lists.01.org
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
2021-08-09 11:32 ` Dan Carpenter
@ 2021-08-09 12:13 ` Qu Wenruo
-1 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2021-08-09 12:13 UTC (permalink / raw)
To: Dan Carpenter, kbuild, Qu Wenruo, linux-btrfs; +Cc: lkp, 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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> 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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
@ 2021-08-09 12:13 ` Qu Wenruo
0 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2021-08-09 12:13 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11913 bytes --]
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 <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> 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(a)lists.01.org
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
2021-08-06 8:12 ` [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file() Qu Wenruo
2021-08-09 11:32 ` Dan Carpenter
@ 2021-08-09 11:32 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2021-08-09 11:32 UTC (permalink / raw)
To: kbuild, Qu Wenruo, linux-btrfs; +Cc: lkp, kbuild-all
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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
fs/btrfs/ioctl.c:1869 btrfs_defrag_file() error: uninitialized symbol 'ret'.
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;
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 }
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;
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 }
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;
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
@ 2021-08-09 11:32 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2021-08-09 11:32 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10810 bytes --]
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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
fs/btrfs/ioctl.c:1869 btrfs_defrag_file() error: uninitialized symbol 'ret'.
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;
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 }
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;
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@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 }
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;
f46b5a66b3316e Christoph Hellwig 2008-06-11 1870 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
@ 2021-08-09 11:32 ` Dan Carpenter
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-08-06 18:01 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 12824 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210806081242.257996-10-wqu@suse.com>
References: <20210806081242.257996-10-wqu@suse.com>
TO: Qu Wenruo <wqu@suse.com>
TO: linux-btrfs(a)vger.kernel.org
Hi Qu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.14-rc4 next-20210805]
[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/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
:::::: branch date: 10 hours ago
:::::: commit date: 10 hours ago
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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
fs/btrfs/ioctl.c:1869 btrfs_defrag_file() error: uninitialized symbol 'ret'.
Old smatch warnings:
fs/btrfs/ioctl.c:816 create_snapshot() warn: '&pending_snapshot->list' not removed from list
fs/btrfs/ioctl.c:1216 defrag_prepare_one_page() warn: should 'index << 12' be a 64 bit type?
fs/btrfs/ioctl.c:1649 defrag_one_range() warn: should 'start_index << 12' be a 64 bit type?
fs/btrfs/ioctl.c:1677 defrag_one_range() warn: should 'start_index << 12' be a 64 bit type?
vim +/ret +1869 fs/btrfs/ioctl.c
bd3c39b7ee16de Qu Wenruo 2021-08-06 1746
fe90d1614439a8 Qu Wenruo 2021-08-06 1747 /*
fe90d1614439a8 Qu Wenruo 2021-08-06 1748 * Btrfs entrace for defrag.
fe90d1614439a8 Qu Wenruo 2021-08-06 1749 *
fe90d1614439a8 Qu Wenruo 2021-08-06 1750 * @inode: Inode to be defragged
fe90d1614439a8 Qu Wenruo 2021-08-06 1751 * @ra: Readahead state. If NULL, one will be allocated@runtime.
fe90d1614439a8 Qu Wenruo 2021-08-06 1752 * @range: Defrag options including range and flags.
fe90d1614439a8 Qu Wenruo 2021-08-06 1753 * @newer_than: Minimal transid to defrag
fe90d1614439a8 Qu Wenruo 2021-08-06 1754 * @max_to_defrag: Max number of sectors to be defragged, if 0, the whole inode
fe90d1614439a8 Qu Wenruo 2021-08-06 1755 * will be defragged.
fe90d1614439a8 Qu Wenruo 2021-08-06 1756 */
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;
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 }
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;
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@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;
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 }
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;
f46b5a66b3316e Christoph Hellwig 2008-06-11 1870 }
f46b5a66b3316e Christoph Hellwig 2008-06-11 1871
---
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: 32948 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file()
2021-08-06 8:12 [PATCH v5 00/11] btrfs: defrag: rework to support sector perfect defrag Qu Wenruo
@ 2021-08-06 8:12 ` Qu Wenruo
2021-08-09 11:32 ` Dan Carpenter
0 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2021-08-06 8:12 UTC (permalink / raw)
To: linux-btrfs
The function defrag_one_cluster() is able to defrag one range well
enough, we only need to do prepration for it, including:
- Clamp and align the defrag range
- Exclude invalid cases
- Proper inode locking
The old infrastructures will not be removed in this patch, as it would
be too noisy to review.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/ioctl.c | 204 +++++++++++++----------------------------------
1 file changed, 55 insertions(+), 149 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 74346fde06f6..b3ba89d6402e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1759,25 +1759,15 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
u64 newer_than, unsigned long max_to_defrag)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
- struct btrfs_root *root = BTRFS_I(inode)->root;
- unsigned long last_index;
+ unsigned long sectors_defragged = 0;
u64 isize = i_size_read(inode);
- u64 last_len = 0;
- u64 skip = 0;
- u64 defrag_end = 0;
- u64 newer_off = range->start;
- unsigned long i;
- unsigned long ra_index = 0;
- int ret;
- int defrag_count = 0;
- int compress_type = BTRFS_COMPRESS_ZLIB;
- u32 extent_thresh = range->extent_thresh;
- unsigned long max_cluster = SZ_256K >> PAGE_SHIFT;
- unsigned long cluster = max_cluster;
- u64 new_align = ~((u64)SZ_128K - 1);
- struct page **pages = NULL;
+ u64 cur;
+ u64 last_byte;
bool do_compress = range->flags & BTRFS_DEFRAG_RANGE_COMPRESS;
bool ra_allocated = false;
+ int compress_type = BTRFS_COMPRESS_ZLIB;
+ int ret;
+ u32 extent_thresh = range->extent_thresh;
if (isize == 0)
return 0;
@@ -1795,6 +1785,14 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
if (extent_thresh == 0)
extent_thresh = SZ_256K;
+ if (range->start + range->len > range->start) {
+ /* Got a specific range */
+ last_byte = min(isize, range->start + range->len) - 1;
+ } else {
+ /* Defrag until file end */
+ last_byte = isize - 1;
+ }
+
/*
* If we were not given a ra, allocate a readahead context. As
* readahead is just an optimization, defrag will work without it so
@@ -1807,159 +1805,67 @@ int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
file_ra_state_init(ra, inode->i_mapping);
}
- pages = kmalloc_array(max_cluster, sizeof(struct page *), GFP_KERNEL);
- if (!pages) {
- ret = -ENOMEM;
- goto out_ra;
- }
+ /* Align the range */
+ cur = round_down(range->start, fs_info->sectorsize);
+ last_byte = round_up(last_byte, fs_info->sectorsize) - 1;
- /* find the last page to defrag */
- if (range->start + range->len > range->start) {
- last_index = min_t(u64, isize - 1,
- range->start + range->len - 1) >> PAGE_SHIFT;
- } else {
- last_index = (isize - 1) >> PAGE_SHIFT;
- }
-
- if (newer_than) {
- ret = find_new_extents(root, inode, newer_than,
- &newer_off, SZ_64K);
- if (!ret) {
- range->start = newer_off;
- /*
- * we always align our defrag to help keep
- * the extents in the file evenly spaced
- */
- i = (newer_off & new_align) >> PAGE_SHIFT;
- } else
- goto out_ra;
- } else {
- i = range->start >> PAGE_SHIFT;
- }
- if (!max_to_defrag)
- max_to_defrag = last_index - i + 1;
-
- /*
- * make writeback starts from i, so the defrag range can be
- * written sequentially.
- */
- if (i < inode->i_mapping->writeback_index)
- inode->i_mapping->writeback_index = i;
-
- while (i <= last_index && defrag_count < max_to_defrag &&
- (i < DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE))) {
- /*
- * make sure we stop running if someone unmounts
- * the FS
- */
- if (!(inode->i_sb->s_flags & SB_ACTIVE))
- break;
-
- if (btrfs_defrag_cancelled(fs_info)) {
- btrfs_debug(fs_info, "defrag_file cancelled");
- ret = -EAGAIN;
- goto error;
- }
-
- if (!should_defrag_range(inode, (u64)i << PAGE_SHIFT,
- extent_thresh, &last_len, &skip,
- &defrag_end, do_compress)){
- unsigned long next;
- /*
- * the should_defrag function tells us how much to skip
- * bump our counter by the suggested amount
- */
- next = DIV_ROUND_UP(skip, PAGE_SIZE);
- i = max(i + 1, next);
- continue;
- }
+ while (cur < last_byte) {
+ u64 cluster_end;
- if (!newer_than) {
- cluster = (PAGE_ALIGN(defrag_end) >>
- PAGE_SHIFT) - i;
- cluster = min(cluster, max_cluster);
- } else {
- cluster = max_cluster;
- }
+ /* The cluster size 256K should always be page aligned */
+ BUILD_BUG_ON(!IS_ALIGNED(CLUSTER_SIZE, PAGE_SIZE));
- if (i + cluster > ra_index) {
- ra_index = max(i, ra_index);
- if (ra)
- page_cache_sync_readahead(inode->i_mapping, ra,
- NULL, ra_index, cluster);
- ra_index += cluster;
- }
+ /* We want the cluster ends at page boundary when possible */
+ cluster_end = (((cur >> PAGE_SHIFT) +
+ (SZ_256K >> PAGE_SHIFT)) << PAGE_SHIFT) - 1;
+ cluster_end = min(cluster_end, last_byte);
btrfs_inode_lock(inode, 0);
if (IS_SWAPFILE(inode)) {
ret = -ETXTBSY;
- } else {
- if (do_compress)
- BTRFS_I(inode)->defrag_compress = compress_type;
- ret = cluster_pages_for_defrag(inode, pages, i, cluster);
+ btrfs_inode_unlock(inode, 0);
+ break;
}
- if (ret < 0) {
+ if (!(inode->i_sb->s_flags & SB_ACTIVE)) {
btrfs_inode_unlock(inode, 0);
- goto out_ra;
+ break;
}
-
- defrag_count += ret;
- balance_dirty_pages_ratelimited(inode->i_mapping);
+ if (do_compress)
+ BTRFS_I(inode)->defrag_compress = compress_type;
+ ret = defrag_one_cluster(BTRFS_I(inode), ra, cur,
+ cluster_end + 1 - cur, extent_thresh,
+ newer_than, do_compress,
+ §ors_defragged, max_to_defrag);
btrfs_inode_unlock(inode, 0);
-
- if (newer_than) {
- if (newer_off == (u64)-1)
- break;
-
- if (ret > 0)
- i += ret;
-
- newer_off = max(newer_off + 1,
- (u64)i << PAGE_SHIFT);
-
- ret = find_new_extents(root, inode, newer_than,
- &newer_off, SZ_64K);
- if (!ret) {
- range->start = newer_off;
- i = (newer_off & new_align) >> PAGE_SHIFT;
- } else {
- break;
- }
- } else {
- if (ret > 0) {
- i += ret;
- last_len += ret << PAGE_SHIFT;
- } else {
- i++;
- last_len = 0;
- }
- }
+ if (ret < 0)
+ break;
+ cur = cluster_end + 1;
}
- ret = defrag_count;
-error:
- if ((range->flags & BTRFS_DEFRAG_RANGE_START_IO)) {
- filemap_flush(inode->i_mapping);
- if (test_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
- &BTRFS_I(inode)->runtime_flags))
+ if (ra_allocated)
+ kfree(ra);
+ if (sectors_defragged) {
+ /*
+ * We have defragged some sectors, for compression case
+ * they need to be written back immediately.
+ */
+ if (range->flags & BTRFS_DEFRAG_RANGE_START_IO) {
filemap_flush(inode->i_mapping);
+ if (test_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
+ &BTRFS_I(inode)->runtime_flags))
+ filemap_flush(inode->i_mapping);
+ }
+ if (range->compress_type == BTRFS_COMPRESS_LZO)
+ btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
+ else if (range->compress_type == BTRFS_COMPRESS_ZSTD)
+ btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
+ ret = sectors_defragged;
}
-
- if (range->compress_type == BTRFS_COMPRESS_LZO) {
- btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
- } else if (range->compress_type == BTRFS_COMPRESS_ZSTD) {
- btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
- }
-
-out_ra:
if (do_compress) {
btrfs_inode_lock(inode, 0);
BTRFS_I(inode)->defrag_compress = BTRFS_COMPRESS_NONE;
btrfs_inode_unlock(inode, 0);
}
- if (ra_allocated)
- kfree(ra);
- kfree(pages);
return ret;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-08-10 6:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-07 10:05 [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file() kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-08-06 8:12 [PATCH v5 00/11] btrfs: defrag: rework to support sector perfect defrag Qu Wenruo
2021-08-06 8:12 ` [PATCH v5 09/11] btrfs: defrag: use defrag_one_cluster() to implement btrfs_defrag_file() Qu Wenruo
2021-08-09 11:32 ` Dan Carpenter
2021-08-06 18:01 ` kernel test robot
2021-08-09 11:32 ` Dan Carpenter
2021-08-09 12:13 ` Qu Wenruo
2021-08-09 12:13 ` Qu Wenruo
2021-08-10 6:19 ` Qu Wenruo
2021-08-10 6:19 ` Qu Wenruo
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.