From patchwork Tue Nov 23 10:02:39 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Piggin X-Patchwork-Id: 72642 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 A53B6B70DA for ; Tue, 23 Nov 2010 21:02:53 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751727Ab0KWKCv (ORCPT ); Tue, 23 Nov 2010 05:02:51 -0500 Received: from ipmail04.adl6.internode.on.net ([150.101.137.141]:53794 "EHLO ipmail04.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751024Ab0KWKCu (ORCPT ); Tue, 23 Nov 2010 05:02:50 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEAGYg60x5Ldhv/2dsb2JhbACiYXK8f4VLBI9whgg Received: from ppp121-45-216-111.lns20.cbr1.internode.on.net (HELO laptop.local0.net) ([121.45.216.111]) by ipmail04.adl6.internode.on.net with ESMTP; 23 Nov 2010 20:32:43 +1030 Received: by laptop.local0.net (Postfix, from userid 1000) id 185312981A; Tue, 23 Nov 2010 21:02:39 +1100 (EST) Date: Tue, 23 Nov 2010 21:02:39 +1100 From: Nick Piggin To: linux-fsdevel@vger.kernel.org, Andrew Morton , Al Viro , linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org, Jan Kara , Eric Sandeen , Theodore Ts'o Subject: [patch] fs: fix deadlocks in writeback_if_idle Message-ID: <20101123100239.GA4232@amd> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Taking s_umount lock inside i_mutex can result in an ABBA deadlock: ======================================================= [ INFO: possible circular locking dependency detected ] 2.6.37-rc3+ #26 ------------------------------------------------------- append_writer/12828 is trying to acquire lock: (&type->s_umount_key#24){+++++.}, at: [] writeback_inodes_sb_if_idle+0x32/0x60 but task is already holding lock: (&sb->s_type->i_mutex_key#14){+.+.+.}, at: [] generic_file_aio_write+0x53/0xd0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #3 (&sb->s_type->i_mutex_key#14){+.+.+.}: [] lock_acquire+0x95/0x1b0 [] __mutex_lock_common+0x59/0x480 [] mutex_lock_nested+0x3e/0x50 [] ext4_end_io_work+0x37/0xb0 [ext4] [] process_one_work+0x1b8/0x5a0 [] worker_thread+0x175/0x3a0 [] kthread+0x96/0xa0 [] kernel_thread_helper+0x4/0x10 -> #2 ((&io->work)){+.+...}: [] lock_acquire+0x95/0x1b0 [] process_one_work+0x1a4/0x5a0 [] worker_thread+0x175/0x3a0 [] kthread+0x96/0xa0 [] kernel_thread_helper+0x4/0x10 -> #1 (ext4-dio-unwritten){+.+...}: [] lock_acquire+0x95/0x1b0 [] flush_workqueue+0x148/0x540 [] ext4_sync_fs+0x3b/0x100 [ext4] [] __sync_filesystem+0x5e/0x90 [] sync_filesystem+0x32/0x60 [] generic_shutdown_super+0x2f/0x100 [] kill_block_super+0x2c/0x50 [] deactivate_locked_super+0x45/0x60 [] deactivate_super+0x45/0x60 [] mntput_no_expire+0xf0/0x190 [] sys_umount+0x79/0x3a0 [] system_call_fastpath+0x16/0x1b -> #0 (&type->s_umount_key#24){+++++.}: [] __lock_acquire+0x1382/0x1490 [] lock_acquire+0x95/0x1b0 [] down_read+0x42/0x60 [] writeback_inodes_sb_if_idle+0x32/0x60 [] ext4_da_write_begin+0x20d/0x310 [ext4] [] generic_file_buffered_write+0x114/0x2a0 [] __generic_file_aio_write+0x240/0x470 [] generic_file_aio_write+0x66/0xd0 [] ext4_file_write+0x3d/0xd0 [ext4] [] do_sync_write+0xd2/0x110 [] vfs_write+0xc8/0x190 [] sys_write+0x4c/0x80 [] system_call_fastpath+0x16/0x1b other info that might help us debug this: 1 lock held by append_writer/12828: #0: (&sb->s_type->i_mutex_key#14){+.+.+.}, at: [] generic_file_aio_write+0x53/0xd0 stack backtrace: Pid: 12828, comm: append_writer Not tainted 2.6.37-rc3+ #26 Call Trace: [] print_circular_bug+0xe9/0xf0 [] __lock_acquire+0x1382/0x1490 [] lock_acquire+0x95/0x1b0 [] ? writeback_inodes_sb_if_idle+0x32/0x60 [] ? sub_preempt_count+0x9d/0xd0 [] down_read+0x42/0x60 [] ? writeback_inodes_sb_if_idle+0x32/0x60 [] writeback_inodes_sb_if_idle+0x32/0x60 [] ext4_da_write_begin+0x20d/0x310 [ext4] [] ? up_read+0x1e/0x40 [] generic_file_buffered_write+0x114/0x2a0 [] ? current_fs_time+0x22/0x30 [] __generic_file_aio_write+0x240/0x470 [] ? generic_file_aio_write+0x53/0xd0 [] generic_file_aio_write+0x66/0xd0 [] ext4_file_write+0x3d/0xd0 [ext4] [] ? fsnotify+0x88/0x5e0 [] do_sync_write+0xd2/0x110 [] ? sub_preempt_count+0x9d/0xd0 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] ? smp_apic_timer_interrupt+0x6a/0xa0 [] vfs_write+0xc8/0x190 [] sys_write+0x4c/0x80 [] system_call_fastpath+0x16/0x1b Also, there can be an AA deadlock if the filesystem takes i_mutex in a workqueue, if it also waits for work completion while holding i_mutex. SysRq : Show Blocked State task PC stack pid father kworker/9:1 D 0000000000000000 6296 118 2 Call Trace: [] ? get_parent_ip+0x11/0x50 [] __mutex_lock_common+0x18c/0x480 [] ? ext4_end_io_work+0x37/0xb0 [ext4] [] ? ext4_end_io_work+0x37/0xb0 [ext4] [] mutex_lock_nested+0x3e/0x50 [] ext4_end_io_work+0x37/0xb0 [ext4] [] process_one_work+0x1b8/0x5a0 [] ? process_one_work+0x14e/0x5a0 [] ? ext4_end_io_work+0x0/0xb0 [ext4] [] worker_thread+0x175/0x3a0 [] ? worker_thread+0x0/0x3a0 [] kthread+0x96/0xa0 [] kernel_thread_helper+0x4/0x10 [] ? finish_task_switch+0x78/0x110 [] ? restore_args+0x0/0x30 [] ? kthread+0x0/0xa0 [] ? kernel_thread_helper+0x0/0x10 dbench D 0000000000000000 2872 2916 1 Call Trace: [] ? trace_hardirqs_off+0xd/0x10 [] schedule_timeout+0x1f5/0x350 [] ? mark_held_locks+0x6b/0xa0 [] ? _raw_spin_unlock_irq+0x2b/0x60 [] ? get_parent_ip+0x11/0x50 [] ? sub_preempt_count+0x9d/0xd0 [] wait_for_common+0x10d/0x190 [] ? default_wake_function+0x0/0x10 [] ? _raw_spin_unlock_bh+0x39/0x40 [] wait_for_completion+0x18/0x20 [] writeback_inodes_sb_nr+0xf7/0x120 [] writeback_inodes_sb+0x4d/0x60 [] ? writeback_inodes_sb_if_idle+0x32/0x60 [] writeback_inodes_sb_if_idle+0x3a/0x60 [] ext4_da_write_begin+0x20d/0x310 [] generic_file_buffered_write+0x114/0x2a0 Avoid both these issues by issuing completely asynchronous writeback request in writeback_inodes_sb_if_idle. Don't let that fool you into thinking these functions don't suck any more. ext4 now passes extensive stress testing with xfstests, fs_mark, dbench, with a writeback_inodes_if_idle call added directly into ext4_da_write_begin to trigger the path frequently. Previously it would spew lockdep stuff and hang in a number of ways very quickly. Signed-off-by: Nick Piggin --- fs/fs-writeback.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) -- 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 Index: linux-2.6/fs/fs-writeback.c =================================================================== --- linux-2.6.orig/fs/fs-writeback.c 2010-11-23 20:57:23.000000000 +1100 +++ linux-2.6/fs/fs-writeback.c 2010-11-23 20:59:10.000000000 +1100 @@ -1152,16 +1152,17 @@ EXPORT_SYMBOL(writeback_inodes_sb); * * Invoke writeback_inodes_sb if no writeback is currently underway. * Returns 1 if writeback was started, 0 if not. + * + * Even if 1 is returned, writeback may not be started if memory allocation + * fails. This function makes no guarantees about anything. */ int writeback_inodes_sb_if_idle(struct super_block *sb) { if (!writeback_in_progress(sb->s_bdi)) { - down_read(&sb->s_umount); - writeback_inodes_sb(sb); - up_read(&sb->s_umount); + bdi_start_writeback(sb->s_bdi, get_nr_dirty_pages()); return 1; - } else - return 0; + } + return 0; } EXPORT_SYMBOL(writeback_inodes_sb_if_idle); @@ -1172,17 +1173,18 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idl * * Invoke writeback_inodes_sb if no writeback is currently underway. * Returns 1 if writeback was started, 0 if not. + * + * Even if 1 is returned, writeback may not be started if memory allocation + * fails. This function makes no guarantees about anything. */ int writeback_inodes_sb_nr_if_idle(struct super_block *sb, unsigned long nr) { if (!writeback_in_progress(sb->s_bdi)) { - down_read(&sb->s_umount); - writeback_inodes_sb_nr(sb, nr); - up_read(&sb->s_umount); + bdi_start_writeback(sb->s_bdi, nr); return 1; - } else - return 0; + } + return 0; } EXPORT_SYMBOL(writeback_inodes_sb_nr_if_idle);