Re: [PATCH] btrfs-progs: balance: Sync the fs before balancing metadata chunks

[Qu Wenruo <wqu@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1321 bytes --]



On 2019/2/26 上午1:21, David Sterba wrote:
> On Tue, Jan 29, 2019 at 02:57:39PM +0800, Qu Wenruo wrote:
>> The most straightforward is to sync the fs before balancing metadata
>> chunks.
>>
>> We could enhance the kernel bytes_may_use calculation, but I doubt about
>> the complexity.
>> So I take the easy fix to reduce the false ENOSPC reports.
> 
> Agreed.
> 
>> +	/*
>> +	 * There may be many over-reserved space for metadata block groups,
>> +	 * especially for inlined file extents.
>> +	 *
>> +	 * Do a sync here will free those over-reserved space and hugely
>> +	 * reduce the possibility of some false ENOSPC
>> +	 */
>> +	if (args->flags & BTRFS_BALANCE_METADATA) {
>> +		ret = btrfs_util_sync(path);
> 
> As the fd is already open, we should use the _fd version,
> 
>> +		if (ret) {
>> +			error("failed to sync the fs before balance: %m");
>> +			ret = -errno;
>> +			goto out;
> 
> and possibly only warn if there's an error returned as the sync failure
> is not a critical condition.

AFAIK if we can't even sync the fs, the balance is definitely going to
fail, as the most common failure mode for syncfs is RO fs, caused by
aborted transaction.

Thus I still think we should error out. Or is there some other
non-critical failure mode I missed?

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]