Re: [PATCH] xfs: improve sync behaviour in face of aggressive dirtying

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jun 22, 2011 at 11:09:11AM +1000, Dave Chinner wrote:
> All good, except I think there's a small problem with this - we have
> to process the ioends before pages will transition from WRITEBACK to
> clean. i.e. it is not until xfs_ioend_destroy() that we call the
> bh->b_end_io() function to update the page state. Hence it would
> have to be:
> 
> xfs_fsync() {
> 
> 	current->journal_info = &ioend_end_list;
> 
> 	filemap_fdatawrite();
> 
> 	list_for_each_entry_reverse(ioend_end_list) {
> 		/* process_ioend also waits for ioend completion */
> 		process_ioend();
> 	}
> 
> 	current->journal_info = NULL;
> 
> 	filemap_fdatawait();

Indeed.

> Direct IO is another matter, but we've already got an
> xfs_ioend_wait() in xfs_fsync() to deal with that. Perhaps that
> could be moved over to your new DIO counter so we do block on all
> pending IO?

Splitting the pending direct I/O requests into the one is indeed the
plan.  We'll still need to track ioends for them, though - and I haven't
though about thedetails for those yet.

> > If that sounds reasonable I'll respin a series to move to
> > per-mount workqueues, remove the EAGAIN case, and use the workqueue
> > flush in sync.  Fsync will be left for later, and I'll ping Josef to
> > resend his fsync prototype change.
> 
> Yes, sounds like a plan.

I've implemented it yesterday, and it appears to work fine.  But there's
another issues I found:  the flush_workqueue will update i_size and mark
the inodes dirty right now from ->sync_fs, but that's after we've done
the VFS writeback.  I guess I nees to order this patch after the one
I'm working on to stop doing non-transaction inode updates.

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