Skip to content

Commit 484167d

Browse files
adam900710kdave
authored andcommitted
btrfs: defrag: fix wrong number of defragged sectors
[BUG] There are users using autodefrag mount option reporting obvious increase in IO: > If I compare the write average (in total, I don't have it per process) > when taking idle periods on the same machine: > Linux 5.16: > without autodefrag: ~ 10KiB/s > with autodefrag: between 1 and 2MiB/s. > > Linux 5.15: > with autodefrag:~ 10KiB/s (around the same as without > autodefrag on 5.16) [CAUSE] When autodefrag mount option is enabled, btrfs_defrag_file() will be called with @max_sectors = BTRFS_DEFRAG_BATCH (1024) to limit how many sectors we can defrag in one try. And then use the number of sectors defragged to determine if we need to re-defrag. But commit b18c3ab ("btrfs: defrag: introduce helper to defrag one cluster") uses wrong unit to increase @sectors_defragged, which should be in unit of sector, not byte. This means, if we have defragged any sector, then @sectors_defragged will be >= sectorsize (normally 4096), which is larger than BTRFS_DEFRAG_BATCH. This makes the @max_sectors check in defrag_one_cluster() to underflow, rendering the whole @max_sectors check useless. Thus causing way more IO for autodefrag mount options, as now there is no limit on how many sectors can really be defragged. [FIX] Fix the problems by: - Use sector as unit when increasing @sectors_defragged - Include @sectors_defragged > @max_sectors case to break the loop - Add extra comment on the return value of btrfs_defrag_file() Reported-by: Anthony Ruhier <aruhier@mailbox.org> Fixes: b18c3ab ("btrfs: defrag: introduce helper to defrag one cluster") Link: https://lore.kernel.org/linux-btrfs/0a269612-e43f-da22-c5bc-b34b1b56ebe8@mailbox.org/ CC: stable@vger.kernel.org # 5.16 Reviewed-by: Filipe Manana <fdmanana@suse.com> Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent b767c2f commit 484167d

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

fs/btrfs/ioctl.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,8 +1442,8 @@ static int defrag_one_cluster(struct btrfs_inode *inode,
14421442
list_for_each_entry(entry, &target_list, list) {
14431443
u32 range_len = entry->len;
14441444

1445-
/* Reached the limit */
1446-
if (max_sectors && max_sectors == *sectors_defragged)
1445+
/* Reached or beyond the limit */
1446+
if (max_sectors && *sectors_defragged >= max_sectors)
14471447
break;
14481448

14491449
if (max_sectors)
@@ -1465,7 +1465,8 @@ static int defrag_one_cluster(struct btrfs_inode *inode,
14651465
extent_thresh, newer_than, do_compress);
14661466
if (ret < 0)
14671467
break;
1468-
*sectors_defragged += range_len;
1468+
*sectors_defragged += range_len >>
1469+
inode->root->fs_info->sectorsize_bits;
14691470
}
14701471
out:
14711472
list_for_each_entry_safe(entry, tmp, &target_list, list) {
@@ -1484,6 +1485,9 @@ static int defrag_one_cluster(struct btrfs_inode *inode,
14841485
* @newer_than: minimum transid to defrag
14851486
* @max_to_defrag: max number of sectors to be defragged, if 0, the whole inode
14861487
* will be defragged.
1488+
*
1489+
* Return <0 for error.
1490+
* Return >=0 for the number of sectors defragged.
14871491
*/
14881492
int btrfs_defrag_file(struct inode *inode, struct file_ra_state *ra,
14891493
struct btrfs_ioctl_defrag_range_args *range,

0 commit comments

Comments
 (0)