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@xxxxxxxxxx) 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@xxxxxxx>
SUSE Labs, CR
|