From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754591AbcFHGJO (ORCPT ); Wed, 8 Jun 2016 02:09:14 -0400 Received: from mail.chinanetcenter.com ([123.103.13.31]:53941 "EHLO chinanetcenter.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751477AbcFHGJM (ORCPT ); Wed, 8 Jun 2016 02:09:12 -0400 Message-ID: <5757B655.6080803@chinanetcenter.com> Date: Wed, 08 Jun 2016 14:08:21 +0800 From: Lin Feng User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Andreas Dilger CC: "Theodore Ts'o" , linux-ext4 , LKML Subject: Re: [PATCH] ext4: mballoc.c: fix ac_g_ex and ac_f_ex misuse bug in EXT4_MB_HINT_TRY_GOAL path References: <1464868898-31336-1-git-send-email-linf@chinanetcenter.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID: AQAAf0AZ9i1UtldXO4XHAA--.1858S2 X-Coremail-Antispam: 1UD129KBjvJXoWxCryUCF1kJw4DCF17tFW5KFg_yoWrGF48pr Z7GF15GFs7Wr1UCFs293Z8Gr48tw4fGr15Xr1jvFyUGFyUAF93Xanrta48Wrn3uw4Yyan5 Za10gay3ursFkaDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUU2Eb7Iv0xC_Kw4lb4IE77IF4wAFc2x0x2IEx4CE42xK8VAvwI8I cIk0rVWrJVCq3wA2ocxC64kIII0Yj41le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqx4xG64 xvF2IEw4CE5I8CrVC2j2WlYx0E74AGY7Cv6cx26FyDJr1UJwAm72CE4IkC6x0Yz7v_Jr0_ Gr1lF7xvr2IY64vIr41lc7I2V7IY0VAS07AlzVAYIcxG8wCF04k20xvY0x0EwIxGrwCF04 k20xvE74AGY7Cv6cx26FyDJr1UJwC20s026c02F40E14v26r1j6r18MI8I3I0E7480Y4vE 14v26r106r1rMI8E67AF67kF1VAFwI0_JF0_Jw1lIxkGc2Ij64vIr4UvcSsGvfC2KfnxnU UI43ZEXa7xUUUUUUUUUUU== X-CM-SenderInfo: holqwqxfkl0t5qhwuvhqwh2hhfrp/ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andreas, Thanks for your reply and review. On 06/08/2016 05:01 AM, Andreas Dilger wrote: > On Jun 2, 2016, at 6:01 AM, Lin Feng wrote: >> >> Descriptions: >> ext4 block allocation core stack: >> ext4_mb_new_blocks >> ext4_mb_normalize_request >> ext4_mb_regular_allocator >> ext4_mb_find_by_goal >> mb_find_extent(e4b, ac->ac_g_ex.fe_start, ac->ac_g_ex.fe_len, &ex); >> >> The start block searching hint for merging(use EXT4_MB_HINT_TRY_GOAL flag) >> set in ext4_mb_normalize_request is stored in ac_f_ex, while in >> EXT4_MB_HINT_TRY_GOAL path which falls in ext4_mb_find_by_goal always use >> ac_g_ex as a hint and the hint set in ext4_mb_normalize_request is never >> use. >> >> We could hit this bug by writing a sparse file from backward mode and the >> file may get fragments even if the physical blocks in the hole is free, >> which is expected to be merged into a single extent. > > This looks reasonable. Do you have any kind of test case that shows the > effect of the change (e.g. fragmentation counts per file before/after)? I found this bug by fiddling the block allocation policy for ext4. In order to see the effect of this patch, we could do the following: On a fresh created ext4 fs, make a new topdir called b to hash the test to some blockgroup relatively empty, trying to not to be effected by original physical fragments. Then write a new file in backward mode. steps: 1. mkdir b && cd b 2. before this patch: [root@CentOS6 b]# rm dat -f && sync && sleep 2 && dd if=/dev/zero of=dat bs=4k count=256 seek=511 conv=notrunc && sync && sleep 2 && dd if=/dev/zero of=dat bs=4k count=254 seek=257 conv=notrunc && sync && filefrag -vv dat 256+0 records in 256+0 records out 1048576 bytes (1.0 MB) copied, 0.000825721 s, 1.3 GB/s 254+0 records in 254+0 records out 1040384 bytes (1.0 MB) copied, 0.000766912 s, 1.4 GB/s Filesystem type is: ef53 File size of dat is 3141632 (767 blocks, blocksize 4096) ext logical physical expected length flags 0 257 9512 254 1 511 557056 9766 256 eof dat: 2 extents found 3. after this patch: [root@CentOS6 b]# rm dat -f && sync && sleep 2 && dd if=/dev/zero of=dat bs=4k count=256 seek=511 conv=notrunc && sync && sleep 2 && dd if=/dev/zero of=dat bs=4k count=254 seek=257 conv=notrunc && sync && filefrag -vv dat 256+0 records in 256+0 records out 1048576 bytes (1.0 MB) copied, 0.000856416 s, 1.2 GB/s 254+0 records in 254+0 records out 1040384 bytes (1.0 MB) copied, 0.000669862 s, 1.6 GB/s Filesystem type is: ef53 File size of dat is 3141632 (767 blocks, blocksize 4096) ext logical physical expected length flags 0 257 556802 510 eof dat: 1 extent found thanks, linfeng > > Reviewed-by: Andreas Dilger > >> Signed-off-by: Lin Feng >> --- >> fs/ext4/mballoc.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c >> index c1ab3ec..e31fc63 100644 >> --- a/fs/ext4/mballoc.c >> +++ b/fs/ext4/mballoc.c >> @@ -3198,15 +3198,15 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac, >> if (ar->pright && (ar->lright == (start + size))) { >> /* merge to the right */ >> ext4_get_group_no_and_offset(ac->ac_sb, ar->pright - size, >> - &ac->ac_f_ex.fe_group, >> - &ac->ac_f_ex.fe_start); >> + &ac->ac_g_ex.fe_group, >> + &ac->ac_g_ex.fe_start); >> ac->ac_flags |= EXT4_MB_HINT_TRY_GOAL; >> } >> if (ar->pleft && (ar->lleft + 1 == start)) { >> /* merge to the left */ >> ext4_get_group_no_and_offset(ac->ac_sb, ar->pleft + 1, >> - &ac->ac_f_ex.fe_group, >> - &ac->ac_f_ex.fe_start); >> + &ac->ac_g_ex.fe_group, >> + &ac->ac_g_ex.fe_start); >> ac->ac_flags |= EXT4_MB_HINT_TRY_GOAL; >> } >> >> -- >> 1.9.3 >> >> > > > Cheers, Andreas > > > > >