[Top] [All Lists]

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

To: Sujit Karataparambil <sjt.kar@xxxxxxxxx>
Subject: Re: [PATCH 1/7] xfs: split inode data writeback from xfs_sync_inodes_ag
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Fri, 15 May 2009 13:21:22 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <921ca19c0905142149m68c9717cg83db37c468769c4d@xxxxxxxxxxxxxx>
References: <20090514171233.942489000@xxxxxxxxxxxxxxxxxxxxxx> <20090514171558.014747000@xxxxxxxxxxxxxxxxxxxxxx> <921ca19c0905142149m68c9717cg83db37c468769c4d@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
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.

<Prev in Thread] Current Thread [Next in Thread>