On 2018/8/21 下午12:21, Misono Tomohiro wrote:
> On 2018/08/15 15:13, Qu Wenruo wrote:
>> Commit c6887cd11149 ("Btrfs: don't do nocow check unless we have to")
>> makes nocow check less frequent to improve performance.
>>
>> However for quota enabled case, such optimization could lead to extra
>> unnecessary data reservation, which results failure for test case like
>> btrfs/153 in fstests.
>>
>> Fix it by reverting to old behavior for quota enabled case.
>>
>> Fixes: c6887cd11149 ("Btrfs: don't do nocow check unless we have to")
>> Signed-off-by: Qu Wenruo <[email protected]>
>> ---
>> fs/btrfs/file.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
>> index 51e77d72068a..f2ce1d707d4c 100644
>> --- a/fs/btrfs/file.c
>> +++ b/fs/btrfs/file.c
>> @@ -1587,6 +1587,7 @@ static noinline ssize_t __btrfs_buffered_write(struct
>> file *file,
>> int ret = 0;
>> bool only_release_metadata = false;
>> bool force_page_uptodate = false;
>> + bool quota_enabled = test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
>>
>> nrptrs = min(DIV_ROUND_UP(iov_iter_count(i), PAGE_SIZE),
>> PAGE_SIZE / (sizeof(struct page *)));
>> @@ -1627,9 +1628,15 @@ static noinline ssize_t __btrfs_buffered_write(struct
>> file *file,
>> fs_info->sectorsize);
>>
>> extent_changeset_release(data_reserved);
>> - ret = btrfs_check_data_free_space(inode, &data_reserved, pos,
>> + /*
>> + * If we have quota enabled, we must do the heavy lift nocow
>> + * check here to avoid reserving data space, or we can hit
>> + * limitation for NOCOW files.
>> + */
>> + if (!quota_enabled)
>> + ret = btrfs_check_data_free_space(inode,
>> &data_reserved, pos,
>> write_bytes);
>> - if (ret < 0) {
>> + if (ret < 0 || quota_enabled) {
>> if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
>> BTRFS_INODE_PREALLOC))
>> &&> check_can_nocow(BTRFS_I(inode), pos,
>>
>
> This fixes btrfs/153 but in turn makes other qgroup tests fail (022,091 etc.)Thanks for the test. I also found the problem too. > > If quota is enabled and file is not marked as nocow, above if is false and > write will stop. > so I think we still need to call btrfs_check_dta_free_space() when quota is > enabled > and file is not nocow, right? Yes, we should btrfs_check_data_free_space() check. I only considered quota+nocow and noquota case. For quota+cow case it should goes the normal way. I'll check if there is any better solution to organize the code. Thanks, Qu
signature.asc
Description: OpenPGP digital signature
