xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: improve sync behaviour in face of aggressive dirtying
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 22 Jun 2011 02:51:13 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx, Wu Fengguang <fengguang.wu@xxxxxxxxx>
In-reply-to: <20110622010911.GS32466@dastard>
References: <20110617131401.GC2141@xxxxxxxxxxxxx> <20110620081802.GA27111@xxxxxxxxxxxxx> <20110621003343.GJ32466@dastard> <20110621092920.GA24540@xxxxxxxxxxxxx> <20110622010911.GS32466@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
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.

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