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

Christoph Hellwig
2011-11-17T07:40:05Z

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.