Re: [PATCH 0/2] Improve writeout pattern from xfs_flush_pages()

[Jan Kara <jack@suse.cz>]

On Wed 03-08-11 17:42:06, Christoph Hellwig wrote:
> On Wed, Aug 03, 2011 at 10:49:03PM +0200, Jan Kara wrote:
> > 
> >   Hi,
> > 
> >   at one of customer's machines, I've spotted an issue that sync(1) called
> > after writing a single huge file has been achieving rather low throughput. After
> > debugging this with blktrace, I've found that the culprit was in flusher thread
> > racing with page writeout happening from XFS sync code. The patches below helped
> > that case. Although they are not a complete solution, I belive they are useful
> > anyway so please consider merging them...
> 
> We currently have three calls to xfs_flush_pages with XBF_ASYNC set:
> 
>  - xfs_setattr_size
>  - xfs_sync_inode_data
>  - xfs_release
> 
> The first one actually is a synchronous writeout, just implemented in
> a rather odd way by doing the xfs_ioend_wait right after it, so your
> change is actively harmful for it.
  Oh, right. BTW cannot be truncate livelocked on a busy file because of
that xfs_ioend_wait()?

> The second is only called from xfs_flush_worker, which is the workqueue
> offload when we hit ENOSPC.  I can see how this might race with the
> writeback code, but the correct fix is to replace it with a call to
> writeback_inodes_sb(_if_idle) on that one is fixed to do a trylock on
> s_umount and thus won't deadlock.
  OK.

> The third one is opportunistic writeout if a file got truncated down on
> final release.  filemap_flush probably is fine here, but there's no need
> for a range version.  If you replace it with filemap_flush please also
> kill the useless wrapper while you're at it.
  Do you mean xfs_flush_pages()? OK, I can do that.

  Thanks for having a look.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR