From patchwork Tue Nov 30 23:22:38 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiaying Zhang X-Patchwork-Id: 73680 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 14B5A1007D1 for ; Wed, 1 Dec 2010 10:22:45 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751041Ab0K3XWn (ORCPT ); Tue, 30 Nov 2010 18:22:43 -0500 Received: from smtp-out.google.com ([216.239.44.51]:25801 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750948Ab0K3XWn (ORCPT ); Tue, 30 Nov 2010 18:22:43 -0500 Received: from hpaq7.eem.corp.google.com (hpaq7.eem.corp.google.com [172.25.149.7]) by smtp-out.google.com with ESMTP id oAUNMe5j005296; Tue, 30 Nov 2010 15:22:41 -0800 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=google.com; s=beta; t=1291159361; bh=QDq9A3Eiv3S/KCWQubrC7QpeS6c=; h=To:Subject:Cc:Message-Id:Date:From; b=kjfq9wPb/FIuN00UleU/TW/wOxqew56RCuW4y2x7r7j7RXTpT5ijihQ2L8TeZ5iCl AgIjgl3YuaTNtrp+Kg/Rg== DomainKey-Signature: a=rsa-sha1; s=beta; d=google.com; c=nofws; q=dns; h=to:subject:cc:message-id:date:from:x-system-of-record; b=SUHBmRzQcBHkzbNtelRaXwiOc8TpmS9ahomUx/eUlLY0jRosES4s79tygE+DSxVlg 2IwVJ+Ne8TWi7q7URorIg== Received: from ruihe.smo.corp.google.com (ruihe.smo.corp.google.com [172.29.60.109]) by hpaq7.eem.corp.google.com with ESMTP id oAUNMcVB020122; Tue, 30 Nov 2010 15:22:39 -0800 Received: by ruihe.smo.corp.google.com (Postfix, from userid 44100) id 4F5A74274C; Tue, 30 Nov 2010 15:22:38 -0800 (PST) To: adilger.kernel@dilger.ca, tytso@mit.ed Subject: [PATCH] discard an inode's preallocated blocks after failed allocation Cc: linux-ext4@vger.kernel.org Message-Id: <20101130232238.4F5A74274C@ruihe.smo.corp.google.com> Date: Tue, 30 Nov 2010 15:22:38 -0800 (PST) From: jiayingz@google.com (Jiaying Zhang) X-System-Of-Record: true Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org We have seen kernel crashes caused by the BUG_ON in ext4_mb_return_to_preallocation() that checks whether the inode's i_prealloc_list is empty. As I understand, the assumption is that when ext4_mb_return_to_preallocation() is called from ext4_free_blocks(), the inode's preallocation list should have been already discarded. However, although we call ext4_discard_preallocations() during ext4 truncate, we don't always call that function in various failure cases before calling ext4_free_blocks(). So it is likely to hit this BUG_ON with disk errors or corrupted fs etc. To fix the problem, the following patch adds ext4_discard_preallocation() before ext4_free_blocks() in failed allocation cases. This will discard any preallocated block extent attached to the inode, but I think it is probably what we should be doing with failed allocation. I am also curious whether we can drop the ext4_mb_return_to_preallocation() call from ext4_free_blocks(). From the comments above that function, it seems to intent to discard the specified blocks only but all it is currently doing is the BUG_ON check on whether the inode's preallocation list is empty. Is there any plan to extend this function later? ext4: discard an inode's preallocated blocks after failed allocation so that we won't hit on the BUG_ON check in ext4_mb_return_to_preallocation() later. Signed-off-by: Jiaying Zhang --- 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 29a4adf..2164df6 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1056,6 +1056,7 @@ cleanup: } if (err) { + ext4_discard_preallocations(inode); /* free all allocated blocks in error case */ for (i = 0; i < depth; i++) { if (!ablocks[i]) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 4f4362c..456cb4a 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -690,6 +690,7 @@ allocated: *err = 0; return ret; failed_out: + ext4_discard_preallocations(inode); for (i = 0; i < index; i++) ext4_free_blocks(handle, inode, 0, new_blocks[i], 1, 0); return ret; @@ -787,6 +788,7 @@ static int ext4_alloc_branch(handle_t *handle, struct inode *inode, return err; failed: /* Allocation failed, free what we already allocated */ + ext4_discard_preallocations(inode); ext4_free_blocks(handle, inode, 0, new_blocks[0], 1, 0); for (i = 1; i <= n ; i++) { /* @@ -878,6 +880,7 @@ static int ext4_splice_branch(handle_t *handle, struct inode *inode, return err; err_out: + ext4_discard_preallocations(inode); for (i = 1; i <= num; i++) { /* * branch[i].bh is newly allocated, so there is no