Re: [PATCH] btrfs: Move loop termination condition in while()

[David Sterba <dsterba@suse.cz>]

On Wed, Nov 01, 2017 at 12:42:18PM +0200, Nikolay Borisov wrote:
> 
> 
> On  1.11.2017 11:46, Roman Mamedov wrote:
> > On Wed,  1 Nov 2017 11:32:18 +0200
> > Nikolay Borisov <nborisov@suse.com> 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.

Agreed, and

Reviewed-by: David Sterba <dsterba@suse.com>