On Wed, Nov 21, 2012 at 04:59:00AM -0500, Christoph Hellwig wrote:
> 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.
OK.
> > > > 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.
OK, I'll fix that.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
