Re: [PATCH 2/5] xfs: use per-filesystem I/O completion workqueues

[Christoph Hellwig <hch@infradead.org>]

On Wed, Nov 16, 2011 at 01:01:20PM -0600, Ben Myers wrote:
> On Tue, Nov 15, 2011 at 03:14:09PM -0500, Christoph Hellwig wrote:
> > commit 77d7a0c "xfs: Non-blocking inode locking in IO completion" introduced
> > a trylocked and defer scheme in xfs_setfilesize to avoid deadlocks when on
> > XFS filesystem is used ontop of another using the loop device, and we
> > fsync in the loop filesystem.
> > 
> > Now that we have the cheap enough concurrency managed workqueues, we can
> > create per-filesystem instead of global workqueues and remove this scheme
> > again, given that it has the potential of delaying size updates and is not
> > helpful once we start to log the inode size.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> ...
> 
> >  /*
> > @@ -168,10 +161,12 @@ xfs_finish_ioend(
> >  	struct xfs_ioend	*ioend)
> >  {
> >  	if (atomic_dec_and_test(&ioend->io_remaining)) {
> > +		struct xfs_mount	*mp = XFS_I(ioend->io_inode)->i_mount;
> > +
> >  		if (ioend->io_type == IO_UNWRITTEN)
> > -			queue_work(xfsconvertd_workqueue, &ioend->io_work);
> > +			queue_work(mp->m_unwritten_workqueue, &ioend->io_work);
> >  		else if (xfs_ioend_is_append(ioend))
> 
> I wonder if we could skip size updates due to the 'fast and loose'
> nature of xfs_ioend_is_append, and end up destroying the ioend below,
> without updating the file size.  It's not strictly related to your patch
> though.

No - xfs_ioend_is_append check that the offset is beyond the on-disk
inode size.  The loose part is that we don't bother with the in-core
i_size and i_new_size which could change due to I/O errors.  di_size
on the other hand will only go downwards during truncate, and we make
sure all outstanding buffered I/Os have finished first.

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