Re: [PATCH 1/7] xfs: split inode data writeback from xfs_sync_inodes_ag

[Christoph Hellwig <hch@infradead.org>]

On Fri, May 15, 2009 at 10:19:11AM +0530, Sujit Karataparambil wrote:
>  should not there be an.
> 
>  error = xfs_flush_pages(ip, 0, -1, (flags & SYNC_WAIT) ?
>                        0 : XFS_B_ASYNC, FI_NONE);
> 
> for the out_wait. This will ensure flush while the xfs_ioend_wait is being
> waited for. Would this be an better way to flush the data than waiting for
> the inode to be flushed during power off or scheduler cycles.
> Would this be an performance hit.

For now I don't want to change behaviour here.    It only matters for
the SYNC_TRYLOCK case which is used for delalloc flushing on ENOSPC, so
it's not too important.

That beeing said I don't really like the current implementation where we
have a SYNC_WAIT that waits for completion of data I/O and need a
separate SYNC_IOWAIT that waits for the after data I/O metadata
transaction completions.  I think we would be better unifying the two,
especially given the current callers:

fs/xfs/linux-2.6/xfs_quotaops.c:        return -xfs_sync_data(mp, 0);
fs/xfs/linux-2.6/xfs_super.c:           xfs_sync_data(mp, 0);
fs/xfs/linux-2.6/xfs_sync.c:    xfs_sync_data(mp, 0);
fs/xfs/linux-2.6/xfs_sync.c:    xfs_sync_data(mp, SYNC_WAIT|SYNC_IOWAIT);
fs/xfs/linux-2.6/xfs_sync.c:    xfs_sync_data(mp, SYNC_TRYLOCK);
fs/xfs/linux-2.6/xfs_sync.c:    xfs_sync_data(mp, SYNC_TRYLOCK | SYNC_IOWAIT);

So in most cases we do a purely asynchronous writeout, we have one case
that does a full synchronous writeout (SYNC_WAIT|SYNC_IOWAIT) and we
have the two ENOSPC flushing cases doing SYNC_TRYLOCK + async writeout
and SYNC_TRYLOCK + IOWAIT.  I don't really see any reason to only do the
IOWAIT here and will try to unify the two flags at some point.

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