Re: stop using ioends for direct write completions

From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Christoph Hellwig <hch@infradead.org>,
	darrick.wong@oracle.com, xfs@oss.sgi.com
Subject: Re: stop using ioends for direct write completions
Date: Tue, 2 Feb 2016 10:31:18 -0500	[thread overview]
Message-ID: <20160202153117.GB1853@bfoster.bfoster> (raw)
In-Reply-To: <20160202112046.GB28777@lst.de>

On Tue, Feb 02, 2016 at 12:20:46PM +0100, Christoph Hellwig wrote:
> On Fri, Jan 29, 2016 at 09:12:33AM -0500, Brian Foster wrote:
> > My understanding is that the original requirement for ioends here was to
> > track the state necessary in order to defer (to wq) completions that had
> > to allocate transactions. Eventually, the deferred buffer state was
> > implemented and we no longer required an ioend for that, so we removed
> > the ioends here:
> > 
> >   2ba6623 xfs: don't allocate an ioend for direct I/O completions
> > 
> > Then just a couple months later, we merged the following to re-add the
> > ioend into the dio path:
> > 
> >   d5cc2e3f xfs: DIO needs an ioend for writes
> 
> And I complained about that one!  I didn't have time to provide a full
> analysis and the patches around this one were criticial enough.  But this
> patch has the analysis on why ioends are not needed and bad which should
> have been the full review back then.  Note that this new version is
> better than the old ioend removal as it does not just use the private
> data as a boolean, but as a bitmask so that additional information
> such as the COW status can be communicated.
> 

FWIW, I don't see any such review comments against the three versions of
the "DIO needs an ioend for writes" patch I have in my mailbox, but I
easily could have missed something..? But if there wasn't time, then
fair enough.

I'm just looking for context. I don't have much of an opinion on which
approach is used here. If it simplifies COW, then that seems good enough
reason to me to take this approach. I'm pointing this out more because
this code seems to have been rewritten the last couple of times we
needed to fix something, which makes backports particularly annoying.
The two patches above were associated with a broader enhancement and a
bug fix (respectively) as a sort of justification, whereas this post had
a much more vague purpose from what I could tell, and therefore why I at
least hadn't taken the time to review it.

If COW is the primary motivator, perhaps we can bundle it with that
work?

Brian

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

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