Re: [PATCH 3/4] btrfs: autodefrag: only scan one inode once

[David Sterba <dsterba@suse.cz>]

On Thu, Feb 24, 2022 at 02:59:24PM +0800, Qu Wenruo wrote:
> Checked the code, it looks fine to me, just one small question related
> to the ret < 0 case.
> 
> Unlike the refactored version, which can return < 0 even if we defragged
> some sectors. (Since we have different members to record those info)
> 
> If we have defragged any sector in btrfs_defrag_file(), but some other
> problems happened later, we will get a return value > 0 in this version.
> 
> It's a not a big deal, as we will skip to the last scanned position
> anyway, and we even have the safenet to increase @cur even if
> range.start doesn't get increased.
> 
> For backport it's completely fine.
> 
> Just want to make sure for the proper version, what's is the expected
> behavior.
> Exit as soon as any error hit, or continue defrag as much as possible?

I'd say continue. Some errors may be transient, some may stick until the
defrag ioctl runs, but that should lead to quickly enumerating the
extents at worst.

> And I'll rebase my btrfs_defrag_ctrl patchset upon your fixes.

I may postpone any defrag cleanups until 5.19 so we don't have different
code bases for the branch that may still require fixes and the
development one. It's too close to code freeze and pull request time,
we need to properly sync the fixes and development branches, so they
don't diverge, like it was with the kmap/lzo reverts that led to the
fixups.