From patchwork Wed Oct 1 14:43:32 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sandeen X-Patchwork-Id: 395547 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 36B2D1400AB for ; Thu, 2 Oct 2014 00:43:39 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751717AbaJAOni (ORCPT ); Wed, 1 Oct 2014 10:43:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35818 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751214AbaJAOnh (ORCPT ); Wed, 1 Oct 2014 10:43:37 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s91EhWmb025824 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 1 Oct 2014 10:43:33 -0400 Received: from liberator.sandeen.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s91EhVsF023725 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 1 Oct 2014 10:43:32 -0400 Message-ID: <542C1314.3030603@redhat.com> Date: Wed, 01 Oct 2014 09:43:32 -0500 From: Eric Sandeen MIME-Version: 1.0 To: "Theodore Ts'o" , Andreas Dilger CC: ext4 development Subject: Re: Journal under-reservation bug on first >2G file References: <542B1C38.9010409@redhat.com> <542B1EFC.4050500@redhat.com> <20141001115320.GA2903@thunk.org> In-Reply-To: <20141001115320.GA2903@thunk.org> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On 10/1/14 6:53 AM, Theodore Ts'o wrote: > On Tue, Sep 30, 2014 at 03:36:17PM -0600, Andreas Dilger wrote: >>> >>> 1.5a) Always set the large_file feature with a fresh mkfs, insteadl >>> of relying on the accident of the resize inode being > 2G! >> >> I think that 1.5a is definitely the way to go for new mke2fs, I'm a >> bit surprised that we didn't do this for "-t ext4" a long time ago >> given that we've enabled lots of other features automatically. > > Yes, I agree that would be a good thing to do. I'll make the change > to mke2fs.conf. > >> There shouldn't be any problem to do this retroactively in e2fsck >> and potentially at mount time for filesystems that already have some >> features enabled that are post-large_file (e.g. extents, flex_bg, etc.) >> This definitely would not impose any compatibility issues, because any >> kernel that supports those features already understands large_file. > > That sounds like a plan. If we only enable it automatically at mount > time (iff we mounted the file system read/write) if any of the ext3 or > ext4 specific features are enabled, that should be completely safe. Ok, so do that, and don't bump the reservations? I suppose the size test & superblock write can be removed, then... This does bug me a little; at one point we were very carefully not enabling any new features by mounting with a new kernel; that was specific to mounting-ext2-with-ext4 etc, but it still feels slightly inconsistent. Although I guess we enable it today by mounting-and- writing-a-big-enough-file. Something like this should fix it too, though, with less unexpected behind-your-back behavior: -ERic --- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 3aa26e9..2f94cd6 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -2563,9 +2563,15 @@ retry_grab: * if there is delayed block allocation. But we still need * to journalling the i_disksize update if writes to the end * of file which has an already mapped buffer. + * If this write might need to update the superblock due to the + * filesize adding a new superblock feature flag, add that too. */ retry_journal: - handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, 1); + handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, + EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, + EXT4_FEATURE_RO_COMPAT_LARGE_FILE) ? + 1 : 2); + if (IS_ERR(handle)) { page_cache_release(page); return PTR_ERR(handle);