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@xxxxxx>
...
> /*
> @@ -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.
> - queue_work(xfsdatad_workqueue, &ioend->io_work);
> + queue_work(mp->m_data_workqueue, &ioend->io_work);
> else
> xfs_destroy_ioend(ioend);
> }
...
> Index: linux-2.6/fs/xfs/xfs_super.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_super.c 2011-11-10 16:50:40.771795378 +0100
> +++ linux-2.6/fs/xfs/xfs_super.c 2011-11-15 09:17:13.763315819 +0100
> @@ -769,6 +769,42 @@ xfs_setup_devices(
> return 0;
> }
>
> +STATIC int
> +xfs_init_mount_workqueues(
> + struct xfs_mount *mp)
> +{
> + snprintf(mp->m_data_workqueue_name, XFS_WQ_NAME_LEN,
> + "xfs-data/%s", mp->m_fsname);
> + mp->m_data_workqueue =
> + alloc_workqueue(mp->m_data_workqueue_name, WQ_MEM_RECLAIM, 1);
> + if (!mp->m_data_workqueue)
> + goto out;
> +
> + snprintf(mp->m_unwritten_workqueue_name, XFS_WQ_NAME_LEN,
> + "xfs-conv/%s", mp->m_fsname);
> + mp->m_unwritten_workqueue =
> + alloc_workqueue(mp->m_unwritten_workqueue_name,
> + WQ_MEM_RECLAIM, 1);
Hrm... mp->m_fsname can be MAXNAMELEN (256 in xfs), and XFS_WQ_NAME_LEN
you chose is 512. As it stands there really isn't a problem here.
And, it sounds like you are wanting to replace this once Tejun improves
the interface... maybe that was worth pointing out.
> + if (!mp->m_unwritten_workqueue)
> + goto out_destroy_data_iodone_queue;
> +
> + return 0;
> +
> +out_destroy_data_iodone_queue:
> + destroy_workqueue(mp->m_data_workqueue);
> +out:
> + return -ENOMEM;
> +#undef XFS_WQ_NAME_LEN
Reviewed-by: Ben Myers <bpm@xxxxxxx>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
