Re: [PATCH 2/2] xfs: fix direct IO nested transaction deadlock.

[Jan Kara <jack@suse.cz>]

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 <jack@suse.cz>
SUSE Labs, CR

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs