From patchwork Tue Mar 12 14:11:23 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lukas Czerner X-Patchwork-Id: 227030 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E2F312C008D for ; Wed, 13 Mar 2013 01:11:34 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932256Ab3CLOLd (ORCPT ); Tue, 12 Mar 2013 10:11:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14974 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754180Ab3CLOLd (ORCPT ); Tue, 12 Mar 2013 10:11:33 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2CEBRbE023065 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 12 Mar 2013 10:11:27 -0400 Received: from dhcp-1-104.brq.redhat.com (dhcp-1-104.brq.redhat.com [10.34.1.104]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2CEBNUx010153 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 12 Mar 2013 10:11:25 -0400 Date: Tue, 12 Mar 2013 15:11:23 +0100 (CET) From: =?ISO-8859-15?Q?Luk=E1=A8_Czerner?= X-X-Sender: lukas@dhcp-1-104.brq.redhat.com To: =?ISO-8859-15?Q?Luk=E1=A8_Czerner?= cc: "Theodore Ts'o" , Eric Whitney , linux-ext4@vger.kernel.org, Zheng Liu Subject: Re: [REGRESSION] allocated N with only M reserved metadata blocks In-Reply-To: Message-ID: References: <20130311185423.GA15478@thunk.org> <20130311210201.GA940@wallace> <20130311212239.GD15478@thunk.org> User-Agent: Alpine 2.00 (LFD 1167 2008-08-23) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Tue, 12 Mar 2013, Lukáš Czerner wrote: > Date: Tue, 12 Mar 2013 10:48:27 +0100 (CET) > From: Lukáš Czerner > To: Lukáš Czerner > Cc: Theodore Ts'o , Eric Whitney , > linux-ext4@vger.kernel.org > Subject: Re: [REGRESSION] allocated N with only M reserved metadata blocks > > On Tue, 12 Mar 2013, Lukáš Czerner wrote: > > > Date: Tue, 12 Mar 2013 08:58:53 +0100 (CET) > > From: Lukáš Czerner > > To: Theodore Ts'o > > Cc: Eric Whitney , linux-ext4@vger.kernel.org > > Subject: Re: [REGRESSION] allocated N with only M reserved metadata blocks > > > > On Mon, 11 Mar 2013, Theodore Ts'o wrote: > > > > > Date: Mon, 11 Mar 2013 17:22:39 -0400 > > > From: Theodore Ts'o > > > To: Eric Whitney > > > Cc: linux-ext4@vger.kernel.org > > > Subject: Re: [REGRESSION] allocated N with only M reserved metadata blocks > > > > > > On Mon, Mar 11, 2013 at 05:02:01PM -0400, Eric Whitney wrote: > > > > > > > > FWIW, this might not be a regression. I believe I've got this warning in my > > > > testing logs from both 3.8 and 3.8-rc7 (commit 01a523eb51 in 3.9-rc1 affects > > > > message format and line numbering) on both x86 and ARM. I didn't run xfstest > > > > 127 prior to 3.8-rc7, so I don't know how far back the warning may have > > > > occurred for that particular test. > > > > > > > > The 3.8-rc7 results are a little different with respect to number of warnings > > > > and test cases, so I'm thinking this one isn't completely deterministic, > > > > testing-wise. Multiple test runs may be required to see it. > > > > > > Thanks for the extra data; this saved be a whole bunch of time, since > > > I probably would have started doing a series git bisects tonight. :-) > > > > > > - Ted > > > > I found that the problem is present even in 3.7.0, however it is not > > present in 3.6.0. I guess we need to bisect now :) > > > > -Lukas > > Apparently the problem is caused by the commit: > > 67a5da564f97f31c4054d358e00b34d7ee570da5 ext4: make the zero-out > chunk size tunable > > Reverting that commit on 3.7 makes the warning disappear. I'll try > to see what's going on there, however from the nature of the change > it seems to me that the problem might have always been there but the > change just makes it easier/possible to hit. We'll see... > > -Lukas So there is indeed a problem with the mentioned commit 67a5da564f97f31c4054d358e00b34d7ee570da5 Due to the bug in that code is has exactly the opposite result - with this commit we will _never_ zero out blocks instead of creating uninitialized extents. In other words, we will always create uninitialized extent. This can be easily fixed by the following patch (which makes the warning go away), but it brings up a question whether this "optimization" was worth it in the first place since noone noticed that it had exactly the opposite effect than it should have had :) However it still does not resolve the issue completely, because even without the zeroout we should have had reserved enough metadata blocks to cover the extent split. I still need to investigate a little bit further. -Lukas diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index cac80120..aeb80bc 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3227,7 +3227,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle, if (EXT4_EXT_MAY_ZEROOUT & split_flag) max_zeroout = sbi->s_extent_max_zeroout_kb >> - inode->i_sb->s_blocksize_bits; + (inode->i_sb->s_blocksize_bits - 10); /* If extent is less than s_max_zeroout_kb, zeroout directly */ if (max_zeroout && (ee_len <= max_zeroout)) {