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

Christoph Hellwig
2011-06-22T06:51:26Z

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.