From patchwork Thu May 14 21:23:25 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Theodore Ts'o X-Patchwork-Id: 27224 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 393C4B6F34 for ; Fri, 15 May 2009 07:23:38 +1000 (EST) Received: by ozlabs.org (Postfix) id 2B874DE017; Fri, 15 May 2009 07:23:38 +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 C50C9DE011 for ; Fri, 15 May 2009 07:23:37 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753169AbZENVXd (ORCPT ); Thu, 14 May 2009 17:23:33 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753064AbZENVXd (ORCPT ); Thu, 14 May 2009 17:23:33 -0400 Received: from THUNK.ORG ([69.25.196.29]:39386 "EHLO thunker.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752863AbZENVXc (ORCPT ); Thu, 14 May 2009 17:23:32 -0400 Received: from root (helo=closure.thunk.org) by thunker.thunk.org with local-esmtp (Exim 4.50 #1 (Debian)) id 1M4iOl-0000I6-Gs; Thu, 14 May 2009 17:23:27 -0400 Received: from tytso by closure.thunk.org with local (Exim 4.69) (envelope-from ) id 1M4iOk-0008QT-0z; Thu, 14 May 2009 17:23:26 -0400 Date: Thu, 14 May 2009 17:23:25 -0400 From: Theodore Tso To: Kevin Shanahan Cc: Andreas Dilger , Alex Tomas , linux-ext4@vger.kernel.org Subject: Re: More ext4 acl/xattr corruption - 4th occurence now Message-ID: <20090514212325.GG21316@mit.edu> References: <20090513062634.GE4972@kulgan> <20090514044011.GC11352@mit.edu> <20090514110659.GA5146@kulgan> <20090514132506.GD5146@kulgan> <20090514140732.GI11352@mit.edu> <20090514143014.GH5146@kulgan> <20090514161254.GJ11352@mit.edu> <20090514210244.GL5146@kulgan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20090514210244.GL5146@kulgan> User-Agent: Mutt/1.5.18 (2008-05-17) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: tytso@mit.edu X-SA-Exim-Scanned: No (on thunker.thunk.org); SAEximRunCond expanded to false Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Fri, May 15, 2009 at 06:32:45AM +0930, Kevin Shanahan wrote: > Okay, so now I've booted into 2.6.29.3 + check_block_validity patch + > short circuit i_cached_extent patch, mounted the fs without > nodelalloc. I was able to run the full exchange backup without > triggering the check_block_validity error. Great! So here's the final fix (it replaces the short circuit i_cached_extent patch) which I plan to push to Linus. It should be much less of a performance hit than simply short-circuiting i_cached_extent... Thanks so much for helping to find track this down!!! If ever someone deserved an "Ext4 Baker Street Irregulars" T-shirt, it would be you.... - Ted commit 039ed7a483fdcb2dbbc29f00cd0d74c101ab14c5 Author: Theodore Ts'o Date: Thu May 14 17:09:37 2009 -0400 ext4: Fix race in ext4_inode_info.i_cached_extent If one CPU is reading from a file while another CPU is writing to the same file different locations, there is nothing protecting the i_cached_extent structure from being used and updated at the same time. This could potentially cause the wrong location on disk to be read or written to, including potentially causing the corruption of the block group descriptors and/or inode table. Many thanks to Ken Shannah for helping to track down this problem. Signed-off-by: "Theodore Ts'o" Reviewed-by:Aneesh Kumar K.V Signed-off-by: "Theodore Ts'o" Reviewed-by: Aneesh Kumar K.V Tested-by: Kevin Shanahan --- 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/extents.c b/fs/ext4/extents.c index 172656c..e3a55eb 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1841,11 +1841,13 @@ ext4_ext_put_in_cache(struct inode *inode, ext4_lblk_t block, { struct ext4_ext_cache *cex; BUG_ON(len == 0); + spin_lock(&EXT4_I(inode)->i_block_reservation_lock); cex = &EXT4_I(inode)->i_cached_extent; cex->ec_type = type; cex->ec_block = block; cex->ec_len = len; cex->ec_start = start; + spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); } /* @@ -1902,12 +1904,17 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block, struct ext4_extent *ex) { struct ext4_ext_cache *cex; + int ret = EXT4_EXT_CACHE_NO; + /* + * We borrow i_block_reservation_lock to protect i_cached_extent + */ + spin_lock(&EXT4_I(inode)->i_block_reservation_lock); cex = &EXT4_I(inode)->i_cached_extent; /* has cache valid data? */ if (cex->ec_type == EXT4_EXT_CACHE_NO) - return EXT4_EXT_CACHE_NO; + goto errout; BUG_ON(cex->ec_type != EXT4_EXT_CACHE_GAP && cex->ec_type != EXT4_EXT_CACHE_EXTENT); @@ -1918,11 +1925,11 @@ ext4_ext_in_cache(struct inode *inode, ext4_lblk_t block, ext_debug("%u cached by %u:%u:%llu\n", block, cex->ec_block, cex->ec_len, cex->ec_start); - return cex->ec_type; + ret = cex->ec_type; } - - /* not in cache */ - return EXT4_EXT_CACHE_NO; +errout: + spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); + return ret; } /*