From patchwork Thu Dec 27 06:29:07 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zheng Liu X-Patchwork-Id: 208270 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 598AC2C00D2 for ; Thu, 27 Dec 2012 17:15:48 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750870Ab2L0GPq (ORCPT ); Thu, 27 Dec 2012 01:15:46 -0500 Received: from mail-pb0-f44.google.com ([209.85.160.44]:60412 "EHLO mail-pb0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750791Ab2L0GPp (ORCPT ); Thu, 27 Dec 2012 01:15:45 -0500 Received: by mail-pb0-f44.google.com with SMTP id uo1so5182144pbc.17 for ; Wed, 26 Dec 2012 22:15:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:mail-followup-to :references:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=TJDpvCq3FAnbWZLssCUIGPsgDNuX/it0OqdtJUn2GIc=; b=VqttngMDeiorPKIg7w3rszMesxfJ3xeBIUeqVg5FbAwcKaeqjNzsUrM8z9Nrkq0vdo AVMZSez7Iw8wF6BbaEwGWX3xXCw+CWvjlNu789IveoJ39UD2i5K4W+Deo9eXVHeI0GYK UcOHuGEAcvsYNedx6mi0mW3ExL1EQwb3vsjRTTCmJocqTAh6ein5Lqhcr/P5/ZMcQwmY XPsW4M8CILsZdY2NujVak19IRchKVT8hqYIEsfrZ8CA2N7cGbK5rJUIlcuHuQpVie8RB xBnlkV4poBKFhRejdygFPjUos11jNpJKraQ2arFScJ0ffYrt2HQbAsM4wZg3mUu/0ah5 85Nw== X-Received: by 10.66.83.6 with SMTP id m6mr87223757pay.52.1356588944685; Wed, 26 Dec 2012 22:15:44 -0800 (PST) Received: from gmail.com ([182.92.247.2]) by mx.google.com with ESMTPS id rs7sm17229590pbc.2.2012.12.26.22.15.36 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 26 Dec 2012 22:15:43 -0800 (PST) Date: Thu, 27 Dec 2012 14:29:07 +0800 From: Zheng Liu To: Alexander Beregalov Cc: "Theodore Ts'o\"" , linux-ext4@vger.kernel.org, Dmitry Monakhov Subject: Re: 3.8.0-rc1: WARNING: at fs/ext4/page-io.c:232 Message-ID: <20121227062907.GA5001@gmail.com> Mail-Followup-To: Alexander Beregalov , "Theodore Ts'o\"" , linux-ext4@vger.kernel.org, Dmitry Monakhov References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Thu, Dec 27, 2012 at 03:27:04AM +0300, Alexander Beregalov wrote: > Hello > > Let me know if you need more info > > EXT4-fs (sda2): INFO: recovery required on readonly filesystem > EXT4-fs (sda2): write access will be enabled during recovery > EXT4-fs (sda2): orphan cleanup on readonly fs > EXT4-fs (sda2): ext4_orphan_cleanup: truncating inode 841849 to 0 bytes > ------------[ cut here ]------------ > WARNING: at fs/ext4/page-io.c:232 ext4_flush_unwritten_io+0x6b/0x80() > Hardware name: P35-DS3 > Modules linked in: > Pid: 1, comm: swapper/0 Not tainted 3.8.0-rc1-00004-g637704c #1 > Call Trace: > [] warn_slowpath_common+0x7a/0xb0 > [] warn_slowpath_null+0x15/0x20 > [] ext4_flush_unwritten_io+0x6b/0x80 > [] ext4_ext_truncate+0x2c/0x1f0 > [] ? ext4_msg+0x50/0x60 > [] ext4_truncate+0x70/0xb0 > [] ext4_fill_super+0x2bab/0x2ce0 > [] mount_bdev+0x1aa/0x1f0 > [] ? default_spin_lock_flags+0x9/0x10 > [] ? ext4_calculate_overhead+0x3a0/0x3a0 > [] ext4_mount+0x10/0x20 > [] mount_fs+0x1b/0xd0 > [] vfs_kern_mount+0x71/0x110 > [] do_mount+0x386/0x980 > [] ? strndup_user+0x53/0x70 > [] sys_mount+0x8b/0xe0 > [] mount_block_root+0xfe/0x298 > [] mount_root+0x56/0x5a > [] prepare_namespace+0x134/0x16d > [] kernel_init+0x196/0x2a0 > [] ? loglevel+0x31/0x31 > [] ? rest_init+0x80/0x80 > [] ret_from_fork+0x7c/0xb0 > [] ? rest_init+0x80/0x80 > ---[ end trace 425942f4f0ed8d07 ]--- > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 835709 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 835629 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 682715 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 832545 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 677529 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 838885 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 676342 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 832311 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 683216 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 828057 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 847476 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 834769 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 846534 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 842096 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 833886 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 688996 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 1085523 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 524364 > EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 12686 > EXT4-fs (sda2): 19 orphan inodes deleted > EXT4-fs (sda2): 1 truncate cleaned up > EXT4-fs (sda2): recovery complete > EXT4-fs (sda2): mounted filesystem with ordered data mode. Opts: (null) Hi Alexander, This warning is from ext4_flush_unwritten_io() because we need to take i_mutex lock before calling this function. Ohterwise we will trigger a WARN_ON_ONCE(). But, unfortunately, we don't take this lock in ext4_orphan_cleanup(). So that is why we will get this warning when cleaning up orphan list. Could you please test this patch? Thanks, - Zheng Subject: [PATCH] ext4: fixup a warning from ext4_flush_unwritten_io() in orphan list cleanup From: Zheng Liu When ext4 tries to clean up orphan list, we will get the following warning from ext4_flush_unwritten_io() because i_mutex lock doesn't be taken. EXT4-fs (sda2): INFO: recovery required on readonly filesystem EXT4-fs (sda2): write access will be enabled during recovery EXT4-fs (sda2): orphan cleanup on readonly fs EXT4-fs (sda2): ext4_orphan_cleanup: truncating inode 841849 to 0 bytes ------------[ cut here ]------------ WARNING: at fs/ext4/page-io.c:232 ext4_flush_unwritten_io+0x6b/0x80() Hardware name: P35-DS3 Modules linked in: Pid: 1, comm: swapper/0 Not tainted 3.8.0-rc1-00004-g637704c #1 Call Trace: [] warn_slowpath_common+0x7a/0xb0 [] warn_slowpath_null+0x15/0x20 [] ext4_flush_unwritten_io+0x6b/0x80 [] ext4_ext_truncate+0x2c/0x1f0 [] ? ext4_msg+0x50/0x60 [] ext4_truncate+0x70/0xb0 [] ext4_fill_super+0x2bab/0x2ce0 [] mount_bdev+0x1aa/0x1f0 [] ? default_spin_lock_flags+0x9/0x10 [] ? ext4_calculate_overhead+0x3a0/0x3a0 [] ext4_mount+0x10/0x20 [] mount_fs+0x1b/0xd0 [] vfs_kern_mount+0x71/0x110 [] do_mount+0x386/0x980 [] ? strndup_user+0x53/0x70 [] sys_mount+0x8b/0xe0 [] mount_block_root+0xfe/0x298 [] mount_root+0x56/0x5a [] prepare_namespace+0x134/0x16d [] kernel_init+0x196/0x2a0 [] ? loglevel+0x31/0x31 [] ? rest_init+0x80/0x80 [] ret_from_fork+0x7c/0xb0 [] ? rest_init+0x80/0x80 ---[ end trace 425942f4f0ed8d07 ]--- EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 835709 EXT4-fs (sda2): ext4_orphan_cleanup: deleting unreferenced inode 835629 Now we try to take i_mutex lock before orphan list cleanup, although we don't need to take it in ext4_orphan_cleanup() because no one write this inode. WARN_ON_ONCE doesn't be removed because this warning could help us to avoid some ciritcal errors. CC: Dmitry Monakhov Reported-by: Alexander Beregalov Signed-off-by: Zheng Liu --- fs/ext4/super.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 3cdb0a2..188d6f1 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -2212,7 +2212,18 @@ static void ext4_orphan_cleanup(struct super_block *sb, __func__, inode->i_ino, inode->i_size); jbd_debug(2, "truncating inode %lu to %lld bytes\n", inode->i_ino, inode->i_size); + /* + * Actually we don't need to take i_mutex lock + * because in orphan list cleanup no one can write + * this inode. We take it here because in calling + * ext4_flush_unwritten_io() this lock needs to be + * taken, and we don't want to remove this + * WARN_ON_ONCE(). It is useful for us to avoid some + * critical errors. + */ + mutex_lock(&inode->i_mutex); ext4_truncate(inode); + mutex_unlock(&inode->i_mutex); nr_truncates++; } else { ext4_msg(sb, KERN_DEBUG,