From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cuda.sgi.com (cuda2.sgi.com [192.48.176.25]) by oss.sgi.com (8.14.3/8.14.3/SuSE Linux 0.8) with ESMTP id qAKGiSvP194883 for ; Tue, 20 Nov 2012 10:44:28 -0600 Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by cuda.sgi.com with ESMTP id M5vwyxHZwDrWotSt (version=TLSv1 cipher=AES256-SHA bits=256 verify=NO) for ; Tue, 20 Nov 2012 08:46:37 -0800 (PST) Date: Tue, 20 Nov 2012 17:37:34 +0100 From: Jan Kara Subject: Re: [PATCH 2/2] xfs: fix direct IO nested transaction deadlock. Message-ID: <20121120163734.GE1408@quack.suse.cz> References: <1353410831-22653-1-git-send-email-david@fromorbit.com> <1353410831-22653-3-git-send-email-david@fromorbit.com> <20121120161015.GB18244@infradead.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20121120161015.GB18244@infradead.org> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: Christoph Hellwig Cc: Jan Kara , xfs@oss.sgi.com On Tue 20-11-12 11:10:15, Christoph Hellwig wrote: > On Tue, Nov 20, 2012 at 10:27:11PM +1100, Dave Chinner wrote: > > This was discovered on a filesystem with a log of only 10MB, and a > > log stripe unit of 256k whih increased the base reservations by > > 512k. Hence a allocation transaction requires 1.2MB of log space to > > be available instead of only 260k, and so greatly increased the > > chance that there wouldn't be enough log space available for the > > nested transaction to succeed. The key to reproducing it is this > > mkfs command: > > > > mkfs.xfs -f -d agcount=16,su=256k,sw=12 -l su=256k,size=2560b $SCRATCH_DEV > > > > The test case was a 1000 fsstress processes running with random > > freeze and unfreezes every few seconds. Thanks to Eryu Guan > > (eguan@redhat.com) for writing the test that found this on a system > > with a somewhat unique default configuration.... > > That sounds like something that could fit xfstests fairly easily. > > Re the patch - you're moving the transaction allocation back into the > end_io handler. That's what my original version did, and I'm pretty > sure you talked me out of it back then. I can't remember the details > but the list should have it. > > > @@ -151,9 +151,11 @@ xfs_setfilesize( > > /* > > * The transaction was allocated in the I/O submission thread, > > * thus we need to mark ourselves as beeing in a transaction > > - * manually. > > + * manually. Similarly for freeze protection. > > */ > > current_set_flags_nested(&tp->t_pflags, PF_FSTRANS); > > + rwsem_acquire_read(&VFS_I(ip)->i_sb->s_writers.lock_map[SB_FREEZE_FS-1], > > + 0, 1, _THIS_IP_); > > The comment above isn't true anymore, and the flags hack should be > removed. It still seems to be true for buffered IO or am I misreading the code? > I'm also not sure the freeze protection still works if the acquire is > outside the original broader scope protection. I'll defer to Jan on > this as I don't really understand this magic enough.q > should be removed respectively replaced with sb_start_intwrite/sb_end_intwrite It seems to work OK. If it were not for buffered IO path which allocates a transaction (and thus freeze protection) in xfs_vm_writepage() we could get rid of this lockdep magic. But so far we can't... Honza -- Jan Kara SUSE Labs, CR _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs