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

Christoph Hellwig
2009-05-15T17:21:15Z

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.