Re: [PATCH] ext4: do not normalize block requests from fallocate.

[Vivek Haldar <haldar@google.com>]

EXT4_GET_BLOCKS_* and EXT4_MB_HINT_* are different flag spaces and
should not be mixed. ext4_ext_map_blocks should not be using
EXT4_MB_HINT_*, it only uses EXT4_GET_BLOCKS_*.

On Mon, May 16, 2011 at 2:39 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> On Fri, 13 May 2011, Vivek Haldar wrote:
>
>> Currently, an fallocate request of size slightly larger than a power of
>> 2 is turned into two block requests, each a power of 2, with the extra
>> blocks pre-allocated for future use. When an application calls
>> fallocate, it already has an idea about how large the file may grow so
>> there is usually little benefit to reserve extra blocks on the
>> preallocation list. This reduces disk fragmentation.
>>
>> Tested: fsstress. Also verified manually that fallocat'ed files are
>> contiguously laid out with this change (whereas without it they begin at
>> power-of-2 boundaries, leaving blocks in between). CPU usage of
>> fallocate is not appreciably higher. In a tight fallocate loop, CPU
>> usage hovers between 5%-8% with this change, and 5%-7% without it.
>
> Hi Vivek,
>
> the patch looks good, but I do not understand why you are introducing
> new #define when you can simply use EXT4_MB_HINT_NOPREALLOC and then in
> the ext4_ext_map_blocks() just do :
>
> ar.flags |= (flags & EXT4_MB_HINT_NOPREALLOC)
>
> and you do not need to introduce new condition. Am I missing something ?
>
> Thanks!
> -Lukas
>
>>
>> Signed-off-by: Vivek Haldar <haldar@google.com>
>> ---
>>  fs/ext4/ext4.h    |    2 ++
>>  fs/ext4/extents.c |    5 ++++-
>>  2 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 076c5d2..e606d34 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -512,6 +512,8 @@ struct ext4_new_group_data {
>>       /* Convert extent to initialized after IO complete */
>>  #define EXT4_GET_BLOCKS_IO_CONVERT_EXT               (EXT4_GET_BLOCKS_CONVERT|\
>>                                        EXT4_GET_BLOCKS_CREATE_UNINIT_EXT)
>> +     /* Don't normalize when fallocat'ing */
>> +#define EXT4_GET_BLOCKS_NO_NORMALIZE         0x0020
>>
>>  /*
>>   * Flags used by ext4_free_blocks
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index e363f21..f02dd52 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -3304,6 +3304,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>>       else
>>               /* disable in-core preallocation for non-regular files */
>>               ar.flags = 0;
>> +     if (flags & EXT4_GET_BLOCKS_NO_NORMALIZE)
>> +             ar.flags |= EXT4_MB_HINT_NOPREALLOC;
>>       newblock = ext4_mb_new_blocks(handle, &ar, &err);
>>       if (!newblock)
>>               goto out2;
>> @@ -3549,7 +3551,8 @@ retry:
>>                       break;
>>               }
>>               ret = ext4_map_blocks(handle, inode, &map,
>> -                                   EXT4_GET_BLOCKS_CREATE_UNINIT_EXT);
>> +                                   EXT4_GET_BLOCKS_CREATE_UNINIT_EXT |
>> +                                   EXT4_GET_BLOCKS_NO_NORMALIZE);
>>               if (ret <= 0) {
>>  #ifdef EXT4FS_DEBUG
>>                       WARN_ON(ret <= 0);
>>
>
> --
>



-- 
Regards,
Vivek