On Wed, Aug 24, 2011 at 01:59:26AM -0400, Christoph Hellwig wrote:
> The new concurrency managed workqueues are cheap enough that we can
> create them per-filesystem instead of global. This allows us to only
> flush items for the current filesystem during sync, and to remove the
> trylock or defer scheme on the ilock, which is not compatible with
> using the workqueue flush for integrity purposes in the sync code.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
The only issue I see with this is that it brings back per-filesystem
workqueue threads. Because all the workqueues are defined with
MEM_RECLAIM, there is a rescuer thread per workqueue that is used
when the CWMQ cannot allocate memory to queue the work to the
appropriate per-cpu queue.
Right now we have:
$ ps -ef |grep [x]fs
root 748 2 0 Aug23 ? 00:00:00 [xfs_mru_cache]
root 749 2 0 Aug23 ? 00:00:00 [xfslogd]
root 750 2 0 Aug23 ? 00:00:00 [xfsdatad]
root 751 2 0 Aug23 ? 00:00:00 [xfsconvertd]
$
where the xfslogd, xfsdatad and xfsconvertd are the rescuer threads.
I don't think this is a big problem, but it is definitely something
worth noting (at least in the commit message) given that we've
removed just about all the per-filesystem threads recently...
Cheers,
Dave.
> Index: xfs/fs/xfs/xfs_aops.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_aops.c 2011-08-23 04:35:20.822345321 +0200
> +++ xfs/fs/xfs/xfs_aops.c 2011-08-23 04:37:02.425128226 +0200
> @@ -131,30 +131,22 @@ static inline bool xfs_ioend_is_append(s
> * will be the intended file size until i_size is updated. If this write does
> * not extend all the way to the valid file size then restrict this update to
> * the end of the write.
> - *
> - * This function does not block as blocking on the inode lock in IO completion
> - * can lead to IO completion order dependency deadlocks.. If it can't get the
> - * inode ilock it will return EAGAIN. Callers must handle this.
> */
> -STATIC int
> +STATIC void
> xfs_setfilesize(
> xfs_ioend_t *ioend)
> {
> xfs_inode_t *ip = XFS_I(ioend->io_inode);
> xfs_fsize_t isize;
>
> - if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL))
> - return EAGAIN;
> -
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> isize = xfs_ioend_new_eof(ioend);
> if (isize) {
> trace_xfs_setfilesize(ip, ioend->io_offset, ioend->io_size);
> ip->i_d.di_size = isize;
> xfs_mark_inode_dirty(ip);
> }
> -
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> - return 0;
> }
If we are going to block here, then we probably should increase the
per-cpu concurrency of the work queue so that we can continue to
process other ioends while this one is blocked.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
