From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752495AbcFFCZN (ORCPT ); Sun, 5 Jun 2016 22:25:13 -0400 Received: from mail.chinanetcenter.com ([123.103.13.31]:43654 "EHLO chinanetcenter.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752030AbcFFCZL (ORCPT ); Sun, 5 Jun 2016 22:25:11 -0400 Message-ID: <5754DEAF.7010705@chinanetcenter.com> Date: Mon, 06 Jun 2016 10:23:43 +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: tytso@mit.edu, adilger.kernel@dilger.ca CC: linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org 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: <1464868898-31336-1-git-send-email-linf@chinanetcenter.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID: AQAAf0Dp8U2v3lRXNLmzAA--.4016S2 X-Coremail-Antispam: 1UD129KBjvJXoW7tr4fKryDGrykXFyxuF43GFg_yoW8tFWkpF sxJF15Grs3Ww18uFs7u3Z8Gw18uw4xGry8Jr1Fyr18AFyUJrZ0ganrtFyFyF9Fk397AF98 ZF4FvayDCrs2937anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUU2Eb7Iv0xC_tr1lb4IE77IF4wAFc2x0x2IEx4CE42xK8VAvwI8I 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, Am I missing something or it's not important at all? Other things: [linfeng@localhost ext4]$ grep EXT4_MB_HINT_MERGE ./* -rHn ./ext4.h:111:#define EXT4_MB_HINT_MERGE 0x0001 ./mballoc.c:1871: } else if (max > 0 && (ac->ac_flags & EXT4_MB_HINT_MERGE)) { EXT4_MB_HINT_MERGE is only tested once and nowhere teaches how to use it. IIUC it also should be folded into EXT4_MB_HINT_TRY_GOAL path or simply skip EXT4_MB_HINT_MERGE test at -L1871. thanks, linfeng On 06/02/2016 08:01 PM, 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. > > 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; > } > >