Re: [PATCH 2/3] xfs: use per-filesystem I/O completion workqueues
From: Dave Chinner <david@fromorbit.com>
Date: 2011-08-25 00:48:17
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@lst.de>
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.
quoted hunk ↗ jump to hunk
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@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs