From patchwork Mon Jun 22 16:42:25 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Curt Wohlgemuth X-Patchwork-Id: 29007 Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 66469B7106 for ; Tue, 23 Jun 2009 02:42:32 +1000 (EST) Received: by ozlabs.org (Postfix) id 56D6EDDDA2; Tue, 23 Jun 2009 02:42:32 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id D9F28DDD0C for ; Tue, 23 Jun 2009 02:42:31 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750970AbZFVQm0 (ORCPT ); Mon, 22 Jun 2009 12:42:26 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751371AbZFVQm0 (ORCPT ); Mon, 22 Jun 2009 12:42:26 -0400 Received: from smtp-out.google.com ([216.239.45.13]:32834 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750970AbZFVQmZ convert rfc822-to-8bit (ORCPT ); Mon, 22 Jun 2009 12:42:25 -0400 Received: from zps75.corp.google.com (zps75.corp.google.com [172.25.146.75]) by smtp-out.google.com with ESMTP id n5MGgRvo014890 for ; Mon, 22 Jun 2009 09:42:27 -0700 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=google.com; s=beta; t=1245688947; bh=6MpRasKCbkvq3NTAPOF9cetVoW4=; h=DomainKey-Signature:MIME-Version:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type: Content-Transfer-Encoding:X-System-Of-Record; b=hvsgZLORJ0iDh9yDV5 io9g6IPnCABDtHVOO8tZkYKanZxExo9usL0Qm5Z9MG7RxEtpxuu1kvGr8A6NswYi4TT w== DomainKey-Signature: a=rsa-sha1; s=beta; d=google.com; c=nofws; q=dns; h=mime-version:in-reply-to:references:date:message-id:subject:from:to: cc:content-type:content-transfer-encoding:x-system-of-record; b=KjsSfaikUSSYW1jMFswWI5xrnjJZxiUnuADnB4IGo6MuZCtB5WZr+fqkPI+sS+8dZ xOM05q0Yr7UJd4Nhnxs1g== Received: from pzk39 (pzk39.prod.google.com [10.243.19.167]) by zps75.corp.google.com with ESMTP id n5MGfrFp006164 for ; Mon, 22 Jun 2009 09:42:25 -0700 Received: by pzk39 with SMTP id 39so173094pzk.8 for ; Mon, 22 Jun 2009 09:42:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.154.14 with SMTP id b14mr2813493wfe.187.1245688945383; Mon, 22 Jun 2009 09:42:25 -0700 (PDT) In-Reply-To: <20090617234604.GF7867@mit.edu> References: <6601abe90906171148w1431258fvd0afa105cda9b77b@mail.gmail.com> <20090617234604.GF7867@mit.edu> Date: Mon, 22 Jun 2009 09:42:25 -0700 Message-ID: <6601abe90906220942se70fb70w5481e178f1525dd8@mail.gmail.com> Subject: Re: RFC PATCH: ext4 no journal corruption with locale-gen From: Curt Wohlgemuth To: Theodore Tso Cc: ext4 development X-System-Of-Record: true Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Hi Ted: I think the following patch is sufficient. It explicitly sets the aops to ext4_writeback_aops if there is no delayed allocation and no journal. I tested the locale-gen example with all combinations of data=writeback data=ordered data=journal and delalloc nodelalloc and it works correctly now. The paths for writeback seem fine to me for an inode w/o a journal. Signed-off-by: Curt Wohlgemuth --- On Wed, Jun 17, 2009 at 4:46 PM, Theodore Tso wrote: > Hi Curt, > > Thanks for your analysis of the bug.  The reason for the strange logic > in ext4_set_aops() is because at the moment the code doesn't support > the combination of data=journalled && delalloc.  That's why it was > explicitly checking for ext4_should_order_data() and > ext4_should_writeback_data(). > > We have a check for this in ext4_fill_super(), so your patch should be > safe, since the combination of ext4_should_journal_data && > test_opt(inode->i_sb, DELALLOC) should never happen. > > As to your question of whether the nodelalloc and nojournal case > should really be ext4_journalled_aops, I suspect ext4_writeback_aops > makes more sense.  I haven't audited all of the code paths to make > sure they DTRT in the non-journalled case yet, though. > >                                                        - Ted > -- > 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 > -- 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 --- 2.6.26/fs/ext4/inode.c.orig 2009-06-09 20:05:27.000000000 -0700 +++ 2.6.26/fs/ext4/inode.c 2009-06-22 08:55:13.000000000 -0700 @@ -3442,15 +3442,12 @@ static const struct address_space_operat void ext4_set_aops(struct inode *inode) { - if (ext4_should_order_data(inode) && - test_opt(inode->i_sb, DELALLOC)) + if (test_opt(inode->i_sb, DELALLOC)) inode->i_mapping->a_ops = &ext4_da_aops; else if (ext4_should_order_data(inode)) inode->i_mapping->a_ops = &ext4_ordered_aops; - else if (ext4_should_writeback_data(inode) && - test_opt(inode->i_sb, DELALLOC)) - inode->i_mapping->a_ops = &ext4_da_aops; - else if (ext4_should_writeback_data(inode)) + else if (ext4_should_writeback_data(inode) || + EXT4_JOURNAL(inode) == NULL) inode->i_mapping->a_ops = &ext4_writeback_aops; else inode->i_mapping->a_ops = &ext4_journalled_aops;