On Wed, Nov 21, 2012 at 06:53:44AM +1100, Dave Chinner wrote:
> Right, I was concerned about blocking IO completion workers waiting
> for log reservations. I'm still concerned about that, but I don't
> see any way around it.
That's information that should be added to a comment..
> > > /*
> > > * 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's still used by buffered IO in that way.
It's conditionaly though, so there should at least be a "may" in the
sentence.
> > > if (ioend->io_type == XFS_IO_UNWRITTEN) {
> > > error = xfs_iomap_write_unwritten(ip, ioend->io_offset,
> > > + ioend->io_size);
> > > + goto done;
> > > + }
> > > +
> > > + /*
> > > + * For direct I/O we do not know if we need to allocate blocks or not so
> > > + * we can't preallocate an append transaction as that results in nested
> > > + * reservations and log space deadlocks. Hence allocate the transaction
> > > + * here. For buffered I/O we preallocate a transaction when submitting
> > > + * the IO.
> > > + */
> > > + if (ioend->io_isdirect && xfs_ioend_is_append(ioend)) {
> >
> > xfs_iomap_write_unwritten already updates the inode size, so this should
> > be an "else if"
>
> The unwritten branch jumps over this completely if it is taken, so
> it makes no difference. I can change it is you want....
Oh, right - I missed that. But it seems the else would do the same as
the goto done in your new version, and I generally prefer else if style
control flow for this over gotos.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
