On 1.11.2017 11:46, Roman Mamedov wrote:
> On Wed, 1 Nov 2017 11:32:18 +0200
> Nikolay Borisov <nborisov@xxxxxxxx> wrote:
>
>> Fallocating a file in btrfs goes through several stages. The one before actually
>> inserting the fallocated extents is to create a qgroup reservation, covering
>> the desired range. To this end there is a loop in btrfs_fallocate which checks
>> to see if there are holes in the fallocated range or !PREALLOC extents past EOF
>> and if so create qgroup reservations for them. Unfortunately, the main condition
>> of the loop is burried right at the end of its body rather than in the actual
>> while statement which makes it non-obvious. Fix this by moving the condition
>> in the while statement where it belongs. No functional changes.
>
> If it turns out that "cur_offset >= alloc_end" from the get go, previously the
> loop body would be entered and executed once. With this change, it will not
> anymore.
Good point, however this cannot happen, because for this to happen then
the following 2 expression need to be equal:
alloc_start = round_down(offset, blocksize);
alloc_end = round_up(offset + len, blocksize);
However, len is guaranteed to be > 0 due to a check in vfs_fallocate so
those can't really be equal.
>
> I did not examine the context to see if such case is possible, likely,
> beneficial or harmful. But if you wanted 100% no functional changes no matter> what, maybe better use a "do ... while" loop?
>
>> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
>> ---
>> fs/btrfs/file.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
>> index e0d15c0d1641..ecbe186cb5da 100644
>> --- a/fs/btrfs/file.c
>> +++ b/fs/btrfs/file.c
>> @@ -3168,7 +3168,7 @@ static long btrfs_fallocate(struct file *file, int mode,
>>
>> /* First, check if we exceed the qgroup limit */
>> INIT_LIST_HEAD(&reserve_list);
>> - while (1) {
>> + while (cur_offset < alloc_end) {
>> em = btrfs_get_extent(BTRFS_I(inode), NULL, 0, cur_offset,
>> alloc_end - cur_offset, 0);
>> if (IS_ERR(em)) {
>> @@ -3204,8 +3204,6 @@ static long btrfs_fallocate(struct file *file, int mode,
>> }
>> free_extent_map(em);
>> cur_offset = last_byte;
>> - if (cur_offset >= alloc_end)
>> - break;
>> }
>>
>> /*
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
